refactor notes list and course list

[btbonval]

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.

karmaworld/karmaworld/assets/js/note-list.js

Lines 3 to 33 in 5dae386

	var dataTable = $('#data_table_list').dataTable({
	// we will set column widths explicitly
	'bAutoWidth': false,
	// don't provide a option for the user to change the table page length
	'bLengthChange': false,
	// sepcify the number of rows in a page
	'iDisplayLength': 20,
	// Position the filter bar at the top
	'sDom': dataTable_sDom,
	// Initial sorting
	'aaSorting': [[1,'desc']]
	});
	if (dataTable.length > 0) {
	// wire up sort chooser
	$('select.note-sort').change(function() {
	dataTable.fnSort([[$(this).val(), 'desc']]);
	});
	// sort by current value of sort chooser, since
	// the browser may change this from our default
	dataTable.fnSort([[$('select.note-sort').val(), 'desc']]);
	$('select.note-category').change(function() {
	var category = $(this).val();
	if (category === 'ALL') {
	dataTable.fnFilter('');
	} else {
	dataTable.fnFilter(category);
	}
	});

karmaworld/karmaworld/assets/js/course-list.js

Lines 18 to 81 in 96a0464

	var dataTable = $('#data_table_list').dataTable({
	// we will set column widths explicitly
	'autoWidth': false,
	// don't provide a option for the user to change the table page length
	'lengthChange': false,
	// sepcify the number of rows in a page
	'displayLength': 20,
	// Position the filter bar at the top
	'dom': '<"top">rt<"bottom"p><"clear">',
	// Initial sorting
	'aaSorting': [[2,'desc']],
	'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);
	});
	}
	});
	// wire up search box
	$('#search-courses').keyup(function() {
	dataTable.fnFilter($(this).val());
	});
	// wire up sort chooser
	$('#sort-by').change(function() {
	var sortCol = $(this).val();
	dataTable.fnSort([[sortCol, 'desc']]);
	});
	// sort by current value of sort chooser, since
	// the browser may change this from our default
	var sortCol = $('#sort-by').val();
	dataTable.fnSort([[sortCol, 'desc']]);
	// filter by chosen school
	$('#school-filter').change(function() {
	var schoolName = $(this).val();
	if (schoolName === 'ALL') {
	dataTable.fnFilter('');
	} else {
	dataTable.fnFilter(schoolName);
	}
	});
	// ensure that upon reload, any previous selection is used
	// (make a reload look like a change for triggering the above code)
	$('#school-filter').trigger('change');

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

[btbonval]

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]

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

• karmaworld/karmaworld/templates/courses/course_list.html

  Line 20 in df32667

  var dataTable_sDom = '<"top">rt<"bottom"p><"clear">';

• karmaworld/karmaworld/templates/courses/course_detail.html

  Line 12 in 5f0151a

  var dataTable_sDom = '<"top">rt<"bottom"p><"clear">';

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):

• karmaworld/karmaworld/templates/courses/course_detail.html

  Lines 106 to 120 in 5f0151a

  	<div id="note-search-bar" class="row">
  	<div class="small-12 large-3 columns">
  	{{ file_upload_form.fp_file }}
  	</div>
  	<div class="small-12 large-9 columns">
  	<form class="search-notes" action="{% url 'note_search' %}" method="GET">
  	<input type="hidden" name="course_id" value="{{ course.id }}" />
  	<input name="query" class="search" type="text" placeholder="Search Notes" />
  	<button type="submit" class="search-icon"><i class="fa fa-search inline"></i></button>
  	</form>
  	</div>
  	<div class="small-12 columns">
  	{% include 'partial/filepicker.html' %}
  	</div>
  	</div>

• karmaworld/karmaworld/templates/courses/course_list.html

  Lines 50 to 60 in df32667

  	<div id="course-search-bar">
  	<div class="row">
  	<div class="small-12 large-3 columns">
  	<button id="add-course-btn" class="inline-button important museo700" data-reveal-id="add-course-form">Add a Course</button>
  	</div>
  	<div class="small-12 large-9 columns">
  	<span><input id="search-courses" class="search adelle-sans" type="text" placeholder="Search Courses and Schools" />
  	<i class="fa fa-search inline search-icon"></i></span>
  	</div>
  	</div>
  	</div>

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

[btbonval]

Here's where the course-list ajax handling is defined.

karmaworld/karmaworld/apps/courses/views.py

Lines 262 to 323 in a1b255c

	def course_list_ajax_handler(request):
	request_dict = querystring_parser.parse(request.GET.urlencode())
	draw = int(request_dict['draw'])
	start = request_dict['start']
	length = request_dict['length']
	search = request_dict.get('search', None)
	objects = Course.objects.all()
	if search and search['value']:
	objects = objects.filter(Q(name__icontains=search['value']) |
	Q(school__name__icontains=search['value']) |
	Q(department__school__name__icontains=search['value']))
	order_fields = []
	for order_index in request_dict['order']:
	order_field = None
	order = request_dict['order'][order_index]
	if order['column'] == 1:
	order_field = 'updated_at'
	elif order['column'] == 2:
	order_field = 'file_count'
	elif order['column'] == 3:
	order_field = 'thank_count'
	if order['dir'] == 'desc':
	order_field = '-' + order_field
	if order_field:
	order_fields.append(order_field)
	objects = objects.order_by(*order_fields)
	displayRecords = objects.count()
	if start > 0:
	objects = objects[start:]
	objects = objects[:length]
	row_data = [
	[
	course_json(course),
	calendar.timegm(course.updated_at.timetuple()),
	course.file_count,
	course.thank_count,
	course.school.name if course.school else course.department.school.name,
	] for course in objects
	]
	response_dict = {
	'draw': draw,
	'recordsTotal': Course.objects.count(),
	'recordsFiltered': displayRecords,
	'data': row_data
	}
	return HttpResponse(json.dumps(response_dict), mimetype='application/json')
	def course_list_ajax(request):
	return ajax_base(request, course_list_ajax_handler, ['GET'])

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:

karmaworld/karmaworld/apps/courses/models.py

Lines 104 to 114 in 57c0252

	@register_channel_name('school_object_by_name')
	class SchoolLookup(AnonLookupChannel):
	"""
	Handles AJAX lookups against the school model's name and alias fields.
	"""
	model = School
	def get_query(self, q, request):
	""" Search against both name and alias. """
	query = models.Q(name__icontains=q) | models.Q(alias__icontains=q)
	return self.model.objects.filter(query)

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]

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.