Commit 775de9e8 by Martyn James

Merge pull request #7150 from edx/dcikatic/courseware_search_tracking

SOL-217 Adding basic analytics events logging for courseware search - In discussing with @mulby, we agreed to allow this to go in , and we'll add a story to improve the tests at a later time.
parents 3f2cd706 5d540360
...@@ -571,15 +571,13 @@ class TestCourseReIndex(CourseTestCase): ...@@ -571,15 +571,13 @@ class TestCourseReIndex(CourseTestCase):
self.assertEqual(response['results'], []) self.assertEqual(response['results'], [])
# Start manual reindex # Start manual reindex
errors = CoursewareSearchIndexer.do_course_reindex(modulestore(), self.course.id) CoursewareSearchIndexer.do_course_reindex(modulestore(), self.course.id)
self.assertEqual(errors, None)
self.html.display_name = "My expanded HTML" self.html.display_name = "My expanded HTML"
modulestore().update_item(self.html, ModuleStoreEnum.UserID.test) modulestore().update_item(self.html, ModuleStoreEnum.UserID.test)
# Start manual reindex # Start manual reindex
errors = CoursewareSearchIndexer.do_course_reindex(modulestore(), self.course.id) CoursewareSearchIndexer.do_course_reindex(modulestore(), self.course.id)
self.assertEqual(errors, None)
# Check results indexed now # Check results indexed now
response = perform_search( response = perform_search(
......
...@@ -6,10 +6,12 @@ import logging ...@@ -6,10 +6,12 @@ import logging
from django.utils.translation import ugettext as _ from django.utils.translation import ugettext as _
from opaque_keys.edx.locator import CourseLocator from opaque_keys.edx.locator import CourseLocator
from search.search_engine_base import SearchEngine from search.search_engine_base import SearchEngine
from eventtracking import tracker
from . import ModuleStoreEnum from . import ModuleStoreEnum
from .exceptions import ItemNotFoundError from .exceptions import ItemNotFoundError
# Use default index and document names for now # Use default index and document names for now
INDEX_NAME = "courseware_index" INDEX_NAME = "courseware_index"
DOCUMENT_TYPE = "courseware_content" DOCUMENT_TYPE = "courseware_content"
...@@ -36,6 +38,7 @@ class CoursewareSearchIndexer(object): ...@@ -36,6 +38,7 @@ class CoursewareSearchIndexer(object):
Add to courseware search index from given location and its children Add to courseware search index from given location and its children
""" """
error_list = [] error_list = []
indexed_count = 0
# TODO - inline for now, need to move this out to a celery task # TODO - inline for now, need to move this out to a celery task
searcher = SearchEngine.get_search_engine(INDEX_NAME) searcher = SearchEngine.get_search_engine(INDEX_NAME)
if not searcher: if not searcher:
...@@ -115,6 +118,7 @@ class CoursewareSearchIndexer(object): ...@@ -115,6 +118,7 @@ class CoursewareSearchIndexer(object):
remove_index_item_location(location) remove_index_item_location(location)
else: else:
index_item_location(location, None) index_item_location(location, None)
indexed_count += 1
except Exception as err: # pylint: disable=broad-except except Exception as err: # pylint: disable=broad-except
# broad exception so that index operation does not prevent the rest of the application from working # broad exception so that index operation does not prevent the rest of the application from working
log.exception( log.exception(
...@@ -127,9 +131,46 @@ class CoursewareSearchIndexer(object): ...@@ -127,9 +131,46 @@ class CoursewareSearchIndexer(object):
if raise_on_error and error_list: if raise_on_error and error_list:
raise SearchIndexingError(_('Error(s) present during indexing'), error_list) raise SearchIndexingError(_('Error(s) present during indexing'), error_list)
return indexed_count
@classmethod
def do_publish_index(cls, modulestore, location, delete=False, raise_on_error=False):
"""
Add to courseware search index published section and children
"""
indexed_count = cls.add_to_search_index(modulestore, location, delete, raise_on_error)
cls._track_index_request('edx.course.index.published', indexed_count, str(location))
return indexed_count
@classmethod @classmethod
def do_course_reindex(cls, modulestore, course_key): def do_course_reindex(cls, modulestore, course_key):
""" """
(Re)index all content within the given course (Re)index all content within the given course
""" """
return cls.add_to_search_index(modulestore, course_key, delete=False, raise_on_error=True) indexed_count = cls.add_to_search_index(modulestore, course_key, delete=False, raise_on_error=True)
cls._track_index_request('edx.course.index.reindexed', indexed_count)
return indexed_count
@staticmethod
def _track_index_request(event_name, indexed_count, location=None):
"""Track content index requests.
Arguments:
location (str): The ID of content to be indexed.
event_name (str): Name of the event to be logged.
Returns:
None
"""
data = {
"indexed_count": indexed_count,
'category': 'courseware_index',
}
if location:
data['location_id'] = location
tracker.emit(
event_name,
data
)
...@@ -732,7 +732,7 @@ class DraftModuleStore(MongoModuleStore): ...@@ -732,7 +732,7 @@ class DraftModuleStore(MongoModuleStore):
self.signal_handler.send("course_published", course_key=course_key) self.signal_handler.send("course_published", course_key=course_key)
# Now it's been published, add the object to the courseware search index so that it appears in search results # Now it's been published, add the object to the courseware search index so that it appears in search results
CoursewareSearchIndexer.add_to_search_index(self, location) CoursewareSearchIndexer.do_publish_index(self, location)
return self.get_item(as_published(location)) return self.get_item(as_published(location))
......
...@@ -357,7 +357,7 @@ class DraftVersioningModuleStore(SplitMongoModuleStore, ModuleStoreDraftAndPubli ...@@ -357,7 +357,7 @@ class DraftVersioningModuleStore(SplitMongoModuleStore, ModuleStoreDraftAndPubli
) )
# Now it's been published, add the object to the courseware search index so that it appears in search results # Now it's been published, add the object to the courseware search index so that it appears in search results
CoursewareSearchIndexer.add_to_search_index(self, location) CoursewareSearchIndexer.do_publish_index(self, location)
return self.get_item(location.for_branch(ModuleStoreEnum.BranchName.published), **kwargs) return self.get_item(location.for_branch(ModuleStoreEnum.BranchName.published), **kwargs)
......
...@@ -12,12 +12,16 @@ import os ...@@ -12,12 +12,16 @@ import os
import pprint import pprint
import unittest import unittest
import inspect import inspect
import mock
from contextlib import contextmanager from contextlib import contextmanager
from lazy import lazy from lazy import lazy
from mock import Mock from mock import Mock
from operator import attrgetter from operator import attrgetter
from path import path from path import path
from eventtracking import tracker
from eventtracking.django import DjangoTracker
from xblock.field_data import DictFieldData from xblock.field_data import DictFieldData
from xblock.fields import ScopeIds, Scope, Reference, ReferenceList, ReferenceValueDict from xblock.fields import ScopeIds, Scope, Reference, ReferenceList, ReferenceValueDict
...@@ -54,11 +58,14 @@ class TestModuleSystem(ModuleSystem): # pylint: disable=abstract-method ...@@ -54,11 +58,14 @@ class TestModuleSystem(ModuleSystem): # pylint: disable=abstract-method
""" """
ModuleSystem for testing ModuleSystem for testing
""" """
def __init__(self, **kwargs): @mock.patch('eventtracking.tracker.emit')
def __init__(self, mock_emit, **kwargs): # pylint: disable=unused-argument
id_manager = CourseLocationManager(kwargs['course_id']) id_manager = CourseLocationManager(kwargs['course_id'])
kwargs.setdefault('id_reader', id_manager) kwargs.setdefault('id_reader', id_manager)
kwargs.setdefault('id_generator', id_manager) kwargs.setdefault('id_generator', id_manager)
kwargs.setdefault('services', {}).setdefault('field-data', DictFieldData({})) kwargs.setdefault('services', {}).setdefault('field-data', DictFieldData({}))
self.tracker = DjangoTracker()
tracker.register_tracker(self.tracker)
super(TestModuleSystem, self).__init__(**kwargs) super(TestModuleSystem, self).__init__(**kwargs)
def handler_url(self, block, handler, suffix='', query='', thirdparty=False): def handler_url(self, block, handler, suffix='', query='', thirdparty=False):
......
...@@ -62,7 +62,10 @@ define([ ...@@ -62,7 +62,10 @@ define([
}, },
error: function (self, xhr) { error: function (self, xhr) {
self.trigger('error'); self.trigger('error');
} },
add: true,
reset: false,
remove: false
}); });
}, },
......
...@@ -4,8 +4,9 @@ define([ ...@@ -4,8 +4,9 @@ define([
'jquery', 'jquery',
'underscore', 'underscore',
'backbone', 'backbone',
'gettext' 'gettext',
], function ($, _, Backbone, gettext) { 'logger'
], function ($, _, Backbone, gettext, Logger) {
'use strict'; 'use strict';
return Backbone.View.extend({ return Backbone.View.extend({
...@@ -17,6 +18,10 @@ define([ ...@@ -17,6 +18,10 @@ define([
'aria-label': 'search result' 'aria-label': 'search result'
}, },
events: {
'click .search-results-item a': 'logSearchItem',
},
initialize: function () { initialize: function () {
var template_name = (this.model.attributes.content_type === "Sequence") ? '#search_item_seq-tpl' : '#search_item-tpl'; var template_name = (this.model.attributes.content_type === "Sequence") ? '#search_item_seq-tpl' : '#search_item-tpl';
this.tpl = _.template($(template_name).html()); this.tpl = _.template($(template_name).html());
...@@ -25,9 +30,38 @@ define([ ...@@ -25,9 +30,38 @@ define([
render: function () { render: function () {
this.$el.html(this.tpl(this.model.attributes)); this.$el.html(this.tpl(this.model.attributes));
return this; return this;
},
/**
* Redirect to a URL. Mainly useful for mocking out in tests.
* @param {string} url The URL to redirect to.
*/
redirect: function(url) {
window.location.href = url;
},
logSearchItem: function(event) {
event.preventDefault();
var self = this;
var target = this.model.id;
var link = $(event.target).attr('href');
var collection = this.model.collection;
var page = collection.page;
var pageSize = collection.pageSize;
var searchTerm = collection.searchTerm;
var index = collection.indexOf(this.model);
Logger.log("edx.course.search.result_selected",
{
"search_term": searchTerm,
"result_position": (page * pageSize + index),
"result_link": target
}).always(function() {
self.redirect(link);
});
} }
}); });
}); });
})(define || RequireJS.define); })(define || RequireJS.define);
...@@ -57,7 +57,7 @@ define([ ...@@ -57,7 +57,7 @@ define([
var item = new SearchItemView({ model: result }); var item = new SearchItemView({ model: result });
return item.render().el; return item.render().el;
}); });
this.$el.find('.search-results').append(items); this.$el.find('.search-results').html(items);
}, },
totalCountMsg: function () { totalCountMsg: function () {
......
...@@ -2,6 +2,7 @@ define([ ...@@ -2,6 +2,7 @@ define([
'jquery', 'jquery',
'sinon', 'sinon',
'backbone', 'backbone',
'logger',
'js/common_helpers/template_helpers', 'js/common_helpers/template_helpers',
'js/search/views/search_form', 'js/search/views/search_form',
'js/search/views/search_item_view', 'js/search/views/search_item_view',
...@@ -14,6 +15,7 @@ define([ ...@@ -14,6 +15,7 @@ define([
$, $,
Sinon, Sinon,
Backbone, Backbone,
Logger,
TemplateHelpers, TemplateHelpers,
SearchForm, SearchForm,
SearchItemView, SearchItemView,
...@@ -117,6 +119,19 @@ define([ ...@@ -117,6 +119,19 @@ define([
expect(this.item.$el).toContainHtml(breadcrumbs); expect(this.item.$el).toContainHtml(breadcrumbs);
}); });
it('log request on follow item link', function () {
this.model.collection = new SearchCollection([this.model], { course_id: 'edx101' });
this.item.render();
// Mock the redirect call
spyOn(this.item, 'redirect').andCallFake( function() {} );
spyOn(this.item, 'logSearchItem').andCallThrough();
spyOn(Logger, 'log').andReturn($.Deferred().resolve());
var link = this.item.$el.find('a');
expect(link.length).toBe(1);
link.trigger('click');
expect(this.item.redirect).toHaveBeenCalled();
});
}); });
...@@ -353,6 +368,7 @@ define([ ...@@ -353,6 +368,7 @@ define([
expect(this.listView.$el).toContainHtml('Search Results'); expect(this.listView.$el).toContainHtml('Search Results');
expect(this.listView.$el).toContainHtml('this is a short excerpt'); expect(this.listView.$el).toContainHtml('this is a short excerpt');
searchResults[1] = searchResults[0]
this.collection.set(searchResults); this.collection.set(searchResults);
this.collection.totalCount = 2; this.collection.totalCount = 2;
this.listView.renderNext(); this.listView.renderNext();
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment