diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index da12dddf1f..8ad33153d2 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -9,7 +9,6 @@ If not all TODOs are marked, this PR is considered WIP (work in progress) - [ ] Have **tests** been written for the new code? If you're fixing a bug, write a regression test (or have a really good reason for not writing one... and I mean **really** good!) - [ ] Has documentation been written/updated? - [ ] New dependencies (if any) added to requirements file -- [ ] Add an entry to CHANGELOG.rst ## Reviewer guidance diff --git a/kalite/coachreports/api_views.py b/kalite/coachreports/api_views.py index ea3c69b777..2642cb584e 100644 --- a/kalite/coachreports/api_views.py +++ b/kalite/coachreports/api_views.py @@ -13,6 +13,28 @@ from kalite.shared.decorators.auth import require_admin from kalite.topic_tools.content_models import get_topic_contents, get_topic_nodes, get_leafed_topics, get_content_parents + +def unique_by_id_and_kind_sort(seq): + """ + Due to the fact that we have duplicate content items for the same content id in our topic tree, as the way that + we have implemented duplication of content across the topic tree. + :param seq: an iterator of content items. + :return: A unique, sorted list of content items. + """ + + seq.sort(key=lambda x: x.get("sort_order", 0)) + seen = {} + result = [] + for item in seq: + marker = item.get("id") + item.get("kind") + + if marker in seen: + continue + seen[marker] = 1 + result.append(item) + return result + + def get_learners_from_GET(request): learner_ids = request.GET.getlist("user_id") @@ -74,7 +96,7 @@ def learner_logs(request): end_date = request.GET.get("end_date") - topic_ids = request.GET.getlist("topic_id", []) + topic_ids = json.loads(request.GET.get("topic_ids", "[]")) learners = get_learners_from_GET(request) @@ -107,8 +129,12 @@ def learner_logs(request): output_objects.extend(objects) output_logs.extend(log_objects) + output_objects = unique_by_id_and_kind_sort(output_objects) + return JsonResponse({ + # All learner log objects for each content item. "logs": output_logs, + # All content items for which logs are being returned. "contents": output_objects, # Sometimes 'learners' gets collapsed to a list from the Queryset. This insures against that eventuality. "learners": [{ diff --git a/kalite/coachreports/static/js/coachreports/coach_reports/models.js b/kalite/coachreports/static/js/coachreports/coach_reports/models.js index 4a0e711101..6ee636992b 100644 --- a/kalite/coachreports/static/js/coachreports/coach_reports/models.js +++ b/kalite/coachreports/static/js/coachreports/coach_reports/models.js @@ -11,6 +11,11 @@ var StateModel = Backbone.Model.extend({ group_name: undefined }); } + if (key === "facility" || key.facility || key === "group" || key.group) { + this.set({ + topic_ids: undefined + }); + } Backbone.Model.prototype.set.call(this, key, val, options); } @@ -34,6 +39,7 @@ var CoachReportModel = Backbone.Model.extend({ this.group = options.group; this.start_date = options.start_date; this.end_date = options.end_date; + this.topic_ids = options.topic_ids; }, url: function() { @@ -41,7 +47,8 @@ var CoachReportModel = Backbone.Model.extend({ facility_id: this.facility, group_id: this.group, start_date: this.start_date, - end_date: this.end_date + end_date: this.end_date, + topic_ids: this.topic_ids }); } }); diff --git a/kalite/coachreports/static/js/coachreports/coach_reports/views.js b/kalite/coachreports/static/js/coachreports/coach_reports/views.js index 1f7234a77f..9e06c6ee6a 100644 --- a/kalite/coachreports/static/js/coachreports/coach_reports/views.js +++ b/kalite/coachreports/static/js/coachreports/coach_reports/views.js @@ -88,7 +88,7 @@ var CoachSummaryView = BaseView.extend({ events: { "click #show_tabular_report": "toggle_tabular_view", - "click #topic-list-submit": "set_data_model" + "click #topic-list-submit": "set_topics" }, @@ -100,7 +100,7 @@ var CoachSummaryView = BaseView.extend({ var targetElem = $("#topic-list").get(0); var frag = document.createDocumentFragment(); - var tids = $.parseJSON(this.data_model.get("topic_ids")); + var tids = $.parseJSON(this.data_model.get("topic_ids") || "[]"); var ctr = -1; parseData.forEach(function(datum, index) { @@ -196,14 +196,19 @@ var CoachSummaryView = BaseView.extend({ _.bindAll(this, "set_data_model", "render"); this.listenTo(this.model, "change:facility", this.set_data_model); this.listenTo(this.model, "change:group", this.set_data_model); + this.listenTo(this.model, "change:topic_ids", this.set_data_model); this.listenTo(this.model, "set_time", this.set_data_model); this.set_data_model(); + }, + set_topics: function() { + var topic_ids = _.map(this.$("#topic-list option:checked"), function (node) {return node.value;}); + this.model.set("topic_ids", JSON.stringify(topic_ids)); }, set_data_model: function (){ if (this.data_model) { - var check_fields = ["facility", "group", "start_date", "end_date"]; + var check_fields = ["facility", "group", "start_date", "end_date", "topic_ids"]; var data_fields = _.pick(this.data_model.attributes, check_fields); var status_fields = _.pick(this.model.attributes, check_fields); if (!_.isEqual(data_fields, status_fields)) { @@ -212,13 +217,12 @@ var CoachSummaryView = BaseView.extend({ } if (!this.data_model) { - var topic_ids = _.map(this.$("#topic-list option:checked"), function (node) {return node.value;}); this.data_model = new Models.CoachReportAggregateModel({ facility: this.model.get("facility"), group: this.model.get("group"), start_date: date_string(this.model.get("start_date")), end_date: date_string(this.model.get("end_date")), - topic_ids: JSON.stringify(topic_ids) + topic_ids: this.model.get("topic_ids") }); if (this.model.get("facility")) { this.listenTo(this.data_model, "sync", this.render); @@ -242,14 +246,16 @@ var CoachSummaryView = BaseView.extend({ var w = 500; var dataset = [struggling/total, complete/total, in_progress/total]; - console.log(dataset); + var svg = d3.select("div.progressbar").append("svg").attr("width", w).attr("height", h). attr("class", "col-md-8 innerbar"); svg.selectAll("rect").data(dataset).enter().append("rect").attr("x", function(d, i){ - return _.reduce(dataset.slice(0, i), function(memo, num) { return memo + num; }, 0) * w; + var out = _.reduce(dataset.slice(0, i), function(memo, num) { return memo + num; }, 0) * w; + out = isNaN(out) ? 0 : out; + return out; }).attr("y", 0).attr("width", function(d) { - return d * w; + return isNaN(d * w) ? 0 : d*w; }).attr("height", h).attr("class", "rect").attr("class", function(d, i){ switch(i) { case(0): @@ -272,7 +278,9 @@ var CoachSummaryView = BaseView.extend({ return in_progress; } }).attr("fill", "black").attr("x", function(d, i){ - return (_.reduce(dataset.slice(0, i), function(memo, num) { return memo + num; }, 0) + d/2) * w; + var out = (_.reduce(dataset.slice(0, i), function(memo, num) { return memo + num; }, 0) + d/2) * w; + out = isNaN(out) ? 0 : out; + return out; }).attr("y", h/2).attr("font-size", "12px").style("text-anchor", "middle"); }, diff --git a/kalite/coachreports/static/js/coachreports/tabular_reports/views.js b/kalite/coachreports/static/js/coachreports/tabular_reports/views.js index cf2c7f24f0..3af16ca85a 100644 --- a/kalite/coachreports/static/js/coachreports/tabular_reports/views.js +++ b/kalite/coachreports/static/js/coachreports/tabular_reports/views.js @@ -375,7 +375,8 @@ var TabularReportView = BaseView.extend({ facility: this.model.get("facility"), group: this.model.get("group"), start_date: date_string(this.model.get("start_date")), - end_date: date_string(this.model.get("end_date")) + end_date: date_string(this.model.get("end_date")), + topic_ids: this.model.get("topic_ids") }); if (this.model.get("facility")) { this.data_model.fetch().then(function() { diff --git a/kalite/coachreports/tests/coachreport_tests.py b/kalite/coachreports/tests/coachreport_tests.py index 56c392a6d2..8d55f91417 100644 --- a/kalite/coachreports/tests/coachreport_tests.py +++ b/kalite/coachreports/tests/coachreport_tests.py @@ -10,7 +10,7 @@ from kalite.testing.mixins.securesync_mixins import CreateZoneMixin from kalite.testing.mixins.facility_mixins import FacilityMixins from kalite.testing.mixins.student_progress_mixins import StudentProgressMixin -from kalite.topic_tools.content_models import get_content_parents, Item ,set_database +from kalite.topic_tools.content_models import get_content_parents, Item, set_database, create class ExternalAPITests(FacilityMixins, @@ -78,6 +78,8 @@ def setUp(self): self.group = self.create_group(name='group1', facility=self.facility) self.empty_group = self.create_group(name='empty_group', facility=self.facility) + self.facility_user = self.create_student() + def test_learner_log_endpoint(self): response_keys = ["logs","contents","learners","page","pages","limit"] self.client.login(username='admin', password='admin') @@ -139,3 +141,114 @@ def test_playlist_progress(self): self.assertEqual(len(exercises),1) #checking if the returned list is accurate self.assertEqual(exercises[0]["title"],"Comparing two-digit numbers") + + +class LearnerLogAPITests(FacilityMixins, + StudentProgressMixin, + CreateZoneMixin, + CreateAdminMixin, + KALiteTestCase): + """ + These tests test the learner log API endpoint in the coachreports app. + """ + + def setUp(self): + super(LearnerLogAPITests, self).setUp() + + self.admin_data = {"username": "admin", "password": "admin"} + self.admin = self.create_admin(**self.admin_data) + + self.zone = self.create_zone() + self.device_zone = self.create_device_zone(self.zone) + self.facility = self.create_facility() + + self.group = self.create_group(name='group1', facility=self.facility) + self.empty_group = self.create_group(name='empty_group', facility=self.facility) + + self.facility_user = self.create_student() + + self.topic1 = create({ + "id": "stuff_here", + "slug": "stuff_here", + "path": "qwrqweqweqwe", + "kind": "Topic", + "title": "", + "description": "", + "available": True, + }) + self.topic2 = create({ + "id": "stuff_there", + "slug": "stuff_there", + "path": "qwrqweqweqw", + "kind": "Topic", + "title": "", + "description": "", + "available": True, + }) + + self.exercise1 = create({ + "id": "stuff_here_ex", + "slug": "stuff_here_ex", + "path": "qwrqweqweqwe/qwrqweqweq", + "kind": "Exercise", + "title": "", + "description": "", + "available": True, + "parent": self.topic1, + }) + self.exercise2 = create({ + "id": "stuff_there_ex", + "slug": "stuff_there_ex", + "path": "qwrqweqweqw/qwrqweqw", + "kind": "Exercise", + "title": "", + "description": "", + "available": True, + "parent": self.topic2, + }) + + self.create_exercise_log(user=self.facility_user, exercise_id=self.exercise1.id) + self.create_exercise_log(user=self.facility_user, exercise_id=self.exercise2.id) + + def tearDown(self): + self.exercise1.delete() + self.exercise2.delete() + self.topic1.delete() + self.topic2.delete() + + def test_learner_log_topic_filters(self): + + self.client.login(username='admin', password='admin') + api_resp_1 = json.loads(self.client.get("%s?facility_id=%s" % (self.reverse("learner_logs"), self.facility.id)).content) + api_resp_2 = json.loads(self.client.get("%s?facility_id=%s&topic_ids=%s" % (self.reverse("learner_logs"), self.facility.id, json.dumps([self.topic1.id]))).content) + assert len(api_resp_2["contents"]) < len(api_resp_1["contents"]) + + def test_learner_log_topic_filters_contents_length(self): + + self.client.login(username='admin', password='admin') + api_resp_2 = json.loads(self.client.get("%s?facility_id=%s&topic_ids=%s" % (self.reverse("learner_logs"), self.facility.id, json.dumps([self.topic1.id]))).content) + assert len(api_resp_2["contents"]) == 1 + + def test_learner_log_topic_filters_contents_id(self): + + self.client.login(username='admin', password='admin') + api_resp_2 = json.loads(self.client.get("%s?facility_id=%s&topic_ids=%s" % (self.reverse("learner_logs"), self.facility.id, json.dumps([self.topic1.id]))).content) + assert api_resp_2["contents"][0]["id"] == self.exercise1.id + + def test_learner_log_contents(self): + + self.client.login(username='admin', password='admin') + api_resp_1 = json.loads(self.client.get("%s?facility_id=%s" % (self.reverse("learner_logs"), self.facility.id)).content) + assert len(api_resp_1["contents"]) == 2 + + def test_learner_log_logs(self): + + self.client.login(username='admin', password='admin') + api_resp_1 = json.loads(self.client.get("%s?facility_id=%s" % (self.reverse("learner_logs"), self.facility.id)).content) + assert len(api_resp_1["logs"]) == 2 + + def test_learner_log_log_contents(self): + + self.client.login(username='admin', password='admin') + api_resp_2 = json.loads(self.client.get("%s?facility_id=%s&topic_ids=%s" % (self.reverse("learner_logs"), self.facility.id, json.dumps([self.topic1.id]))).content) + assert api_resp_2["logs"][0]["exercise_id"] == self.exercise1.id diff --git a/kalite/distributed/static/css/distributed/content_recommendation.less b/kalite/distributed/static/css/distributed/content_recommendation.less index 0d8c510f1c..979ecbfc95 100644 --- a/kalite/distributed/static/css/distributed/content_recommendation.less +++ b/kalite/distributed/static/css/distributed/content_recommendation.less @@ -1,4 +1,10 @@ .content-nextsteps-topic-link div { background-color: #5aa685; color: white; -} \ No newline at end of file +} + +i.content-rec-icon { + font-size:4em; + top: 0.2em; + position: relative; +} diff --git a/kalite/distributed/static/js/distributed/contentrec/hbtemplates/content-explore-topic.handlebars b/kalite/distributed/static/js/distributed/contentrec/hbtemplates/content-explore-topic.handlebars index bc2334894c..92f134fc33 100644 --- a/kalite/distributed/static/js/distributed/contentrec/hbtemplates/content-explore-topic.handlebars +++ b/kalite/distributed/static/js/distributed/contentrec/hbtemplates/content-explore-topic.handlebars @@ -7,11 +7,11 @@
-
+

{{ suggested_topic.title }}:

{{#if suggested_topic.description }} -
+

{{_ "Description" }}:

{{ suggested_topic.description }} diff --git a/kalite/distributed/static/js/distributed/contentrec/hbtemplates/content-nextsteps-lesson.handlebars b/kalite/distributed/static/js/distributed/contentrec/hbtemplates/content-nextsteps-lesson.handlebars index e0451e9fc8..00acf5c7d0 100644 --- a/kalite/distributed/static/js/distributed/contentrec/hbtemplates/content-nextsteps-lesson.handlebars +++ b/kalite/distributed/static/js/distributed/contentrec/hbtemplates/content-nextsteps-lesson.handlebars @@ -12,7 +12,7 @@ alt="{{ truncate description 150 }}" {{/if}}> {{else}} - + {{/if}}
diff --git a/kalite/distributed/static/js/distributed/contentrec/hbtemplates/content-resume.handlebars b/kalite/distributed/static/js/distributed/contentrec/hbtemplates/content-resume.handlebars index 8e2c0fa038..6741c37c46 100644 --- a/kalite/distributed/static/js/distributed/contentrec/hbtemplates/content-resume.handlebars +++ b/kalite/distributed/static/js/distributed/contentrec/hbtemplates/content-resume.handlebars @@ -49,7 +49,7 @@
- +
diff --git a/kalite/distributed/static/js/distributed/exercises/models.js b/kalite/distributed/static/js/distributed/exercises/models.js index dd4fc217a1..bbe55e86ef 100644 --- a/kalite/distributed/static/js/distributed/exercises/models.js +++ b/kalite/distributed/static/js/distributed/exercises/models.js @@ -65,7 +65,7 @@ var ExerciseDataModel = ContentModels.ContentDataModel.extend({ "secondsPerFastProblem": this.get("seconds_per_fast_problem"), "authorName": this.get("author_name"), "relatedVideos": this.get("related_videos"), - "fileName": this.get("template") + "fileName": this.get("file_name") }, "exerciseProgress": { "level": "" // needed to keep khan-exercises from blowing up diff --git a/kalite/distributed/static/js/distributed/topics/views.js b/kalite/distributed/static/js/distributed/topics/views.js index 1cf6da5485..1d1bdde1cd 100644 --- a/kalite/distributed/static/js/distributed/topics/views.js +++ b/kalite/distributed/static/js/distributed/topics/views.js @@ -236,58 +236,61 @@ var SidebarView = BaseView.extend({ } }, - resize_for_narrow: _.debounce(function() { - var current_level = this.state_model.get("current_level"); - var column_width = this.$(".topic-container-inner").width(); - var last_column_width = this.$(".topic-container-inner:last-child").width(); - // Hack to give the last child of .topic-container-inner to be 1.5 times the - // width of their parents. Also, sidebar overflow out of the left side of screen - // is computed and set here. + resize_for_narrow: _.throttle(function() { + if (this.state_model.get("open")) { + var current_level = this.state_model.get("current_level"); + var column_width = this.$(".topic-container-inner").width(); + var last_column_width = this.$(".topic-container-inner:last-child").width(); + // Hack to give the last child of .topic-container-inner to be 1.5 times the + // width of their parents. Also, sidebar overflow out of the left side of screen + // is computed and set here. - // THE magic variable that controls number of visible panels - var numOfPanelsToShow = 4; + // THE magic variable that controls number of visible panels + var numOfPanelsToShow = 4; - if ($(window).width() < 1120) - numOfPanelsToShow = 3; + if ($(window).width() < 1120) + numOfPanelsToShow = 3; - if ($(window).width() < 920) - numOfPanelsToShow = 2; + if ($(window).width() < 920) + numOfPanelsToShow = 2; - if ($(window).width() < 620) + if ($(window).width() < 620) numOfPanelsToShow = 1; + // Used to get left value in number form + var sidebarPanelPosition = this.sidebar.position(); + var sidebarPanelLeft = sidebarPanelPosition.left; - // Used to get left value in number form - var sidebarPanelPosition = this.sidebar.position(); - var sidebarPanelLeft = sidebarPanelPosition.left; + this.width = (current_level - 1) * column_width + last_column_width + 10; + this.sidebar.width(this.width); + var sidebarPanelNewLeft = -(column_width * (current_level - numOfPanelsToShow)) + this.sidebarBack.width(); + if (sidebarPanelNewLeft > 0) sidebarPanelNewLeft = 0; - this.width = (current_level - 1) * column_width + last_column_width + 10; - this.sidebar.width(this.width); - var sidebarPanelNewLeft = -(column_width * (current_level - numOfPanelsToShow)) + this.sidebarBack.width(); - if (sidebarPanelNewLeft > 0) sidebarPanelNewLeft = 0; + // Signature color flash (also hides a slight UI glitch) + var originalBackColor = this.sidebarBack.css('background-color'); + this.sidebarBack.css('background-color', this.sidebarTab.css('background-color')).animate({'background-color': originalBackColor}); - // Signature color flash (also hides a slight UI glitch) - var originalBackColor = this.sidebarBack.css('background-color'); - this.sidebarBack.css('background-color', this.sidebarTab.css('background-color')).animate({'background-color': originalBackColor}); - - var self = this; - this.sidebar.animate({"left": sidebarPanelNewLeft}, 115, function() { - self.set_sidebar_back(); - }); + var self = this; + this.sidebar.animate({"left": sidebarPanelNewLeft}, 115, function () { + self.set_sidebar_back(); + }); - this.sidebarTab.animate({left: this.sidebar.width() + sidebarPanelNewLeft}, 115); + this.sidebarTab.animate({left: this.sidebar.width() + sidebarPanelNewLeft}, 115); + } }, 100), // Pretty much the code for pre-back-button sidebar resize - resize_for_wide: _.debounce(function() { + resize_for_wide: _.throttle(function() { var current_level = this.state_model.get("current_level"); var column_width = this.$(".topic-container-inner").width(); var last_column_width = 400; this.width = (current_level-1) * column_width + last_column_width + 10; this.sidebar.width(this.width); - this.sidebar.css({left: 0}); - this.sidebarTab.css({left: this.width}); - + if (this.state_model.get("open")) { + this.sidebar.css({left: 0}); + this.sidebarTab.css({left: this.width}); + } + this.set_sidebar_back(); }, 100), @@ -322,8 +325,6 @@ var SidebarView = BaseView.extend({ this.sidebarTab.css({left: 0}).html(''); this.$(".sidebar-fade").hide(); } - - this.set_sidebar_back(); }, 100), set_sidebar_back: function() { diff --git a/kalite/distributed/static/js/distributed/utils/api.js b/kalite/distributed/static/js/distributed/utils/api.js index b17ece9a81..6e5646f8ff 100644 --- a/kalite/distributed/static/js/distributed/utils/api.js +++ b/kalite/distributed/static/js/distributed/utils/api.js @@ -57,7 +57,7 @@ function handleFailedAPI(resp, error_prefix) { var messages = {}; switch (resp.status) { case 0: - messages = {error: gettext("Could not connect to the server.") + " " + gettext("Please try again later.")}; + messages = {error: gettext("Connecting to the server.") + " " + gettext("Please wait...")}; break; case 401: diff --git a/kalite/distributed/templates/distributed/base.html b/kalite/distributed/templates/distributed/base.html index dfcc053b72..5bbabd320f 100644 --- a/kalite/distributed/templates/distributed/base.html +++ b/kalite/distributed/templates/distributed/base.html @@ -60,7 +60,7 @@ STATIC_URL : "{% static "" %}", - ALL_ASSESSMENT_ITEMS_URL : "{% url 'assessment_item' assessment_item='' %}", + ALL_ASSESSMENT_ITEMS_URL : "{% url 'assessment_item' assessment_item_id='' %}", GET_VIDEO_LOGS_URL : "{% url 'api_dispatch_list' resource_name='videolog' %}", GET_EXERCISE_LOGS_URL : "{% url 'api_dispatch_list' resource_name='exerciselog' %}", GET_CONTENT_LOGS_URL : "{% url 'api_dispatch_list' resource_name='contentlog' %}", diff --git a/kalite/facility/api_resources.py b/kalite/facility/api_resources.py index c22cdb6a16..87c0ae2353 100644 --- a/kalite/facility/api_resources.py +++ b/kalite/facility/api_resources.py @@ -1,7 +1,6 @@ import datetime -import os from dateutil.tz import tzlocal - +from django.db.models.signals import post_save from tastypie import fields from tastypie.http import HttpUnauthorized from tastypie.resources import ModelResource, ALL_WITH_RELATIONS @@ -40,6 +39,27 @@ class Meta: resource_name = 'group' authorization = TeacherOrAdminCanReadWrite() +FACILITY_LIST = None + +def facility_list(): + global FACILITY_LIST + + if FACILITY_LIST is None: + if settings.CENTRAL_SERVER: + FACILITY_LIST = [] + else: + # To enable login, list the id and names of all facilities. + # This keeps it cached so that the status api call can return this to the client side + # without significantly increasing DB load on every status call. + FACILITY_LIST = [{"id": id, "name": name} for id, name in Facility.objects.values_list("id", "name")] + + return FACILITY_LIST + +def flag_facility_cache(**kwargs): + global FACILITY_LIST + FACILITY_LIST = None + +post_save.connect(flag_facility_cache, sender=Facility) class FacilityUserResource(ModelResource): facility = fields.ForeignKey(FacilityResource, 'facility') @@ -188,7 +208,7 @@ def generate_status(self, request, **kwargs): "messages": message_dicts, "status_timestamp": datetime.datetime.now(tzlocal()), "version": version.VERSION, - "facilities": request.session.get("facilities"), + "facilities": facility_list(), "simplified_login": settings.SIMPLIFIED_LOGIN, "docs_exist": getattr(settings, "DOCS_EXIST", False), "zone_id": getattr(Device.get_own_device().get_zone(), "id", "None"), diff --git a/kalite/facility/forms.py b/kalite/facility/forms.py index f684d23533..d0377403e2 100644 --- a/kalite/facility/forms.py +++ b/kalite/facility/forms.py @@ -165,14 +165,17 @@ def __init__(self, facility, *args, **kwargs): self.fields["facility"].initial = facility self.fields["facility"].queryset = Facility.objects.by_zone(facility.get_zone()) + if self.fields["facility"].queryset.count() < 2: + self.fields["facility"].widget = forms.HiddenInput() + class Meta: model = FacilityGroup fields = ("name", "description", "facility", "zone_fallback", ) widgets = { - "facility": forms.HiddenInput(), "zone_fallback": forms.HiddenInput(), # TODO(jamalex): this shouldn't be in here } + def clean_name(self): name = self.cleaned_data.get("name", "") ungrouped = re.compile("[uU]+ngrouped") diff --git a/kalite/facility/middleware.py b/kalite/facility/middleware.py index 21832a430f..c8f73bc07f 100644 --- a/kalite/facility/middleware.py +++ b/kalite/facility/middleware.py @@ -15,10 +15,6 @@ def refresh_session_facility_info(request, facility_count): request.session["facility_count"] = facility_count request.session["facility_exists"] = request.session["facility_count"] > 0 - # To enable simplified login, also list the id and names of all facilities in the session object. - # This keeps it cached so that the status api call can return this to the client side - # without significantly increasing DB load on every status call. - request.session["facilities"] = [{"id": id, "name": name} for id, name in Facility.objects.values_list("id", "name")] def flag_facility_cache(**kwargs): global FACILITY_CACHE_STALE diff --git a/kalite/facility/templates/facility/facility_group.html b/kalite/facility/templates/facility/facility_group.html index ac1ba6c68f..9cf8a99c29 100644 --- a/kalite/facility/templates/facility/facility_group.html +++ b/kalite/facility/templates/facility/facility_group.html @@ -1,29 +1,12 @@ {% extends "distributed/base_manage.html" %} {% load i18n %} -{% block headcss %}{{ block.super }} - -{% endblock headcss %} - {% block headjs %}{{ block.super }} {% endblock headjs %} diff --git a/kalite/facility/views.py b/kalite/facility/views.py index e9ef1ae1f9..20478e0009 100644 --- a/kalite/facility/views.py +++ b/kalite/facility/views.py @@ -184,7 +184,6 @@ def group_edit(request, facility, group_id): "form": form, "group_id": group_id, "facility": facility, - "singlefacility": request.session["facility_count"] == 1, "title": _("Add a new group") if group_id == 'new' else _("Edit group"), } diff --git a/kalite/i18n/management/commands/makemessages.py b/kalite/i18n/management/commands/makemessages.py index 59619b93fc..28f692a4cf 100644 --- a/kalite/i18n/management/commands/makemessages.py +++ b/kalite/i18n/management/commands/makemessages.py @@ -11,16 +11,27 @@ from django.core.management.commands import makemessages +IGNORE_PATTERNS = [ + "*/node_modules*", + "*dist-packages*", + "*python-packages*", + "*/kalite/static*", + "*bundle*.js", + "*/ka-lite/build/*", + "*/js/i18n/*.js", + "*/ka-lite/docs/*" +] + + class Command(makemessages.Command): def handle_noargs(self, *args, **options): # Set some sensible defaults - ignore_pattern = "node_modules*" - if ignore_pattern not in options["ignore_patterns"]: - options["ignore_patterns"].append(ignore_pattern) + for ignore_pattern in IGNORE_PATTERNS: + if ignore_pattern not in options["ignore_patterns"]: + options["ignore_patterns"].append(ignore_pattern) + print("Ignoring the following patterns: {}".format(options["ignore_patterns"])) - print("Added \"{patt}\" to --ignore patterns. Ignoring: {all}".format(patt=ignore_pattern, - all=options["ignore_patterns"])) print("Calling base makemessages command.") super(Command, self).handle_noargs(*args, **options) diff --git a/kalite/main/api_urls.py b/kalite/main/api_urls.py index 5635ae6f4e..ceab72aae6 100755 --- a/kalite/main/api_urls.py +++ b/kalite/main/api_urls.py @@ -18,7 +18,7 @@ url(r'^', include(ContentLogResource().urls)), url(r'^', include(ContentRatingResource().urls)), - url(r'^assessment_item/(?P.*)/$', 'assessment_item', {}, 'assessment_item'), + url(r'^assessment_item/(?P.*)/$', 'assessment_item', {}, 'assessment_item'), url(r'^content_recommender/?$', 'content_recommender', {}, 'content_recommender'), # A flat data structure for building a graphical knowledge map diff --git a/kalite/main/api_views.py b/kalite/main/api_views.py index 1d1aae30b1..4c81143b67 100755 --- a/kalite/main/api_views.py +++ b/kalite/main/api_views.py @@ -34,6 +34,9 @@ def search_api(request, channel): matches, exact, pages = search_topic_nodes(query=query, channel=channel, language=request.language, page=1, items_per_page=15, exact=False) + if not matches: + messages.warning(request, _("Search completed, no content was found for your search. Try something else.")) + return JsonResponse(matches) @@ -41,11 +44,6 @@ def search_api(request, channel): def content_item(request, channel, content_id): language = request.language - # Hardcode the Brazilian Portuguese mapping that only the central server knows about - # TODO(jamalex): BURN IT ALL DOWN! - if language == "pt-BR": - language = "pt" - content = get_content_item(channel=channel, content_id=content_id, language=language) if not content: @@ -107,8 +105,8 @@ def set_bool_flag(flag_name, rec_dict): @api_handle_error_with_json -def assessment_item(request, assessment_item): - assessment_item = get_assessment_item_data(channel=getattr(request, "channel", "khan"), - language=getattr(request, "language", "en"), - assessment_item_id=assessment_item) - return JsonResponse(assessment_item) +def assessment_item(request, assessment_item_id): + assessment_item_dict = get_assessment_item_data(channel=getattr(request, "channel", "khan"), + language=getattr(request, "language", "en"), + assessment_item_id=assessment_item_id) + return JsonResponse(assessment_item_dict) diff --git a/kalite/main/tests/api_tests.py b/kalite/main/tests/api_tests.py index 70c89532e1..f486f52cb6 100755 --- a/kalite/main/tests/api_tests.py +++ b/kalite/main/tests/api_tests.py @@ -1,9 +1,50 @@ """ """ +import json +from django.core.urlresolvers import reverse + from ..models import VideoLog, ExerciseLog from kalite.facility.models import Facility, FacilityUser -from kalite.testing.base import KALiteTestCase +from kalite.testing.base import KALiteTestCase, KALiteClientTestCase from kalite.testing.client import KALiteClient +from kalite.topic_tools.content_models import set_database, Using, Item, get_or_create + + +class ContentItemApiViewTestCase(KALiteClientTestCase): + def setUp(self, db=None): + item_attributes = { + u"title": u"My Cool Item", + u"description": u"A description!", + u"available": True, + u"kind": u"Video", + u"id": u"my_content", + u"slug": u"my_content", + u"path": u"khan/my_content", + u"extra_fields": u"{}", + u"size_on_disk": 0, + u"sort_order": 0.0, + u"remote_size": 0, + u"files_complete": 0, + u"total_files": 0, + u"youtube_id": u"", + } + self.expected = item_attributes.copy() + self.expected.update({ + u"messages": [], # Added to all responses + u"parent": {}, # Since the item's parent is not set, it gets translated to an empty dict in the response + }) + get_or_create(item_attributes) + self.maxDiff = None + + def test_sanity(self): + resp = self.client.get(reverse('content_item', kwargs={ + 'channel': 'khan', + 'content_id': 'my_content', + })) + actual = json.loads(resp.content) + actual.pop(u"pk") # We have no control over this value, so don't check it + self.assertDictEqual(actual, self.expected) + class TestSaveExerciseLog(KALiteTestCase): @@ -122,7 +163,6 @@ def test_update_exerciselog(self): self.assertEqual(exerciselog.attempts, self.NEW_ATTEMPTS + 1, "The ExerciseLog did not have the correct number of attempts.") - class TestSaveVideoLog(KALiteTestCase): ORIGINAL_POINTS = 84 diff --git a/kalite/testing/mixins/student_progress_mixins.py b/kalite/testing/mixins/student_progress_mixins.py index b1cd8672b1..63bc28048b 100644 --- a/kalite/testing/mixins/student_progress_mixins.py +++ b/kalite/testing/mixins/student_progress_mixins.py @@ -45,7 +45,7 @@ class CreateExerciseLogMixin(object): @classmethod def create_exercise_log(cls, **kwargs): fields = CreateExerciseLogMixin.DEFAULTS.copy() - fields['user'] = kwargs.get("user") + fields.update(kwargs) return ExerciseLog.objects.create(**fields) diff --git a/kalite/topic_tools/content_models.py b/kalite/topic_tools/content_models.py index d760f918a2..a367ffa78b 100644 --- a/kalite/topic_tools/content_models.py +++ b/kalite/topic_tools/content_models.py @@ -104,13 +104,8 @@ def set_database(function): """ def wrapper(*args, **kwargs): - # Hardcode the Brazilian Portuguese mapping that only the central server knows about - # TODO(jamalex): BURN IT ALL DOWN! language = kwargs.get("language", "en") - if language == "pt-BR": - language = "pt" - path = kwargs.pop("database_path", None) if not path: path = CONTENT_DATABASE_PATH.format( @@ -286,6 +281,7 @@ def get_topic_update_nodes(parent=None, **kwargs): Item.total_files, Item.id, Item.path, + Item.youtube_id, ).join(Parent, on=(Item.parent == Parent.pk)).where((selector) & (Item.total_files != 0)) return values @@ -509,15 +505,28 @@ def bulk_insert(items, **kwargs): Item.insert_many(items[idx:idx + 500]).execute() +@set_database +def create(item, **kwargs): + """ + Wrapper around create that allows us to specify a database + and also parse the model data to compress extra fields. + :param item: A dictionary containing content metadata for one node. + :return Item + """ + if item: + return Item.create(**parse_model_data(item)) + + @set_database def get_or_create(item, **kwargs): """ Wrapper around get or create that allows us to specify a database and also parse the model data to compress extra fields. :param item: A dictionary containing content metadata for one node. + :return tuple of Item and Boolean for whether created or not. """ if item: - Item.create_or_get(**parse_model_data(item)) + return Item.create_or_get(**parse_model_data(item)) @set_database diff --git a/kalite/topic_tools/content_recommendation.py b/kalite/topic_tools/content_recommendation.py index 5caf9f215c..0c5c721455 100644 --- a/kalite/topic_tools/content_recommendation.py +++ b/kalite/topic_tools/content_recommendation.py @@ -36,7 +36,8 @@ def get_resume_recommendations(user, request): final = get_most_recent_incomplete_item(user) if final: - return [get_content_item(language=request.language, channel=getattr(final, "channel", "khan"), content_id=final.get("id"))] + content = get_content_item(language=request.language, channel=getattr(final, "channel", "khan"), content_id=final.get("id")) + return [content] if content else [] else: return [] @@ -100,8 +101,9 @@ def filter_complete(ex): if exercise_id in exercise_parents_table: subtopic_id = exercise_parents_table[exercise_id]['subtopic_id'] exercise = get_content_item(language=request.language, content_id=exercise_id) - exercise["topic"] = get_content_item(language=request.language, content_id=subtopic_id, topic=True) - final.append(exercise) + if exercise: + exercise["topic"] = get_content_item(language=request.language, content_id=subtopic_id, topic=True) or {} + final.append(exercise) #final recommendations are a combination of struggling, group filtering, and topic_tree filtering @@ -208,8 +210,8 @@ def get_explore_recommendations(user, request): if recommended_topic: final.append({ - 'suggested_topic': get_content_item(language=request.language, content_id=recommended_topic, topic=True), - 'interest_topic': get_content_item(language=request.language, content_id=subtopic_id, topic=True), + 'suggested_topic': get_content_item(language=request.language, content_id=recommended_topic, topic=True) or {}, + 'interest_topic': get_content_item(language=request.language, content_id=subtopic_id, topic=True) or {}, }) added.append(recommended_topic) @@ -424,12 +426,12 @@ def get_neighbors_at_dist_1(topic_index, subtopic_index, topic): next = subtopic_index + 1 #if there is a previous topic (neighbor to left) - if(prev > -1 ): + if (prev > -1 ) and prev < len(topic['children']): neighbors.append(topic['children'][prev] + ' 1') # neighbor on the left side #else check if there is a neighboring topic (left) else: - if (topic_index-1) > -1: + if (topic_index-1) > -1 and topic_index - 1 < len(tree) and len(tree[(topic_index-1)]['children']) > 1: neighbor_length = len(tree[(topic_index-1)]['children']) neighbors.append(tree[(topic_index-1)]['children'][(neighbor_length-1)] + ' 4') @@ -442,7 +444,7 @@ def get_neighbors_at_dist_1(topic_index, subtopic_index, topic): #else check if there is a neighboring topic (right) else: - if (topic_index + 1) < len(tree): + if (topic_index + 1) < len(tree) and tree[(topic_index+1)]['children']: #the 4 denotes the # of nodes in path to this other node, will always be 4 neighbors.append(tree[(topic_index+1)]['children'][0] + ' 4') @@ -505,14 +507,17 @@ def get_subsequent_neighbors(nearest_neighbors, data, curr): at_four_left = True else: - #if immediate left node is 4 - if data[ curr ]['related_subtopics'][1].split(' ')[1] == '4': - at_four_left = True - new_dist = 4 - elif data[left]['related_subtopics'][1].split(' ')[1] == '4': #if the next left neighbor is at dist 4 - at_four_left = True - new_dist = 4 - else: #this means that the next left node is at dist 1 + try: + #if immediate left node is 4 + if data[ curr ]['related_subtopics'][1].split(' ')[1] == '4': + at_four_left = True + new_dist = 4 + elif data[left]['related_subtopics'][1].split(' ')[1] == '4': #if the next left neighbor is at dist 4 + at_four_left = True + new_dist = 4 + else: #this means that the next left node is at dist 1 + new_dist = 1 + except IndexError: new_dist = 1 other_neighbors.append(data[left]['related_subtopics'][1].split(' ')[0] + ' ' + str(new_dist)) diff --git a/kalite/topic_tools/tests/resume_tests.py b/kalite/topic_tools/tests/resume_tests.py index 62118bf1da..fd5c90fec5 100644 --- a/kalite/topic_tools/tests/resume_tests.py +++ b/kalite/topic_tools/tests/resume_tests.py @@ -3,76 +3,104 @@ get the "Resume" recommendations. ''' import datetime - -from kalite.topic_tools.content_recommendation import get_most_recent_incomplete_item +from kalite.topic_tools.content_recommendation import get_most_recent_incomplete_item, get_resume_recommendations from kalite.testing.base import KALiteTestCase from kalite.facility.models import Facility, FacilityUser from kalite.main.models import ExerciseLog + class TestResumeMethods(KALiteTestCase): + ORIGINAL_POINTS = 37 + ORIGINAL_ATTEMPTS = 3 + ORIGINAL_STREAK_PROGRESS = 20 + NEW_POINTS_LARGER = 22 + NEW_ATTEMPTS = 5 + NEW_STREAK_PROGRESS_LARGER = 10 + NEW_POINTS_SMALLER = 0 + NEW_STREAK_PROGRESS_SMALLER = 0 + EXERCISE_ID = "number_line" + EXERCISE_ID2 = "radius_diameter_and_circumference" + INVALID_EXERCISE_ID = "s_diameter_and_circumference" + USERNAME1 = "test_user_resume1" + USERNAME2 = "test_user_resume2" + USERNAME3 = "test_user_resume3" + PASSWORD = "dummies" + FACILITY = "Test Facility Resume" + TIMESTAMP_LATER = datetime.datetime(2014, 11, 17, 20, 51, 2, 342662) + TIMESTAMP_EARLY = datetime.datetime(2014, 10, 8, 15, 59, 59, 370290) + + def setUp(self): + """Performed before every test""" + + # a brand new user + self.facility = Facility(name=self.FACILITY) + self.facility.save() + + self.user_with_no_activity = FacilityUser(username=self.USERNAME1, facility=self.facility) + self.user_with_no_activity.set_password(self.PASSWORD) + self.user_with_no_activity.save() + + # a user with valid exercises + self.user_with_activity = FacilityUser(username=self.USERNAME2, facility=self.facility) + self.user_with_activity.set_password(self.PASSWORD) + self.user_with_activity.save() + + # a user with invalid exercises + self.user_with_old_activity = FacilityUser(username=self.USERNAME3, facility=self.facility) + self.user_with_old_activity.set_password(self.PASSWORD) + self.user_with_old_activity.save() + + # add some exercises for second user (both incomplete) + self.original_exerciselog2 = ExerciseLog(exercise_id=self.EXERCISE_ID, user=self.user_with_activity, + complete=False) + self.original_exerciselog2.points = self.ORIGINAL_POINTS + self.original_exerciselog2.attempts = self.ORIGINAL_POINTS + self.original_exerciselog2.streak_progress = self.ORIGINAL_STREAK_PROGRESS + self.original_exerciselog2.latest_activity_timestamp = self.TIMESTAMP_EARLY + self.original_exerciselog2.completion_timestamp = self.TIMESTAMP_EARLY + self.original_exerciselog2.save() + + self.original_exerciselog2 = ExerciseLog(exercise_id=self.EXERCISE_ID2, user=self.user_with_activity, + complete=False) + self.original_exerciselog2.points = self.ORIGINAL_POINTS + self.original_exerciselog2.attempts = self.ORIGINAL_POINTS + self.original_exerciselog2.streak_progress = self.ORIGINAL_STREAK_PROGRESS + self.original_exerciselog2.latest_activity_timestamp = self.TIMESTAMP_LATER + self.original_exerciselog2.completion_timestamp = self.TIMESTAMP_LATER + self.original_exerciselog2.save() + + self.original_exerciselog3 = ExerciseLog(exercise_id=self.INVALID_EXERCISE_ID, user=self.user_with_old_activity, + complete=False) + self.original_exerciselog3.points = self.ORIGINAL_POINTS + self.original_exerciselog3.attempts = self.ORIGINAL_POINTS + self.original_exerciselog3.streak_progress = self.ORIGINAL_STREAK_PROGRESS + self.original_exerciselog3.latest_activity_timestamp = self.TIMESTAMP_LATER + self.original_exerciselog3.completion_timestamp = self.TIMESTAMP_LATER + self.original_exerciselog3.save() + + def test_get_most_recent_incomplete_item(self): + '''get_most_recent_incomplete_item()''' + + # test user with activity first + expected = { + "id": unicode(self.EXERCISE_ID2, 'utf-8'), + "timestamp": self.TIMESTAMP_LATER, + "kind": "Exercise" + } + actual = get_most_recent_incomplete_item(self.user_with_activity) + self.assertEqual(expected, actual) + + # new user just created (no activity logged) + self.assertIsNone(get_most_recent_incomplete_item(user=self.user_with_no_activity)) + + def test_get_empty_list_invalid_resume(self): + # Used to mock a request object that is only queried for its 'lang' property. + class MicroMock(object): + def __init__(self, **kwargs): + self.__dict__.update(kwargs) + + # test user with invalid activity + actual = get_resume_recommendations(self.user_with_old_activity, MicroMock(language="en")) - ORIGINAL_POINTS = 37 - ORIGINAL_ATTEMPTS = 3 - ORIGINAL_STREAK_PROGRESS = 20 - NEW_POINTS_LARGER = 22 - NEW_ATTEMPTS = 5 - NEW_STREAK_PROGRESS_LARGER = 10 - NEW_POINTS_SMALLER = 0 - NEW_STREAK_PROGRESS_SMALLER = 0 - EXERCISE_ID = "number_line" - EXERCISE_ID2 = "radius_diameter_and_circumference" - USERNAME1 = "test_user_resume1" - USERNAME2 = "test_user_resume2" - PASSWORD = "dummies" - FACILITY = "Test Facility Resume" - TIMESTAMP_LATER = datetime.datetime(2014, 11, 17, 20, 51, 2, 342662) - TIMESTAMP_EARLY = datetime.datetime(2014, 10, 8, 15, 59, 59, 370290) - - def setUp(self): - '''Performed before every test''' - - #a brand new user - self.facility = Facility(name=self.FACILITY) - self.facility.save() - - self.user_with_no_activity = FacilityUser(username=self.USERNAME1, facility=self.facility) - self.user_with_no_activity.set_password(self.PASSWORD) - self.user_with_no_activity.save() - - #a user with valid exercises - self.user_with_activity = FacilityUser(username=self.USERNAME2, facility=self.facility) - self.user_with_activity.set_password(self.PASSWORD) - self.user_with_activity.save() - - #add some exercises for second user (both incomplete) - self.original_exerciselog2 = ExerciseLog(exercise_id=self.EXERCISE_ID, user = self.user_with_activity, complete=False) - self.original_exerciselog2.points = self.ORIGINAL_POINTS - self.original_exerciselog2.attempts = self.ORIGINAL_POINTS - self.original_exerciselog2.streak_progress = self.ORIGINAL_STREAK_PROGRESS - self.original_exerciselog2.latest_activity_timestamp = self.TIMESTAMP_EARLY - self.original_exerciselog2.completion_timestamp = self.TIMESTAMP_EARLY - self.original_exerciselog2.save() - - self.original_exerciselog2 = ExerciseLog(exercise_id=self.EXERCISE_ID2, user = self.user_with_activity, complete=False) - self.original_exerciselog2.points = self.ORIGINAL_POINTS - self.original_exerciselog2.attempts = self.ORIGINAL_POINTS - self.original_exerciselog2.streak_progress = self.ORIGINAL_STREAK_PROGRESS - self.original_exerciselog2.latest_activity_timestamp = self.TIMESTAMP_LATER - self.original_exerciselog2.completion_timestamp = self.TIMESTAMP_LATER - self.original_exerciselog2.save() - - def test_get_most_recent_incomplete_item(self): - '''get_most_recent_incomplete_item()''' - - #test user with activity first - expected = { - "id": unicode(self.EXERCISE_ID2, 'utf-8'), - "timestamp": self.TIMESTAMP_LATER, - "kind": "Exercise" - } - actual = get_most_recent_incomplete_item(self.user_with_activity) - self.assertEqual(expected, actual) - - - #new user just created (no activity logged) - self.assertIsNone(get_most_recent_incomplete_item(user=self.user_with_no_activity)) + # ensure that no recommendations are returned + self.assertEqual(len(actual), 0) diff --git a/kalite/updates/api_views.py b/kalite/updates/api_views.py index 0fb0a0d1ff..f5b4d0cfca 100644 --- a/kalite/updates/api_views.py +++ b/kalite/updates/api_views.py @@ -119,20 +119,22 @@ def cancel_update_progress(request, process_log): @require_admin @api_handle_error_with_json def start_video_download(request): - force_job("videodownload", stop=True, locale=request.language) - """ API endpoint for launching the videodownload job. """ - paths = OrderedSet(simplejson.loads(request.body or "{}").get("paths", [])) + force_job("videodownload", stop=True, locale=request.language) + + paths = OrderedSet(json.loads(request.body or "{}").get("paths", [])) - youtube_ids = get_download_youtube_ids(paths) + lang = json.loads(request.body or "{}").get("lang", "en") + + youtube_ids = get_download_youtube_ids(paths, language=lang) queue = VideoQueue() - queue.add_files(youtube_ids, language=request.language) + queue.add_files(youtube_ids, language=lang) - force_job("videodownload", _("Download Videos"), locale=request.language) + force_job("videodownload", _("Download Videos"), locale=lang) return JsonResponseMessageSuccess(_("Launched video download process successfully.")) @@ -144,9 +146,11 @@ def delete_videos(request): API endpoint for deleting videos. """ - paths = OrderedSet(simplejson.loads(request.body or "{}").get("paths", [])) + paths = OrderedSet(json.loads(request.body or "{}").get("paths", [])) + + lang = json.loads(request.body or "{}").get("lang", "en") - youtube_ids = get_download_youtube_ids(paths) + youtube_ids = get_download_youtube_ids(paths, language=lang) num_deleted = 0 @@ -155,7 +159,7 @@ def delete_videos(request): if delete_downloaded_files(id): num_deleted += 1 - annotate_content_models_by_youtube_id(youtube_ids=youtube_ids.keys(), language=request.language) + annotate_content_models_by_youtube_id(youtube_ids=youtube_ids.keys(), language=lang) return JsonResponseMessageSuccess(_("Deleted %(num_videos)s video(s) successfully.") % {"num_videos": num_deleted}) @@ -164,7 +168,7 @@ def delete_videos(request): @api_handle_error_with_json def cancel_video_download(request): - force_job("videodownload", stop=True, locale=request.language) + force_job("videodownload", stop=True) queue = VideoQueue() @@ -177,7 +181,9 @@ def cancel_video_download(request): @api_handle_error_with_json def video_scan(request): - force_job("videoscan", _("Scan for Videos"), language=request.language) + lang = json.loads(request.body or "{}").get("lang", "en") + + force_job("videoscan", _("Scan for Videos"), language=lang) return JsonResponseMessageSuccess(_("Scanning for videos started.")) @@ -216,10 +222,10 @@ def delete_language_pack(request): @require_admin @api_handle_error_with_json -def get_update_topic_tree(request, lang_code=None): +def get_update_topic_tree(request): parent = request.GET.get("parent") - lang_code = lang_code or request.language # Get annotations for the current language. + lang_code = request.GET.get("lang") or request.language # Get annotations for the current language. return JsonResponse(get_topic_update_nodes(parent=parent, language=lang_code)) diff --git a/kalite/updates/download_track.py b/kalite/updates/download_track.py index d90036a98a..7fa23da6fc 100644 --- a/kalite/updates/download_track.py +++ b/kalite/updates/download_track.py @@ -45,11 +45,14 @@ def load(self): def remove_file(self, youtube_id): """Remove the last file from the list, and check that it matches the passed in youtube_id""" - removed_file = self.queue.pop() - if removed_file.get("youtube_id") != youtube_id: - logging.warn("Tried to remove {youtube_id} from file queue but found {removed_file} instead.".format(youtube_id=youtube_id, removed_file=removed_file.get("youtube_id"))) - else: - self.save() + try: + removed_file = self.queue.pop() + if removed_file.get("youtube_id") != youtube_id: + logging.warn("Tried to remove {youtube_id} from file queue but found {removed_file} instead.".format(youtube_id=youtube_id, removed_file=removed_file.get("youtube_id"))) + else: + self.save() + except IndexError: + logging.warn("Tried to remove {youtube_id} from file queue, but was empty instead.".format(youtube_id=youtube_id)) def clear(self): """Clear all currently queued videos""" diff --git a/kalite/updates/static/js/updates/bundle_modules/update_videos.js b/kalite/updates/static/js/updates/bundle_modules/update_videos.js index fd854b0b01..746ef281ee 100644 --- a/kalite/updates/static/js/updates/bundle_modules/update_videos.js +++ b/kalite/updates/static/js/updates/bundle_modules/update_videos.js @@ -47,7 +47,7 @@ function video_check_callback(progress_log, resp) { var status = progress_log.stage_status; if (keyCompleted) { - if (!status) { + if (!status && (status !== "error")) { setNodeClass(tree.getNodeByKey(keyCompleted), "complete"); } else if (status == "error") { // update # errors @@ -67,8 +67,6 @@ function video_check_callback(progress_log, resp) { }); return; - } else if (lastKey != currentKey) { - setNodeClass(tree.getNodeByKey(currentKey), "partial"); } } else if (progress_log.completed) { @@ -86,6 +84,7 @@ function video_check_callback(progress_log, resp) { } var tree; +var lang_code; var video_callbacks = { start: video_start_callback, @@ -108,6 +107,7 @@ var scan_callbacks = { $(function() { + lang_code = $("#download_language_selector option:selected")[0].value; $("#content_tree").html(""); @@ -120,7 +120,7 @@ $(function() { clickFolderMode: 2, source: { url: window.Urls.get_update_topic_tree(), - data: { parent: "root"}, + data: { parent: "root", lang: lang_code}, cache: false }, postProcess: function(event, data) { @@ -136,17 +136,21 @@ $(function() { node["lazy"] = true; node["folder"] = true; } - node["key"] = node["id"]; + node["key"] = node.youtube_id || node.id; }); }, lazyLoad: function(event, data){ var node = data.node; data.result = { url: window.Urls.get_update_topic_tree(), - data:{ parent: node.key}, + data:{ parent: node.key, lang: lang_code}, cache: false }; }, + loadChildren: function(event, data) { + // Apply parent's state to new child nodes: + data.node.fixSelection3AfterClick(); + }, select: function(event, node) { // only allow selection if we're not currently downloading if (!videos_downloading) { @@ -215,7 +219,7 @@ $(function() { var paths = _.map(tree.getSelectedNodes(true), function(node) { return node.data.path; }); // Do the request - api.doRequest(window.Urls.start_video_download(), {paths: paths}) + api.doRequest(window.Urls.start_video_download(), {paths: paths, lang: lang_code}) .success(function() { base.updatesStart("videodownload", 2000, video_callbacks); }) @@ -260,7 +264,7 @@ $(function() { var paths = _.map(tree.getSelectedNodes(true), function(node) { return node.data.path; }); // Do the request - api.doRequest(window.Urls.delete_videos(), {paths: paths}) + api.doRequest(window.Urls.delete_videos(), {paths: paths, lang: lang_code}) .success(function() { //update fancytree to reflect the current status of the videos $.each(downloading_node, function(ind, node) { @@ -324,7 +328,7 @@ $(function() { } $("#download_language_selector").change(function() { - var lang_code = $("#download_language_selector option:selected")[0].value; + lang_code = $("#download_language_selector option:selected")[0].value; window.location.href = get_params.setGetParam(window.location.href, "lang", lang_code); }); @@ -336,7 +340,7 @@ $(function() { $("#scan-videos").attr("disabled", "disabled"); // Do the request - api.doRequest(window.Urls.video_scan()) + api.doRequest(window.Urls.video_scan(), {lang: lang_code}) .success(function() { base.updatesStart("videoscan", 2000, scan_callbacks); @@ -411,10 +415,12 @@ function getSelectedStartedMetadata(data_type) { } function setNodeClass(node, className) { - if (node.extraClasses != className) { - node.extraClasses = className; - if (node.parent) { - updateNodeClass(node.parent); + if (node !== null) { + if (node.extraClasses != className) { + node.extraClasses = className; + if (node.parent !== null) { + updateNodeClass(node.parent); + } } } } diff --git a/kalite/updates/tests/api_tests.py b/kalite/updates/tests/api_tests.py index 2420445861..6e2ca3f76d 100644 --- a/kalite/updates/tests/api_tests.py +++ b/kalite/updates/tests/api_tests.py @@ -1,11 +1,17 @@ -import os +import json +import os from django.contrib.auth.models import User from django.core.management import call_command - from kalite.main.tests.base import MainTestCase +from kalite.testing.base import KALiteTestCase, KALiteClientTestCase from kalite.testing.client import KALiteClient +from kalite.testing.mixins.django_mixins import CreateAdminMixin +from kalite.testing.mixins.facility_mixins import FacilityMixins +from kalite.testing.mixins.securesync_mixins import CreateZoneMixin from kalite.topic_tools.content_models import get_content_item +from mock import patch + class TestAdminApiCalls(MainTestCase): @@ -73,3 +79,45 @@ def test_delete_existing_video_file(self): self.assertFalse(videofile.get("available")) assert videofile.get("size_on_disk") == 0 assert videofile.get("files_complete") == 0 + + +class VideoLanguageUpdateTestCase(CreateZoneMixin, + CreateAdminMixin, + FacilityMixins, + KALiteClientTestCase): + + def setUp(self, db=None): + super(VideoLanguageUpdateTestCase, self).setUp() + + self.admin_data = {"username": "admin", "password": "admin"} + self.admin = self.create_admin(**self.admin_data) + + self.zone = self.create_zone() + self.device_zone = self.create_device_zone(self.zone) + self.facility = self.create_facility() + + @patch('kalite.updates.api_views.force_job') + @patch('kalite.updates.api_views.get_download_youtube_ids', return_value={"test": "this"}) + def test_start_video_download(self, force_job, get_download_youtube_ids): + self.client.login(username='admin', password='admin') + lang = "es-ES" + api_resp = json.loads(self.client.post_json(url_name="start_video_download", data={"paths": "this/here", "lang": lang}).content) + args, kwargs = get_download_youtube_ids.call_args + self.assertEqual(kwargs["locale"], lang) + + @patch('kalite.updates.api_views.annotate_content_models_by_youtube_id') + @patch('kalite.updates.api_views.get_download_youtube_ids', return_value={"test": "this"}) + def test_start_video_delete(self, annotate_content_models_by_youtube_id, get_download_youtube_ids): + self.client.login(username='admin', password='admin') + lang = "es-ES" + api_resp = json.loads(self.client.post_json(url_name="delete_videos", data={"paths": "this/here", "lang": lang}).content) + args, kwargs = get_download_youtube_ids.call_args + self.assertEqual(kwargs["language"], lang) + + @patch('kalite.updates.api_views.force_job') + def test_start_video_scan(self, force_job): + self.client.login(username='admin', password='admin') + lang = "es-ES" + api_resp = json.loads(self.client.post_json(url_name="video_scan", data={"lang": lang}).content) + args, kwargs = force_job.call_args + self.assertEqual(kwargs["language"], lang) diff --git a/kalite/version.py b/kalite/version.py index 23503d5214..6808485946 100644 --- a/kalite/version.py +++ b/kalite/version.py @@ -7,7 +7,7 @@ # Must also be of the form N.N.N for internal use, where N is a non-negative integer MAJOR_VERSION = "0" MINOR_VERSION = "16" -PATCH_VERSION = "0" +PATCH_VERSION = "b1" VERSION = "%s.%s.%s" % (MAJOR_VERSION, MINOR_VERSION, PATCH_VERSION) SHORTVERSION = "%s.%s" % (MAJOR_VERSION, MINOR_VERSION) diff --git a/kalitectl.py b/kalitectl.py index 5c60a01d87..9bbfe66ca5 100644 --- a/kalitectl.py +++ b/kalitectl.py @@ -89,10 +89,18 @@ import cherrypy # We do not understand --option value, only --option=value. -# Match all patterns of "--option value" and fail if they exist -__validate_cmd_options = re.compile(r"--?[^\s]+\s+(?:(?!--|-[\w]))") +# Similarly, we don't understand -o Foo, only -oFoo +# Match all patterns as above and fail if they exist +# With single-dash options, there's a corner case if a dash appears in a positional argument, like so: +# kalite manage retrievecontentpack local es-ES foo.zip +# Be sure to match whitespace *before* the option starts, so that dashes *inside* arguments are okay! +# (?!...) is a negative lookahead assertion. It only matches if the ... pattern *isn't* found! +__validate_cmd_options = re.compile(r"\s--?[^\s]+\s+(?!--|-[\w])") if __validate_cmd_options.search(" ".join(sys.argv[1:])): - sys.stderr.write("Please only use --option=value or -x123 patterns. No spaces allowed between option and value. The option parser gets confused if you do otherwise.\n\nWill be fixed in a future release.") + sys.stderr.write("Please only use --option=value or -x123 patterns. No spaces allowed between option and value. " + "The option parser gets confused if you do otherwise." + "\nAdditionally, please put all Django management command options *after* positional arguments." + "\n\nWill be fixed in a future release.") sys.exit(1) from threading import Thread