...

This is essentially a big brain dump for those who will be continuing to work on the forums code since I will be moving to a new product area soon.

Dead Features

There are several features that were either never completed or were removed but still have bits of code to support them floating around in both the back end and front end. These bits should be removed, as they have suffered from bit rot and are unnecessary distractions.

• Tagging of posts (including a feature to find related posts by tag similarity)
• Highlighting text within posts (presumably in search results)

LMS Python code

Permissions

...

We should use require.js for this code. One of the nasty consequences of not having done so is that the directory structure and file naming are crafted so that the files are loaded according to dependency order. This results in files that are not in the right place, oddly named classes and files, and multiple classes in one file. As we implement require.js, we should be sure to fix all of these problems.

Remove globals

Several bits of state are stored in global scope, including the list of users with privileged roles, the current user, and the course id. These should all be passed as appropriate to the objects and functions that need them.

Markdown and MathJax

These are consistent pain points for their complexity. In testing, we frequently turn functions related to these features into spies to avoid invoking them. We should figure out ways to isolate them and simplify the code that invokes them.

Use <button> instead of <a>

In many views and templates, we have special code to ensure that our things that look like buttons also act like buttons (i.e. use the button role and activate upon pressing the spacebar). We should really just use button elements, but we must ensure that we make any necessary CSS changes to avoid changing their appearance.

Better use of the Backbone framework

...

http://stackoverflow.com/questions/16823746/backbone-js-listento-vs-on

Let views create their own elements

Most of the time, we pass an existing element in the DOM as the el option when constructing a view. It is more efficient and concise to define tagName and className on the view and add its element to the page when rendering has completed (because the browser can do a single update).

Jasmine tests

Many of our Jasmine tests are really doing behavioral testing as opposed to unit testing. We should try to find ways to make these classes more unit testable and do behavioral testing in bok choy, which is the more appropriate framework for that kind of thing.

Specific files (paths relative to common/static/coffee/src/discussion)

discussion_filter.coffee

Duplicates functionality in DiscussionThreadListView; the two should share code to the extent possible.

discussion_router.coffee

This class currently acts as a main entry point for the discussion tab app. We should separate that behavior into another class and make DiscussionRouter actually just a Backbone Router. Also, we should implement more routes than the two currently available, which are the discussion home and the single-thread view. At the very least, we should have a route for a specific topic.

views/discussion_thread_list_view.coffee

This class currently does way too much. It manages all of the elements in the sidebar itself. This functionality should be broken out into more specific views.

The goHome function is only invoked by DiscussionRouter. It was originally implemented on DiscussionThreadListView because the sidebar used to have a home button that would return to the discussion home screen, but that functionality should really have resided all along in whatever view contains DiscussionThreadListView.

The updateSidebar function ensures that the sidebar appears to stay fixed relative to the viewport (but within the confines of the header/footer). This has historically been error prone, and it is computationally relatively expensive for something that is executed every time the user scrolls. Along with the fact that some users have specifically expressed displeasure with the feature, we should consider simply removing it.

views/discussion_thread_profile_view.coffee

This view should be consolidated with the other thread views. It used to be the case that the user profile page acted similarly to an inline discussion (i.e. each thread was presented in an abbreviated form and could be expanded and viewed inline). I do not know why this functionality was removed, but given the recent refactoring of the thread views, it could probably be reinstated without much difficulty.

views/discussion_thread_view.coffee

I think this view should be broken into four parts:

1. A full thread view (i.e. most of what it does now)
2. An abbreviated thread view
3. A container view that would switch between the abbreviated and full views when the thread is expanded/collapsed. This view would only be used for inline discussions (and maybe user profiles at some point)
4. A new response view

This would avoid complexity around re-rendering the body content (which has caused bugs around failing to render markup and XSS attacks) and hiding specific pieces of the full view (look through the templates for the post-extended-content class).

views/thread_response_view.coffee

The new comment view should be split off from this view.