Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Bookmark Events

Browse Source 
TNL-1944
This commit is contained in:
Muhammad Ammar
2015-05-29 18:38:24 +05:00
committed by muzaffaryousaf
parent 3cbbb8f3b1
commit 1e9907a8c8
11 changed files with 271 additions and 45 deletions
Download Patch File Download Diff File Expand all files Collapse all files

52
common/test/acceptance/tests/lms/test_bookmarks.py
View File

@@ -44,6 +44,17 @@ class BookmarksTestMixin(EventsTestMixin, UniqueCourseTest):
]
self.course_fixture.add_children(*xblocks).install()
def verify_event_data(self, event_type, event_data):
"""
Verify emitted event data.
Arguments:
event_type: expected event type
event_data: expected event data
"""
actual_events = self.wait_for_events(event_filter={'event_type': event_type}, number_of_matches=1)
self.assert_events_match(event_data, actual_events)
class BookmarksTest(BookmarksTestMixin):
"""
@@ -213,13 +224,6 @@ class BookmarksTest(BookmarksTestMixin):
When I click on bookmarked link
Then I can navigate to correct bookmarked unit
"""
# NOTE: We are checking the order of bookmarked units at API
# We are unable to check the order here because we are bookmarking
# the units by sending POSTs to API, And the time(created) between
# the bookmarked units is in milliseconds. These milliseconds are
# discarded by the current version of MySQL we are using due to the
# lack of support. Due to which order of bookmarked units will be
# incorrect.
self._test_setup()
self._bookmark_units(2)
@@ -230,9 +234,10 @@ class BookmarksTest(BookmarksTestMixin):
bookmarked_breadcrumbs = self.bookmarks_page.breadcrumbs()
# Verify bookmarked breadcrumbs
# Verify bookmarked breadcrumbs and bookmarks order (most recently bookmarked unit should come first)
breadcrumbs = self._breadcrumb(2)
self.assertItemsEqual(bookmarked_breadcrumbs, breadcrumbs)
breadcrumbs.reverse()
self.assertEqual(bookmarked_breadcrumbs, breadcrumbs)
# get usage ids for units
xblocks = self.course_fixture.get_nested_xblocks(category="vertical")
@@ -289,3 +294,32 @@ class BookmarksTest(BookmarksTestMixin):
self.bookmarks_page.click_bookmarks_button()
self.assertTrue(self.bookmarks_page.results_present())
self.assertEqual(self.bookmarks_page.count(), 11)
def test_bookmarked_unit_accessed_event(self):
"""
Scenario: Bookmark events are emitted with correct data when we access/visit a bookmarked unit.
Given that I am a registered user
And I visit my courseware page
And I have bookmarked a unit
When I click on bookmarked unit
Then `edx.course.bookmark.accessed` event is emitted
"""
self._test_setup(num_chapters=1)
self.reset_event_tracking()
# create expected event data
xblocks = self.course_fixture.get_nested_xblocks(category="vertical")
event_data = [
{
'event': {
'bookmark_id': '{},{}'.format(self.USERNAME, xblocks[0].locator),
'component_type': xblocks[0].category,
'component_usage_id': xblocks[0].locator,
}
}
]
self._bookmark_unit(0)
self.bookmarks_page.click_bookmarks_button()
self.bookmarks_page.click_bookmarked_block(0)
self.verify_event_data('edx.bookmark.accessed', event_data)

26
lms/djangoapps/bookmarks/api.py
View File

@@ -1,7 +1,7 @@
"""
Bookmarks Python API.
"""
from eventtracking import tracker
from . import DEFAULT_FIELDS, OPTIONAL_FIELDS
from .models import Bookmark
from .serializers import BookmarkSerializer
@@ -68,10 +68,12 @@ def create_bookmark(user, usage_key):
Raises:
ItemNotFoundError: If no block exists for the usage_key.
"""
bookmark = Bookmark.create({
bookmark, created = Bookmark.create({
'user': user,
'usage_key': usage_key
})
if created:
_track_event('edx.bookmark.added', bookmark)
return BookmarkSerializer(bookmark, context={'fields': DEFAULT_FIELDS + OPTIONAL_FIELDS}).data
@@ -91,3 +93,23 @@ def delete_bookmark(user, usage_key):
"""
bookmark = Bookmark.objects.get(user=user, usage_key=usage_key)
bookmark.delete()
_track_event('edx.bookmark.removed', bookmark)
def _track_event(event_name, bookmark):
"""
Emit events for a bookmark.
Arguments:
event_name: name of event to track
bookmark: Bookmark object
"""
tracker.emit(
event_name,
{
'course_id': unicode(bookmark.course_key),
'bookmark_id': bookmark.resource_id,
'component_type': bookmark.usage_key.block_type,
'component_usage_id': unicode(bookmark.usage_key),
}
)

10
lms/djangoapps/bookmarks/models.py
View File

@@ -46,8 +46,7 @@ class Bookmark(TimeStampedModel):
bookmark_data['path'] = cls.get_path(block)
user = bookmark_data.pop('user')
bookmark, __ = cls.objects.get_or_create(usage_key=usage_key, user=user, defaults=bookmark_data)
return bookmark
return cls.objects.get_or_create(usage_key=usage_key, user=user, defaults=bookmark_data)
@staticmethod
def get_path(block):
@@ -69,3 +68,10 @@ class Bookmark(TimeStampedModel):
parents_data.reverse()
return parents_data
@property
def resource_id(self):
"""
Return the resource id: {username,usage_id}.
"""
return "{0},{1}".format(self.user.username, self.usage_key) # pylint: disable=no-member

8
lms/djangoapps/bookmarks/serializers.py
View File

@@ -11,7 +11,7 @@ class BookmarkSerializer(serializers.ModelSerializer):
"""
Serializer for the Bookmark model.
"""
id = serializers.SerializerMethodField('resource_id') # pylint: disable=invalid-name
id = serializers.Field(source='resource_id') # pylint: disable=invalid-name
course_id = serializers.Field(source='course_key')
usage_id = serializers.Field(source='usage_key')
block_type = serializers.Field(source='usage_key.block_type')
@@ -44,9 +44,3 @@ class BookmarkSerializer(serializers.ModelSerializer):
'path',
'created',
)
def resource_id(self, bookmark):
"""
Return the REST resource id: {username,usage_id}.
"""
return "{0},{1}".format(bookmark.user.username, bookmark.usage_key)

53
lms/djangoapps/bookmarks/tests/test_api.py
View File

@@ -8,6 +8,8 @@ from opaque_keys.edx.keys import UsageKey
from student.tests.factories import UserFactory
from util.testing import EventTestMixin
from xmodule.modulestore.exceptions import ItemNotFoundError
from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
@@ -17,7 +19,23 @@ from .. import api, DEFAULT_FIELDS, OPTIONAL_FIELDS
from ..models import Bookmark
class BookmarksAPITests(ModuleStoreTestCase):
class BookmarkApiEventTestMixin(EventTestMixin):
""" Mixin for verifying that bookmark api events were emitted during a test. """
def setUp(self): # pylint: disable=arguments-differ
super(BookmarkApiEventTestMixin, self).setUp('lms.djangoapps.bookmarks.api.tracker')
def assert_bookmark_event_emitted(self, event_name, course_id, bookmark_id, usage_key):
""" Assert that an event has been emitted. """
self.assert_event_emitted(
event_name,
course_id=course_id,
bookmark_id=bookmark_id,
component_type=usage_key.category,
component_usage_id=unicode(usage_key),
)
class BookmarksAPITests(BookmarkApiEventTestMixin, ModuleStoreTestCase):
"""
These tests cover the parts of the API methods.
"""
@@ -68,6 +86,8 @@ class BookmarksAPITests(ModuleStoreTestCase):
)
self.all_fields = DEFAULT_FIELDS + OPTIONAL_FIELDS
self.reset_tracker()
def assert_bookmark_response(self, response_data, bookmark, optional_fields=False):
"""
Determines if the given response data (dict) matches the given bookmark.
@@ -137,7 +157,14 @@ class BookmarksAPITests(ModuleStoreTestCase):
"""
self.assertEqual(len(api.get_bookmarks(user=self.user, course_key=self.course.id)), 1)
api.create_bookmark(user=self.user, usage_key=self.vertical_1.location)
bookmark_data = api.create_bookmark(user=self.user, usage_key=self.vertical_1.location)
self.assert_bookmark_event_emitted(
'edx.bookmark.added',
self.course_id,
bookmark_data['id'],
self.vertical_1.location
)
self.assertEqual(len(api.get_bookmarks(user=self.user, course_key=self.course.id)), 2)
@@ -148,12 +175,23 @@ class BookmarksAPITests(ModuleStoreTestCase):
self.assertEqual(len(api.get_bookmarks(user=self.user, course_key=self.course.id)), 1)
bookmark_data = api.create_bookmark(user=self.user, usage_key=self.vertical_1.location)
self.assert_bookmark_event_emitted(
'edx.bookmark.added',
self.course_id,
bookmark_data['id'],
self.vertical_1.location
)
self.assertEqual(len(api.get_bookmarks(user=self.user, course_key=self.course.id)), 2)
self.reset_tracker()
bookmark_data_2 = api.create_bookmark(user=self.user, usage_key=self.vertical_1.location)
self.assertEqual(len(api.get_bookmarks(user=self.user, course_key=self.course.id)), 2)
self.assertEqual(bookmark_data, bookmark_data_2)
self.assert_no_events_were_emitted()
def test_create_bookmark_raises_error(self):
"""
Verifies that create_bookmark raises error as expected.
@@ -161,6 +199,8 @@ class BookmarksAPITests(ModuleStoreTestCase):
with self.assertRaises(ItemNotFoundError):
api.create_bookmark(user=self.user, usage_key=UsageKey.from_string('i4x://brb/100/html/340ef1771a0940'))
self.assert_no_events_were_emitted()
def test_delete_bookmark(self):
"""
Verifies that delete_bookmark removes bookmark as expected.
@@ -169,6 +209,13 @@ class BookmarksAPITests(ModuleStoreTestCase):
api.delete_bookmark(user=self.user, usage_key=self.vertical.location)
self.assert_bookmark_event_emitted(
'edx.bookmark.removed',
self.course_id,
self.bookmark.resource_id,
self.vertical.location
)
bookmarks_data = api.get_bookmarks(user=self.user)
self.assertEqual(len(bookmarks_data), 1)
self.assertNotEqual(unicode(self.vertical.location), bookmarks_data[0]['usage_id'])
@@ -179,3 +226,5 @@ class BookmarksAPITests(ModuleStoreTestCase):
"""
with self.assertRaises(ObjectDoesNotExist):
api.delete_bookmark(user=self.other_user, usage_key=self.vertical.location)
self.assert_no_events_were_emitted()

2
lms/djangoapps/bookmarks/tests/test_models.py
View File

@@ -66,7 +66,7 @@ class BookmarkModelTest(ModuleStoreTestCase):
Tests creation of bookmark.
"""
bookmark_data = self.get_bookmark_data(self.vertical)
bookmark_object = Bookmark.create(bookmark_data)
bookmark_object, __ = Bookmark.create(bookmark_data)
self.assert_valid_bookmark(bookmark_object, bookmark_data)
def test_get_path(self):

85
lms/djangoapps/bookmarks/tests/test_views.py
View File

@@ -6,10 +6,12 @@ import ddt
import json
import urllib
from django.core.urlresolvers import reverse
from mock import patch
from rest_framework.test import APIClient
from student.tests.factories import UserFactory
from xmodule.modulestore import ModuleStoreEnum
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
@@ -148,12 +150,20 @@ class BookmarksListViewTests(BookmarksViewTestsMixin):
GET /api/bookmarks/v1/bookmarks/?course_id={course_id1}
POST /api/bookmarks/v1/bookmarks
"""
def assert_bookmark_listed_event_emitted(self, mock_tracker, **kwargs):
""" Assert that edx.course.bookmark.listed event is emitted with correct data. """
mock_tracker.assert_any_call(
'edx.bookmark.listed',
kwargs
)
@ddt.data(
('course_id={}', False),
('course_id={}&fields=path,display_name', True),
)
@ddt.unpack
def test_get_bookmarks_successfully(self, query_params, check_optionals):
@patch('lms.djangoapps.bookmarks.views.tracker.emit')
def test_get_bookmarks_successfully(self, query_params, check_optionals, mock_tracker):
"""
Test that requesting bookmarks for a course returns records successfully in
expected order without optional fields.
@@ -174,7 +184,17 @@ class BookmarksListViewTests(BookmarksViewTestsMixin):
self.assert_valid_bookmark_response(bookmarks[0], self.bookmark_2, optional_fields=check_optionals)
self.assert_valid_bookmark_response(bookmarks[1], self.bookmark_1, optional_fields=check_optionals)
def test_get_bookmarks_with_pagination(self):
self.assert_bookmark_listed_event_emitted(
mock_tracker,
course_id=self.course_id,
list_type='per_course',
bookmarks_count=2,
page_size=10,
page_number=1
)
@patch('lms.djangoapps.bookmarks.views.tracker.emit')
def test_get_bookmarks_with_pagination(self, mock_tracker):
"""
Test that requesting bookmarks for a course return results with pagination 200 code.
"""
@@ -191,7 +211,17 @@ class BookmarksListViewTests(BookmarksViewTestsMixin):
self.assertEqual(len(bookmarks), 1)
self.assert_valid_bookmark_response(bookmarks[0], self.bookmark_2)
def test_get_bookmarks_with_invalid_data(self):
self.assert_bookmark_listed_event_emitted(
mock_tracker,
course_id=self.course_id,
list_type='per_course',
bookmarks_count=2,
page_size=1,
page_number=1
)
@patch('lms.djangoapps.bookmarks.views.tracker.emit')
def test_get_bookmarks_with_invalid_data(self, mock_tracker):
"""
Test that requesting bookmarks with invalid data returns 0 records.
"""
@@ -200,7 +230,10 @@ class BookmarksListViewTests(BookmarksViewTestsMixin):
bookmarks = response.data['results']
self.assertEqual(len(bookmarks), 0)
def test_get_all_bookmarks_when_course_id_not_given(self):
self.assertFalse(mock_tracker.emit.called) # pylint: disable=maybe-no-member
@patch('lms.djangoapps.bookmarks.views.tracker.emit')
def test_get_all_bookmarks_when_course_id_not_given(self, mock_tracker):
"""
Test that requesting bookmarks returns all records for that user.
"""
@@ -212,6 +245,14 @@ class BookmarksListViewTests(BookmarksViewTestsMixin):
self.assert_valid_bookmark_response(bookmarks[1], self.bookmark_2)
self.assert_valid_bookmark_response(bookmarks[2], self.bookmark_1)
self.assert_bookmark_listed_event_emitted(
mock_tracker,
list_type='all_courses',
bookmarks_count=3,
page_size=10,
page_number=1
)
def test_anonymous_access(self):
"""
Test that an anonymous client (not logged in) cannot call GET or POST.
@@ -312,6 +353,42 @@ class BookmarksListViewTests(BookmarksViewTestsMixin):
self.assertEqual(405, self.client.put(reverse('bookmarks')).status_code)
self.assertEqual(405, self.client.delete(reverse('bookmarks')).status_code)
@patch('lms.djangoapps.bookmarks.views.tracker.emit')
@ddt.unpack
@ddt.data(
{'page_size': -1, 'expected_bookmarks_count': 2, 'expected_page_size': 10, 'expected_page_number': 1},
{'page_size': 0, 'expected_bookmarks_count': 2, 'expected_page_size': 10, 'expected_page_number': 1},
{'page_size': 999, 'expected_bookmarks_count': 2, 'expected_page_size': 500, 'expected_page_number': 1}
)
def test_listed_event_for_different_page_size_values(self, mock_tracker, page_size, expected_bookmarks_count,
expected_page_size, expected_page_number):
""" Test that edx.course.bookmark.listed event values are as expected for different page size values """
query_parameters = 'course_id={}&page_size={}'.format(urllib.quote(self.course_id), page_size)
self.send_get(client=self.client, url=reverse('bookmarks'), query_parameters=query_parameters)
self.assert_bookmark_listed_event_emitted(
mock_tracker,
course_id=self.course_id,
list_type='per_course',
bookmarks_count=expected_bookmarks_count,
page_size=expected_page_size,
page_number=expected_page_number
)
@patch('lms.djangoapps.bookmarks.views.tracker.emit')
def test_listed_event_for_page_number(self, mock_tracker):
""" Test that edx.course.bookmark.listed event values are as expected when we request a specific page number """
self.send_get(client=self.client, url=reverse('bookmarks'), query_parameters='page_size=2&page=2')
self.assert_bookmark_listed_event_emitted(
mock_tracker,
list_type='all_courses',
bookmarks_count=3,
page_size=2,
page_number=2
)
@ddt.ddt
class BookmarksDetailViewTests(BookmarksViewTestsMixin):

26
lms/djangoapps/bookmarks/views.py
View File

@@ -4,6 +4,7 @@ HTTP end-points for the Bookmarks API.
For more information, see:
https://openedx.atlassian.net/wiki/display/TNL/Bookmarks+API
"""
from eventtracking import tracker
import logging
from django.core.exceptions import ObjectDoesNotExist
@@ -166,6 +167,31 @@ class BookmarksListView(ListCreateAPIView, BookmarksViewMixin):
return api.get_bookmarks(user=self.request.user, course_key=course_key, serialized=False)
def paginate_queryset(self, queryset, page_size=None):
""" Override GenericAPIView.paginate_queryset for the purpose of eventing """
page = super(BookmarksListView, self).paginate_queryset(queryset, page_size)
course_id = self.request.QUERY_PARAMS.get('course_id')
if course_id:
try:
CourseKey.from_string(course_id)
except InvalidKeyError:
return page
event_data = {
'list_type': 'all_courses',
'bookmarks_count': page.paginator.count,
'page_size': self.get_paginate_by(),
'page_number': page.number,
}
if course_id is not None:
event_data['list_type'] = 'per_course'
event_data['course_id'] = course_id
tracker.emit('edx.bookmark.listed', event_data)
return page
def post(self, request):
"""
POST /api/bookmarks/v1/bookmarks/

19
lms/static/js/bookmarks/views/bookmarks_list.js
View File

@@ -1,7 +1,7 @@
;(function (define, undefined) {
'use strict';
define(['gettext', 'jquery', 'underscore', 'backbone', 'moment'],
function (gettext, $, _, Backbone, _moment) {
define(['gettext', 'jquery', 'underscore', 'backbone', 'logger', 'moment'],
function (gettext, $, _, Backbone, Logger, _moment) {
var moment = _moment || window.moment;
@@ -62,7 +62,20 @@
},
visitBookmark: function (event) {
window.location = event.target.pathname;
var bookmarkedComponent = $(event.currentTarget);
var bookmark_id = bookmarkedComponent.data('bookmarkId');
var component_usage_id = bookmarkedComponent.data('usageId');
var component_type = bookmarkedComponent.data('componentType');
Logger.log(
'edx.bookmark.accessed',
{
bookmark_id: bookmark_id,
component_type: component_type,
component_usage_id: component_usage_id
}
).always(function () {
window.location.href = event.currentTarget.pathname;
});
},
/**

33
lms/static/js/spec/bookmarks/bookmarks_list_view_spec.js
View File

@@ -1,7 +1,7 @@
define(['backbone', 'jquery', 'underscore', 'js/common_helpers/ajax_helpers', 'js/common_helpers/template_helpers',
'js/bookmarks/views/bookmarks_list_button'
define(['backbone', 'jquery', 'underscore', 'logger', 'js/common_helpers/ajax_helpers',
'js/common_helpers/template_helpers', 'js/bookmarks/views/bookmarks_list_button'
],
function (Backbone, $, _, AjaxHelpers, TemplateHelpers, BookmarksListButtonView) {
function (Backbone, $, _, Logger, AjaxHelpers, TemplateHelpers, BookmarksListButtonView) {
'use strict';
describe("lms.courseware.bookmarks", function () {
@@ -17,14 +17,14 @@ define(['backbone', 'jquery', 'underscore', 'js/common_helpers/ajax_helpers', 'j
'templates/bookmarks/bookmarks_list'
]
);
bookmarksButtonView = new BookmarksListButtonView();
spyOn(Logger, 'log').andReturn($.Deferred().resolve());
this.addMatchers({
toHaveBeenCalledWithUrl: function (expectedUrl) {
return expectedUrl === this.actual.argsForCall[0][0].target.pathname;
}
});
bookmarksButtonView = new BookmarksListButtonView();
});
var createBookmarksData = function (count) {
@@ -39,6 +39,7 @@ define(['backbone', 'jquery', 'underscore', 'js/common_helpers/ajax_helpers', 'j
created: new Date().toISOString(),
course_id: 'COURSE_ID',
usage_id: 'UNIT_USAGE_ID_' + i,
block_type: 'vertical',
path: [
{display_name: 'SECTION_DISAPLAY_NAME', usage_id: 'SECTION_USAGE_ID'},
{display_name: 'SUBSECTION_DISAPLAY_NAME', usage_id: 'SUBSECTION_USAGE_ID'}
@@ -68,17 +69,21 @@ define(['backbone', 'jquery', 'underscore', 'js/common_helpers/ajax_helpers', 'j
expect(bookmarks.length, results.length);
for(var b = 0; b < results.length; b++) {
courseId = results[b].course_id;
usageId = results[b].usage_id;
for(var bookmark_index = 0; bookmark_index < results.length; bookmark_index++) {
courseId = results[bookmark_index].course_id;
usageId = results[bookmark_index].usage_id;
expect(bookmarks[b]).toHaveAttr('href', createBookmarkUrl(courseId, usageId));
expect(bookmarks[bookmark_index]).toHaveAttr('href', createBookmarkUrl(courseId, usageId));
expect($(bookmarks[b]).find('.list-item-breadcrumbtrail').html().trim()).
toBe(breadcrumbTrail(results[b].path, results[b].display_name));
expect($(bookmarks[bookmark_index]).data('bookmarkId')).toBe(bookmark_index);
expect($(bookmarks[bookmark_index]).data('componentType')).toBe('vertical');
expect($(bookmarks[bookmark_index]).data('usageId')).toBe(usageId);
expect($(bookmarks[b]).find('.list-item-date').text().trim()).
toBe('Bookmarked on ' + view.humanFriendlyDate(results[b].created));
expect($(bookmarks[bookmark_index]).find('.list-item-breadcrumbtrail').html().trim()).
toBe(breadcrumbTrail(results[bookmark_index].path, results[bookmark_index].display_name));
expect($(bookmarks[bookmark_index]).find('.list-item-date').text().trim()).
toBe('Bookmarked on ' + view.humanFriendlyDate(results[bookmark_index].created));
}
};

2
lms/templates/bookmarks/bookmarks_list.underscore
View File

@@ -5,7 +5,7 @@
<div class='bookmarks-results-list'>
<% _.each(bookmarks, function(bookmark, index) { %>
<a class="bookmarks-results-list-item" href="<%= bookmark.blockUrl() %>" aria-labelledby="bookmark-link-<%= index %>" aria-describedby="bookmark-type-<%= index %> bookmark-date-<%= index %>">
<a class="bookmarks-results-list-item" href="<%= bookmark.blockUrl() %>" aria-labelledby="bookmark-link-<%= index %>" data-bookmark-id="<%= bookmark.get('id') %>" data-component-type="<%= bookmark.get('block_type') %>" data-usage-id="<%= bookmark.get('usage_id') %>" aria-describedby="bookmark-type-<%= index %> bookmark-date-<%= index %>">
<div class="list-item-content">
<div class="list-item-left-section">
<h3 id="bookmark-link-<%= index %>" class="list-item-breadcrumbtrail"> <%= _.pluck(bookmark.get('path'), 'display_name').concat([bookmark.get('display_name')]).join(' <i class="icon fa fa-caret-right" aria-hidden="true"></i><span class="sr">-</span> ') %> </h3>