refactor notes list and course list

btbonval commented 9 years ago

These two files have a lot of code in common and serve the same basic function, but differ based on whether notes or courses are being displayed. It seems that they should be refactored into unified code as much as possible.

https://github.com/FinalsClub/karmaworld/blob/5dae386ea2b4588ccc476ada06af8dcae66316af/karmaworld/assets/js/note-list.js#L3-L33

https://github.com/FinalsClub/karmaworld/blob/96a0464de624b5ebe1f11bbcec45ff274902b52f/karmaworld/assets/js/course-list.js#L18-L81

There are a number of differences between them. Comparing and contrasting will be a good practice before refactoring.

btbonval commented 9 years ago

mostly the same:

var dataTable = $('#data_table_list').dataTable({
  'bAutoWidth': false,
  'bLengthChange': false,

  // specify the number of rows in a page
  'iDisplayLength': 20,
  // or
  'displayLength': 20,

  // Position the filter bar at the top
  'sDom': dataTable_sDom,
  // or
  'dom': '<"top">rt<"bottom"p><"clear">',

  // Initial sorting
  'aaSorting': [[1,'desc']]
  // or
  'aaSorting': [[2,'desc']],
});

// only notes check for table length
if (dataTable.length > 0) {

  // searching.
  // courses is handled here, search notes exists but is handled via FORM submit
  // rather than JS filtering as done here for courses
  $('#search-courses').keyup(function() {
    dataTable.fnFilter($(this).val());
  });

  // sorting.
  $('select.note-sort').change(function() {
    dataTable.fnSort([[$(this).val(), 'desc']]);
  });
  // vs
  $('#sort-by').change(function() {
    var sortCol = $(this).val();
    dataTable.fnSort([[sortCol, 'desc']]);
  });

  // filtering.
  $('select.note-category').change(function() {
    var category = $(this).val();
    if (category === 'ALL') {
      dataTable.fnFilter('');
    } else {
      dataTable.fnFilter(category);
    }
  });
  // vs
  $('#school-filter').change(function() {
    var schoolName = $(this).val();
    if (schoolName === 'ALL') {
      dataTable.fnFilter('');
    } else {
      dataTable.fnFilter(schoolName);
    }
  });

  // updating on reload.
  // only found in notes-list
  dataTable.fnSort([[$('select.note-sort').val(), 'desc']]);
  // only found in courses-list
  $('#school-filter').trigger('change');

}

Here's a bunch of stuff that course-list does with dataTable that note-list does not. Looks like it is mostly hints on rendering and naming columns. There is some AJAX here but I'm not sure what it handles.

var dataTable = $('#data_table_list').dataTable({

// ...

  'paging': true,
  'columns': [
    { 'name': 'course', 'orderable': false, 'searchable': true, 'visible': true,
      'class': 'small-12 columns data-table-entry-wrapper' },
    { 'name': 'date', 'orderable': true, 'searchable': false, 'visible': false },
    { 'name': 'note_count', 'orderable': true, 'searchable': false, 'visible': false },
    { 'name': 'popularity', 'orderable': true, 'searchable': false, 'visible': false }
  ],
  'createdRow': function(row, data, index) {
    $(row).addClass('table-row');
  },
  // Use server-side processing
  'processing': true,
  'serverSide': true,
  'ajax': function(data, callback, settings) {
    $.get(course_list_ajax_url, data, function(dataWrapper, textStatus, jqXHR) {
      for (i = 0; i < dataWrapper.data.length; i++) {
        dataWrapper.data[i][0] = tableRow(dataWrapper.data[i][0]);
      }
      callback(dataWrapper);
    });
  }

// ...

});

btbonval commented 9 years ago

Both of the templates define the sDom weird string, so we should be able to make use of that in both JS files.

Searching on the notes list (course-detail) is done with a FORM, while course-list does it using JS (and so there are only some DOM placeholders in the template):

It shouldn't be too hard to convert notes searching to AJAX.

btbonval commented 9 years ago

Here's where the course-list ajax handling is defined. https://github.com/FinalsClub/karmaworld/blob/a1b255c7a8e4cabb054ac1dab7113015020e1090/karmaworld/apps/courses/views.py#L262-L323

It looks like that was written out by hand with loving care, but we already have AJAX select to simplify a lot of that kind of work. See for example: https://github.com/FinalsClub/karmaworld/blob/57c0252ee05a489f6218652efd8d85df830003bf/karmaworld/apps/courses/models.py#L104-L114

Although that example only supports filtering as written, it should be a bit cleaner to rewrite course_list_ajax_handler() as a LookupChannel and modify that for an equivalent AJAX lookup for notes.

btbonval commented 9 years ago

The course-list AJAX code supports ordering, but that ordering is overridden by the javascript ordering performed by dataTable. That chunk of order code is probably not needed.

FinalsClub / karmaworld

refactor notes list and course list #395