From 840797185e20b15726b670c6b69f8b58ec129956 Mon Sep 17 00:00:00 2001 From: George Schneeloch Date: Mon, 20 Jul 2015 12:04:55 -0400 Subject: [PATCH 01/38] Used different haystack index for tests to prevent conflict with web application --- tox.ini | 1 + 1 file changed, 1 insertion(+) diff --git a/tox.ini b/tox.ini index acc6951f..78771809 100644 --- a/tox.ini +++ b/tox.ini @@ -11,6 +11,7 @@ commands = py.test {posargs} passenv = * setenv = CELERY_ALWAYS_EAGER=True + HAYSTACK_INDEX=testindex LORE_DB_DISABLE_SSL=True [testenv:docs] From 44f5c3c0aeaa7a1645e93d733036d43fb5ffb078 Mon Sep 17 00:00:00 2001 From: George Schneeloch Date: Fri, 17 Jul 2015 12:00:26 -0400 Subject: [PATCH 02/38] Implemented learning resource panel updating on every panel open --- ui/static/ui/js/learning_resources.jsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ui/static/ui/js/learning_resources.jsx b/ui/static/ui/js/learning_resources.jsx index 7c7936cf..5945a15d 100644 --- a/ui/static/ui/js/learning_resources.jsx +++ b/ui/static/ui/js/learning_resources.jsx @@ -222,9 +222,11 @@ define('learning_resources', [ VocabularyOption: VocabularyOption, LearningResourcePanel: LearningResourcePanel, loader: function (repoSlug, learningResourceId, container) { + // Unmount and remount the component to ensure that its state + // is always up to date with the rest of the app. + React.unmountComponentAtNode(container); React.render(, container); } }; From 2cf145c5cda048b7ebf21cd0bd761d60a3df492d Mon Sep 17 00:00:00 2001 From: Giovanni Di Milia Date: Thu, 16 Jul 2015 10:25:38 -0400 Subject: [PATCH 03/38] Added sorting to search results --- learningresources/tests/base.py | 16 ++++ search/__init__.py | 2 + search/forms.py | 11 ++- search/search_indexes.py | 4 + search/sorting.py | 54 +++++++++++++ search/tests/base.py | 21 ++++- search/tests/test_indexing.py | 69 +++++++++++++++++ search/tests/test_search_view.py | 1 + search/tests/test_sorting.py | 98 ++++++++++++++++++++++++ ui/templates/includes/facet_panel.html | 2 +- ui/templates/repository.html | 23 +++++- ui/tests/test_importer_views.py | 2 + ui/tests/test_learningresources_views.py | 52 +++++++++++++ ui/views.py | 19 +++++ 14 files changed, 364 insertions(+), 10 deletions(-) create mode 100644 search/sorting.py create mode 100644 search/tests/test_sorting.py diff --git a/learningresources/tests/base.py b/learningresources/tests/base.py index a5553a34..ea271aeb 100644 --- a/learningresources/tests/base.py +++ b/learningresources/tests/base.py @@ -57,6 +57,22 @@ def assert_status_code(self, url, code, return_body=False): if return_body: return resp.content.decode("utf-8") + def create_resource(self, **kwargs): + """Creates a learning resource with extra fields""" + learn_res = create_resource( + course=self.course, + parent=None, + resource_type=kwargs.get('resource_type') or "example", + title=kwargs.get('title') or "other silly example", + content_xml=kwargs.get('content_xml') or "other blah", + mpath=kwargs.get('mpath') or "/otherblah" + ) + learn_res.xa_nr_views = kwargs.get('xa_nr_views', 0) + learn_res.xa_nr_attempts = kwargs.get('xa_nr_attempts', 0) + learn_res.xa_avg_grade = kwargs.get('xa_avg_grade', 0) + learn_res.save() + return learn_res + def setUp(self): """set up""" super(LoreTestCase, self).setUp() diff --git a/search/__init__.py b/search/__init__.py index c8d1263b..e9e961eb 100644 --- a/search/__init__.py +++ b/search/__init__.py @@ -1,6 +1,8 @@ """ Import bits of the 'search' app which must be there when Django starts. """ +from __future__ import unicode_literals + from haystack.query import SearchQuerySet from search import signals diff --git a/search/forms.py b/search/forms.py index 27bfa32b..989733e0 100644 --- a/search/forms.py +++ b/search/forms.py @@ -1,6 +1,8 @@ """ Search form. """ +from __future__ import unicode_literals + from haystack.forms import FacetedSearchForm @@ -8,16 +10,19 @@ class SearchForm(FacetedSearchForm): """Customized version of haystack.forms.FacetedSearchForm""" def __init__(self, *args, **kwargs): - """__init__ override to get repo slug.""" + """__init__ override to get repo slug and sorting.""" self.repo_slug = kwargs.pop("repo_slug") + self.sortby = kwargs.pop("sortby") super(SearchForm, self).__init__(*args, **kwargs) def search(self): """Override search to filter on repository.""" sqs = super(SearchForm, self).search() - return sqs.narrow("repository_exact:{0}".format(self.repo_slug)) + sqs = sqs.narrow("repository_exact:{0}".format(self.repo_slug)) + return sqs.order_by('-{0}'.format(self.sortby)) def no_query_found(self): """We want to return everything, not nothing (the default).""" - return self.searchqueryset.narrow( + sqs = self.searchqueryset.narrow( "repository_exact:{0}".format(self.repo_slug)) + return sqs.order_by('-{0}'.format(self.sortby)) diff --git a/search/search_indexes.py b/search/search_indexes.py index 419877fd..f34efe4f 100644 --- a/search/search_indexes.py +++ b/search/search_indexes.py @@ -38,6 +38,10 @@ class LearningResourceIndex(indexes.SearchIndex, indexes.Indexable): # page because that page is always for a single repository. repository = indexes.CharField(faceted=True) + nr_views = indexes.IntegerField(model_attr="xa_nr_views") + nr_attempts = indexes.IntegerField(model_attr="xa_nr_attempts") + avg_grade = indexes.FloatField(model_attr="xa_avg_grade") + def get_model(self): """Return the model for which this configures indexing.""" return LearningResource diff --git a/search/sorting.py b/search/sorting.py new file mode 100644 index 00000000..3ca3959a --- /dev/null +++ b/search/sorting.py @@ -0,0 +1,54 @@ +""" +Definitions of sorting fields +""" +from __future__ import unicode_literals + + +class LoreSortingFields(object): + """ + Definition of possible sorting fields + """ + SORT_BY_NR_VIEWS = ('nr_views', 'Number of Views (desc)') + SORT_BY_NR_ATTEMPTS = ('nr_attempts', 'Number of Attempts (desc)') + SORT_BY_AVG_GRADE = ('avg_grade', 'Average Grade (desc)') + + DEFAULT_SORTING_FIELD = SORT_BY_NR_VIEWS[0] + + @classmethod + def all_sorting_options(cls): + """ + Returns all possible sortings + """ + return [ + cls.SORT_BY_NR_VIEWS, + cls.SORT_BY_NR_ATTEMPTS, + cls.SORT_BY_AVG_GRADE, + ] + + @classmethod + def all_sorting_fields(cls): + """ + Returns a list of all the sorting fields + """ + return [ + sorting_option[0] for sorting_option in cls.all_sorting_options() + ] + + @classmethod + def get_sorting_option(cls, field): + """ + Returns the sorting option tuple given the field + """ + if field not in cls.all_sorting_fields(): + field = cls.DEFAULT_SORTING_FIELD + return (field, dict(cls.all_sorting_options()).get(field)) + + @classmethod + def all_sorting_options_but(cls, field): + """ + Returns all the sorting options but the one with the specified field + """ + return [ + sorting_option for sorting_option in cls.all_sorting_options() + if sorting_option[0] != field + ] diff --git a/search/tests/base.py b/search/tests/base.py index 9edc6b15..0beef629 100644 --- a/search/tests/base.py +++ b/search/tests/base.py @@ -1,8 +1,10 @@ # -*- coding: utf-8 -*- """Tests for search engine indexing.""" +from __future__ import unicode_literals from learningresources.tests.base import LoreTestCase from search.forms import SearchForm +from search.sorting import LoreSortingFields from taxonomy.models import Term, Vocabulary @@ -24,14 +26,27 @@ def setUp(self): for label in ("easy", "medium", "very difficult", "ancòra") ] + def search(self, query, sorting=LoreSortingFields.DEFAULT_SORTING_FIELD): + """ + Helper function to perform a search + """ + form = SearchForm( + data={"q": query}, + repo_slug=self.repo.slug, + sortby=sorting + ) + return form.search() + def count_results(self, query): """Return count of matching indexed records.""" - form = SearchForm(data={"q": query}, repo_slug=self.repo.slug) - return form.search().count() + return self.search(query).count() def count_faceted_results(self, vocab, term): """Return count of matching indexed records by facet.""" facet_query = "{0}_exact:{1}".format(vocab, term) form = SearchForm( - selected_facets=[facet_query], repo_slug=self.repo.slug) + selected_facets=[facet_query], + repo_slug=self.repo.slug, + sortby=LoreSortingFields.DEFAULT_SORTING_FIELD + ) return form.search().count() diff --git a/search/tests/test_indexing.py b/search/tests/test_indexing.py index ee82d346..07d3ed71 100644 --- a/search/tests/test_indexing.py +++ b/search/tests/test_indexing.py @@ -1,5 +1,7 @@ """Tests for search engine indexing.""" +from __future__ import unicode_literals +from search.sorting import LoreSortingFields from search.tests.base import SearchTestCase @@ -36,3 +38,70 @@ def test_strip_xml(self): self.resource.save() self.assertTrue(self.count_results("you're it") == 1) self.assertTrue(self.count_results(xml) == 0) + + def test_sorting(self): + """Test sorting for search""" + # remove the default resource to control the environment + self.resource.delete() + # create some resources + res1 = self.create_resource(**dict( + resource_type="example", + title="silly example 1", + content_xml="blah 1", + mpath="/blah1", + xa_nr_views=1001, + xa_nr_attempts=99, + xa_avg_grade=9.9 + )) + res2 = self.create_resource(**dict( + resource_type="example", + title="silly example 2", + content_xml="blah 2", + mpath="/blah2", + xa_nr_views=1003, + xa_nr_attempts=98, + xa_avg_grade=6.8 + )) + res3 = self.create_resource(**dict( + resource_type="example", + title="silly example 3", + content_xml="blah 3", + mpath="/blah3", + xa_nr_views=1002, + xa_nr_attempts=101, + xa_avg_grade=7.3 + )) + self.assertEqual(self.count_results(''), 3) + # sorting by number of views + results = self.search( + '', + sorting=LoreSortingFields.SORT_BY_NR_VIEWS[0] + ) + # expected position (res2, res3, res1) + top_res = results[0] + self.assertEqual( + top_res.nr_views, + res2.xa_nr_views + ) + # sorting by number of attempts + results = self.search( + '', + sorting=LoreSortingFields.SORT_BY_NR_ATTEMPTS[0] + ) + # expected position (res3, res1, res2) + top_res = results[0] + self.assertEqual( + top_res.nr_attempts, + res3.xa_nr_attempts + ) + # sorting by average grade + results = self.search( + '', + sorting=LoreSortingFields.SORT_BY_AVG_GRADE[0] + ) + # expected position (res1, res3, res2) + top_res = results[0] + self.assertEqual( + top_res.avg_grade, + res1.xa_avg_grade + ) diff --git a/search/tests/test_search_view.py b/search/tests/test_search_view.py index 3af4c54b..a5a5ed4e 100644 --- a/search/tests/test_search_view.py +++ b/search/tests/test_search_view.py @@ -1,5 +1,6 @@ # -*- coding: utf-8 -*- """Tests for repostitory listing view search.""" +from __future__ import unicode_literals from django.core.urlresolvers import reverse diff --git a/search/tests/test_sorting.py b/search/tests/test_sorting.py new file mode 100644 index 00000000..d4b0ef29 --- /dev/null +++ b/search/tests/test_sorting.py @@ -0,0 +1,98 @@ +""" +Tests for search sorting options. +""" + +from __future__ import unicode_literals + +from django.test.testcases import TestCase + +from search.sorting import LoreSortingFields + + +class SortingTest(TestCase): + """Test sorting options""" + + def test_all_sorting_options(self): + """Test all_sorting_options method""" + self.assertEqual( + LoreSortingFields.all_sorting_options(), + [ + LoreSortingFields.SORT_BY_NR_VIEWS, + LoreSortingFields.SORT_BY_NR_ATTEMPTS, + LoreSortingFields.SORT_BY_AVG_GRADE, + ] + ) + + def test_all_sorting_fields(self): + """Test all_sorting_fields method""" + self.assertEqual( + LoreSortingFields.all_sorting_fields(), + [ + LoreSortingFields.SORT_BY_NR_VIEWS[0], + LoreSortingFields.SORT_BY_NR_ATTEMPTS[0], + LoreSortingFields.SORT_BY_AVG_GRADE[0], + ] + ) + + def test_get_sorting_option(self): + """Test get_sorting_option method""" + self.assertEqual( + LoreSortingFields.get_sorting_option( + LoreSortingFields.SORT_BY_NR_VIEWS[0] + ), + LoreSortingFields.SORT_BY_NR_VIEWS + ) + self.assertEqual( + LoreSortingFields.get_sorting_option( + LoreSortingFields.SORT_BY_NR_ATTEMPTS[0] + ), + LoreSortingFields.SORT_BY_NR_ATTEMPTS + ) + self.assertEqual( + LoreSortingFields.get_sorting_option( + LoreSortingFields.SORT_BY_AVG_GRADE[0] + ), + LoreSortingFields.SORT_BY_AVG_GRADE + ) + self.assertEqual( + LoreSortingFields.get_sorting_option('foo_field'), + LoreSortingFields.SORT_BY_NR_VIEWS + ) + + def test_all_sorting_options_but(self): + """Test all_sorting_options_but method""" + self.assertEqual( + LoreSortingFields.all_sorting_options_but( + LoreSortingFields.SORT_BY_NR_VIEWS[0] + ), + [ + LoreSortingFields.SORT_BY_NR_ATTEMPTS, + LoreSortingFields.SORT_BY_AVG_GRADE, + ] + ) + self.assertEqual( + LoreSortingFields.all_sorting_options_but( + LoreSortingFields.SORT_BY_NR_ATTEMPTS[0] + ), + [ + LoreSortingFields.SORT_BY_NR_VIEWS, + LoreSortingFields.SORT_BY_AVG_GRADE, + ] + ) + self.assertEqual( + LoreSortingFields.all_sorting_options_but( + LoreSortingFields.SORT_BY_AVG_GRADE[0] + ), + [ + LoreSortingFields.SORT_BY_NR_VIEWS, + LoreSortingFields.SORT_BY_NR_ATTEMPTS, + ] + ) + self.assertEqual( + LoreSortingFields.all_sorting_options_but('foo_field'), + [ + LoreSortingFields.SORT_BY_NR_VIEWS, + LoreSortingFields.SORT_BY_NR_ATTEMPTS, + LoreSortingFields.SORT_BY_AVG_GRADE, + ] + ) diff --git a/ui/templates/includes/facet_panel.html b/ui/templates/includes/facet_panel.html index 110eceba..6e9b434e 100644 --- a/ui/templates/includes/facet_panel.html +++ b/ui/templates/includes/facet_panel.html @@ -1,7 +1,7 @@ {% load static %} {% if facets.fields.course %} -

diff --git a/ui/templates/repository.html b/ui/templates/repository.html index 4548b26d..671800c4 100644 --- a/ui/templates/repository.html +++ b/ui/templates/repository.html @@ -29,7 +29,24 @@

-
+
+ {% if page.object_list %} + + Sort by +
+ + + +
+ {% endif %} +
{% for result in page.object_list %} @@ -42,12 +59,12 @@
{% if page.has_other_pages %} {% if page.has_previous %} - + {% endif %} {% if page.has_next %} - + {% endif %} diff --git a/ui/tests/test_importer_views.py b/ui/tests/test_importer_views.py index bf775564..955df1ac 100644 --- a/ui/tests/test_importer_views.py +++ b/ui/tests/test_importer_views.py @@ -13,6 +13,7 @@ from learningresources.tests.base import LoreTestCase from roles.api import assign_user_to_repo_group, remove_user_from_repo_group from roles.api import GroupTypes +from search.sorting import LoreSortingFields from ui.views import RepositoryView HTTP_OK = 200 @@ -170,6 +171,7 @@ def test_wiped_index(self): """ repo_view = RepositoryView() repo_view.repo = self.repo + repo_view.sortby = LoreSortingFields.DEFAULT_SORTING_FIELD repo_view.request = mock.MagicMock() with mock.patch('ui.views.get_perms') as _: with mock.patch( diff --git a/ui/tests/test_learningresources_views.py b/ui/tests/test_learningresources_views.py index 7f8a1846..faf812b1 100644 --- a/ui/tests/test_learningresources_views.py +++ b/ui/tests/test_learningresources_views.py @@ -17,6 +17,7 @@ from learningresources.models import Repository, StaticAsset from roles.api import assign_user_to_repo_group, remove_user_from_repo_group from roles.permissions import GroupTypes +from search.sorting import LoreSortingFields from learningresources.tests.base import LoreTestCase @@ -261,6 +262,57 @@ def test_repo_course_filter(self): resp = self.client.get(self.repository_url + querystring, follow=True) self.assertTrue(resp.status_code == HTTP_OK) + def test_listing_with_sorting(self): + """ + Hit the listing with sorting and test that the current sorting + changes in the interface. + The actual sorting of results is tested in search.tests.test_indexing + """ + url = self.repository_url + "?sortby={0}" + base_sorting_str = ('') + # test no sort type + body = self.assert_status_code( + self.repository_url, + HTTP_OK, + return_body=True + ) + self.assertIn( + base_sorting_str.format( + LoreSortingFields.get_sorting_option( + LoreSortingFields.DEFAULT_SORTING_FIELD + )[1] + ), + body + ) + # test all the allowed sort types + for sort_option in LoreSortingFields.all_sorting_options(): + sort_url = url.format(sort_option[0]) + body = self.assert_status_code( + sort_url, + HTTP_OK, + return_body=True + ) + self.assertIn( + base_sorting_str.format(sort_option[1]), + body + ) + # test sorting by not allowed sort type + url_not_allowed_sort_type = url.format('foo_field') + body = self.assert_status_code( + url_not_allowed_sort_type, + HTTP_OK, + return_body=True + ) + self.assertIn( + base_sorting_str.format( + LoreSortingFields.get_sorting_option( + LoreSortingFields.DEFAULT_SORTING_FIELD + )[1] + ), + body + ) + def test_serve_media(self): """Hit serve media""" self.assertEqual( diff --git a/ui/views.py b/ui/views.py index 5b96d7e9..d2c1f97f 100644 --- a/ui/views.py +++ b/ui/views.py @@ -30,6 +30,7 @@ from roles.api import assign_user_to_repo_group from roles.permissions import GroupTypes, RepoPermission from search import get_sqs +from search.sorting import LoreSortingFields from taxonomy.models import Vocabulary from ui.forms import UploadForm, RepositoryForm @@ -136,6 +137,14 @@ def __call__(self, request, repo_slug): if repo_slug not in set([x.slug for x in repos]): return HttpResponseForbidden("unauthorized") self.repo = [x for x in repos if x.slug == repo_slug][0] + # get sorting from params if it's there + sortby = dict(request.GET.copy()).get('sortby', []) + if (len(sortby) > 0 and + sortby[0] in LoreSortingFields.all_sorting_fields()): + self.sortby = sortby[0] + else: + # default value + self.sortby = LoreSortingFields.DEFAULT_SORTING_FIELD return super(RepositoryView, self).__call__(request) def dispatch(self, *args, **kwargs): @@ -152,6 +161,9 @@ def extra_context(self): # something like ?page=2&page=3&page=4. if "page" in params: params.pop("page") + # for the same reason I remove the sorting + if "sortby" in params: + params.pop("sortby") if len(params) > 0: qs_prefix = [] for key in params.keys(): @@ -175,6 +187,12 @@ def extra_context(self): if k in vocabularies }, "qs_prefix": qs_prefix, + "sorting_options": { + "current": LoreSortingFields.get_sorting_option( + self.sortby), + "all": LoreSortingFields.all_sorting_options_but( + self.sortby) + } }) return context @@ -188,6 +206,7 @@ def build_form(self, form_kwargs=None): if form_kwargs is None: form_kwargs = {} form_kwargs["repo_slug"] = self.repo.slug + form_kwargs["sortby"] = self.sortby return super(RepositoryView, self).build_form(form_kwargs) From 67d56e3a0d70a708873720d0438c44dd528fa753 Mon Sep 17 00:00:00 2001 From: George Schneeloch Date: Mon, 13 Jul 2015 17:21:03 -0400 Subject: [PATCH 04/38] Implemented preview link for learning resource panel --- apiary.apib | 14 +++++--- importer/api/__init__.py | 11 ++++-- learningresources/api.py | 22 ++++++++---- .../0011_learningresource_url_name.py | 20 +++++++++++ learningresources/models.py | 30 ++++++++++++++++ learningresources/tests/base.py | 6 ++-- lore/settings.py | 5 +++ pylintrc | 6 ++++ requirements.txt | 3 +- rest/serializers.py | 7 ++++ rest/tests/test_rest.py | 35 +++++++++++++++++-- ui/static/ui/js/learning_resources.jsx | 6 +++- ui/templates/includes/learning_resource.html | 1 + 13 files changed, 144 insertions(+), 22 deletions(-) create mode 100644 learningresources/migrations/0011_learningresource_url_name.py diff --git a/apiary.apib b/apiary.apib index 57c8ecf2..e73175d4 100644 --- a/apiary.apib +++ b/apiary.apib @@ -21,7 +21,7 @@ URL Structure is `https://{domain}/api/v1/{resource}/{resource_id}/` *Example: `http://lore.odl.mit.edu/api/v1/repositories/physics-1/` will return a JSON representation of the repository with name "physics-1".* -*Example: `http://lore.odl.mit.edu/api/v1/repositories/physics-1/learningresources/1007/` will return a JSON representation +*Example: `http://lore.odl.mit.edu/api/v1/repositories/physics-1/learning_resources/1007/` will return a JSON representation of the learning resource with ID=1007.* @@ -168,7 +168,8 @@ Create a repository by providing its JSON representation. "xa_nr_attempts": 0, "xa_avg_grade": 0.0, "xa_histogram_grade": 0.0, - "terms": [] + "terms": [], + "preview_url": "https://www.example.com/courses..." }, ... ] @@ -199,7 +200,8 @@ Create a repository by providing its JSON representation. "xa_nr_attempts": 0, "xa_avg_grade": 0.0, "xa_histogram_grade": 0.0, - "terms": [] + "terms": [], + "preview_url": "https://www.example.com/courses..." } ### Partially Update a Learningresource [PATCH] @@ -227,7 +229,8 @@ Create a repository by providing its JSON representation. "xa_nr_attempts": 0, "xa_avg_grade": 0.0, "xa_histogram_grade": 0.0, - "terms": [] + "terms": [], + "preview_url": "https://www.example.com/courses..." } ### Update a Learningresource [PUT] @@ -256,7 +259,8 @@ Create a repository by providing its JSON representation. "xa_nr_attempts": 0, "xa_avg_grade": 0.0, "xa_histogram_grade": 0.0, - "terms": [] + "terms": [], + "preview_url": "https://www.example.com/courses..." } ## Group Staticassets diff --git a/importer/api/__init__.py b/importer/api/__init__.py index cfdc38bc..fcf915b2 100644 --- a/importer/api/__init__.py +++ b/importer/api/__init__.py @@ -92,7 +92,9 @@ def import_course_from_path(path, repo_id, user_id): Returns: course (learningresources.Course) """ - bundle = XBundle() + bundle = XBundle( + keep_urls=True, keep_studio_urls=True, preserve_url_name=True + ) bundle.import_from_directory(path) static_dir = join(path, 'static') course = import_course(bundle, repo_id, user_id, static_dir) @@ -112,6 +114,7 @@ def import_course(bundle, repo_id, user_id, static_dir): learningresources.models.Course """ src = bundle.course + course = create_course( org=src.attrib["org"], repo_id=repo_id, @@ -130,9 +133,10 @@ def import_children(course, element, parent): of an XML tree. Args: - course (learningresources.Course): Course + course (learningresources.models.Course): Course element (lxml.etree): XML element within xbundle - parent (learningresources.LearningResource): Parent LearningResource + parent (learningresources.models.LearningResource): + Parent LearningResource Returns: None """ @@ -142,6 +146,7 @@ def import_children(course, element, parent): title=element.attrib.get("display_name", "MISSING"), content_xml=etree.tostring(element), mpath=mpath, + url_name=element.attrib.get("url_name", None), ) target = "/static/" if element.tag == "video": diff --git a/learningresources/api.py b/learningresources/api.py index 19d75c80..d9c1dad8 100644 --- a/learningresources/api.py +++ b/learningresources/api.py @@ -47,13 +47,14 @@ def create_course(org, repo_id, course_number, run, user_id): Args: org (unicode): Organization + repo_id (int): Repository id course_number (unicode): Course number run (unicode): Run user_id (int): Primary key of user creating the course Raises: ValueError: Duplicate course Returns: - course (learningresource.Course): The created course + course (learningresources.models.Course): The created course """ # Check on unique values before attempting a get_or_create, because @@ -65,7 +66,9 @@ def create_course(org, repo_id, course_number, run, user_id): if Course.objects.filter(**unique).exists(): raise ValueError("Duplicate course") kwargs = { - "org": org, "course_number": course_number, "run": run, + "org": org, + "course_number": course_number, + "run": run, 'imported_by_id': user_id, "repository_id": repo_id, } @@ -75,20 +78,24 @@ def create_course(org, repo_id, course_number, run, user_id): # pylint: disable=too-many-arguments -def create_resource(course, parent, resource_type, title, content_xml, mpath): +def create_resource( + course, parent, resource_type, title, content_xml, mpath, url_name +): """ Create a learning resource. Args: - course (learningresources.Course): Course - parent (learningresources.LearningResource): Parent LearningResource + course (learningresources.models.Course): Course + parent (learningresources.models.LearningResource): + Parent LearningResource resource_type (unicode): Name of LearningResourceType title (unicode): Title of resource content_xml (unicode): XML mpath (unicode): Materialized path - + url_name (unicode): Resource identifier Returns: - resource (learningresources.LearningResource): New LearningResource + resource (learningresources.models.LearningResource): + New LearningResource """ params = { "course": course, @@ -96,6 +103,7 @@ def create_resource(course, parent, resource_type, title, content_xml, mpath): "title": title, "content_xml": content_xml, "materialized_path": mpath, + "url_name": url_name, } if parent is not None: params["parent_id"] = parent.id diff --git a/learningresources/migrations/0011_learningresource_url_name.py b/learningresources/migrations/0011_learningresource_url_name.py new file mode 100644 index 00000000..a671b26f --- /dev/null +++ b/learningresources/migrations/0011_learningresource_url_name.py @@ -0,0 +1,20 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import models, migrations + +# pylint: skip-file + +class Migration(migrations.Migration): + + dependencies = [ + ('learningresources', '0010_static_asset_file_length'), + ] + + operations = [ + migrations.AddField( + model_name='learningresource', + name='url_name', + field=models.TextField(null=True), + ), + ] diff --git a/learningresources/models.py b/learningresources/models.py index 8e8d2ca7..ebb2fb86 100644 --- a/learningresources/models.py +++ b/learningresources/models.py @@ -5,6 +5,8 @@ from __future__ import unicode_literals import logging +# pylint currently has a bug which incorrectly flags this as an error +import six.moves.urllib.parse as urllib_parse # pylint: disable=import-error from django.db import models from django.db import transaction @@ -16,6 +18,7 @@ from audit.models import BaseModel from roles.api import roles_init_new_repo, roles_update_repo from roles.permissions import RepoPermission +from lore.settings import LORE_PREVIEW_BASE_URL log = logging.getLogger(__name__) @@ -129,6 +132,33 @@ class LearningResource(BaseModel): xa_nr_attempts = models.IntegerField(default=0) xa_avg_grade = models.FloatField(default=0) xa_histogram_grade = models.FloatField(default=0) + url_name = models.TextField(null=True) + + def get_preview_url(self): + """Create a preview URL.""" + key = "{org}/{course}/{run}".format( + org=self.course.org, + course=self.course.course_number, + run=self.course.run, + ) + + if self.url_name is not None: + url_format = 'courses/{key}/jump_to_id/{preview_id}' + return LORE_PREVIEW_BASE_URL + urllib_parse.quote( + url_format.format( + base_url=LORE_PREVIEW_BASE_URL, + key=key, + preview_id=self.url_name, + ) + ) + else: + url_format = 'courses/{key}/courseware' + return LORE_PREVIEW_BASE_URL + urllib_parse.quote( + url_format.format( + base_url=LORE_PREVIEW_BASE_URL, + key=key, + ) + ) @python_2_unicode_compatible diff --git a/learningresources/tests/base.py b/learningresources/tests/base.py index ea271aeb..8b57c43b 100644 --- a/learningresources/tests/base.py +++ b/learningresources/tests/base.py @@ -65,7 +65,8 @@ def create_resource(self, **kwargs): resource_type=kwargs.get('resource_type') or "example", title=kwargs.get('title') or "other silly example", content_xml=kwargs.get('content_xml') or "other blah", - mpath=kwargs.get('mpath') or "/otherblah" + mpath=kwargs.get('mpath') or "/otherblah", + url_name=kwargs.get('url_name') or None ) learn_res.xa_nr_views = kwargs.get('xa_nr_views', 0) learn_res.xa_nr_attempts = kwargs.get('xa_nr_attempts', 0) @@ -92,7 +93,7 @@ def setUp(self): user_id=self.user.id, ) self.course = create_course( - org="test org", + org="test-org", repo_id=self.repo.id, course_number="infinity", run="Febtober", @@ -105,6 +106,7 @@ def setUp(self): title="silly example", content_xml="blah", mpath="/blah", + url_name="url_name1", ) assign_user_to_repo_group( diff --git a/lore/settings.py b/lore/settings.py index 1940c246..7d08c265 100644 --- a/lore/settings.py +++ b/lore/settings.py @@ -219,6 +219,11 @@ def get_var(name, default): DEFAULT_FILE_STORAGE = 'django.core.files.storage.FileSystemStorage' +# Lore preview settings +LORE_PREVIEW_BASE_URL = get_var( + 'LORE_PREVIEW_BASE_URL', 'https://www.sandbox.edx.org/') + + # Configure e-mail settings EMAIL_HOST = get_var('LORE_EMAIL_HOST', 'localhost') EMAIL_PORT = get_var('LORE_EMAIL_PORT', 25) diff --git a/pylintrc b/pylintrc index cfdc640a..0e8e20de 100644 --- a/pylintrc +++ b/pylintrc @@ -5,6 +5,12 @@ const-rgx = (([A-Z_][A-Z0-9_]*)|(__.*__)|log|urlpatterns)$ [TYPECHECK] generated-members = status_code +ignored-classes= + six, + six.moves, +ignored-modules= + six, + six.moves, [MESSAGES CONTROL] disable = no-member, old-style-class, no-init, too-few-public-methods, abstract-method diff --git a/requirements.txt b/requirements.txt index 346e19e3..10231db6 100644 --- a/requirements.txt +++ b/requirements.txt @@ -16,6 +16,7 @@ redis==2.10.3 static3==0.6.1 tox>=2.0.2,<3.0.0 uwsgi==2.0.11 +six==1.9.0 # django guardian branch that fixed bug in master git+https://github.com/mitodl/django-guardian@de7ca5c39dfd0870067497f5843581e9cd2b017f#egg=django-guardian-dev @@ -24,7 +25,7 @@ git+https://github.com/mitodl/django-guardian@de7ca5c39dfd0870067497f5843581e9cd git+https://github.com/mingchen/django-cas-ng@69463fe7c23f025be7165c2967f4ceb983d60472#egg=django-cas-ng # Importer application requirements -git+https://github.com/mitodl/xbundle.git@7799a0957c502103fb030fc0b16c64204f551c75#egg=xbundle +git+https://github.com/mitodl/xbundle.git@84b5774faf55cf02ddb57abb8df330fdad3b678a#egg=xbundle git+https://github.com/gdub/python-archive.git@61b9afde21621a8acce964d3336a7fb5d2d07ca6#egg=python-archive django-storages-redux==1.2.3 python-magic==0.4.6 diff --git a/rest/serializers.py b/rest/serializers.py index 375caff0..affb3d45 100644 --- a/rest/serializers.py +++ b/rest/serializers.py @@ -186,6 +186,7 @@ class LearningResourceSerializer(ModelSerializer): learning_resource_type = StringRelatedField() terms = SlugRelatedField( many=True, slug_field='slug', queryset=Term.objects.all()) + preview_url = SerializerMethodField() class Meta: # pylint: disable=missing-docstring @@ -206,6 +207,7 @@ class Meta: 'xa_avg_grade', 'xa_histogram_grade', 'terms', + 'preview_url', ) read_only_fields = tuple(set(fields) - {'description', 'terms'}) @@ -223,6 +225,11 @@ def validate_terms(self, terms): "for this learning resource".format(term.label)) return terms + @staticmethod + def get_preview_url(obj): + """Construct preview URL for learning resource""" + return obj.get_preview_url() + class StaticAssetSerializer(ModelSerializer): """Serializer for StaticAsset""" diff --git a/rest/tests/test_rest.py b/rest/tests/test_rest.py index 2da666ee..d5a1951a 100644 --- a/rest/tests/test_rest.py +++ b/rest/tests/test_rest.py @@ -14,6 +14,7 @@ from importer.tasks import import_file from learningresources.models import ( Repository, + LearningResource, LearningResourceType, ) from learningresources.api import get_resources @@ -697,7 +698,8 @@ def assert_not_changed(new_dict): def test_immutable_fields_learning_resource(self): """Test immutable fields for term""" - resource = self.import_course_tarball(self.repo) + self.import_course_tarball(self.repo) + resource = LearningResource.objects.first() lr_id = resource.id lr_dict = { @@ -716,7 +718,8 @@ def test_immutable_fields_learning_resource(self): "xa_nr_attempts": 2, "xa_avg_grade": 3.0, "xa_histogram_grade": 4.0, - "terms": [] + "terms": [], + "preview_url": "", } def assert_not_changed(new_dict): @@ -727,7 +730,7 @@ def assert_not_changed(new_dict): 'id', 'learning_resource_type', 'static_assets', 'title', 'content_xml', 'materialized_path', 'url_path', 'parent', 'copyright', 'xa_nr_views', 'xa_nr_attempts', 'xa_avg_grade', - 'xa_histogram_grade' + 'xa_histogram_grade', 'preview_url' ) for field in fields: self.assertNotEqual(lr_dict[field], new_dict[field]) @@ -928,3 +931,29 @@ def test_vocabulary_learning_resource_types(self): ['example'], self.get_vocabulary( self.repo.slug, vocab_slug)['learning_resource_types']) + + def test_preview_url(self): + """ + Assert preview url behavior for learning resources + """ + learning_resource = LearningResource.objects.first() + expected_jump_to_id_url = ( + "https://www.sandbox.edx.org/courses/" + "test-org/infinity/Febtober/jump_to_id/url_name1" + ) + self.assertEqual( + expected_jump_to_id_url, + learning_resource.get_preview_url() + ) + + resource_dict = self.get_learning_resource( + self.repo.slug, learning_resource.id) + self.assertEqual( + expected_jump_to_id_url, resource_dict['preview_url']) + + learning_resource.url_name = None + self.assertEqual( + "https://www.sandbox.edx.org/courses/" + "test-org/infinity/Febtober/courseware", + learning_resource.get_preview_url() + ) diff --git a/ui/static/ui/js/learning_resources.jsx b/ui/static/ui/js/learning_resources.jsx index 5945a15d..558c1579 100644 --- a/ui/static/ui/js/learning_resources.jsx +++ b/ui/static/ui/js/learning_resources.jsx @@ -87,6 +87,8 @@ define('learning_resources', [ Select XML + Preview

{options} @@ -160,12 +162,14 @@ define('learning_resources', [ var learningResourceType = data.learning_resource_type; var description = data.description; var selectedTerms = data.terms; + var previewUrl = data.preview_url; thiz.setState({ contentXml: contentXml, messageText: undefined, errorText: undefined, - description: description + description: description, + previewUrl: previewUrl, }); Utils.getVocabulariesAndTerms( diff --git a/ui/templates/includes/learning_resource.html b/ui/templates/includes/learning_resource.html index 2314fd56..76725c2c 100644 --- a/ui/templates/includes/learning_resource.html +++ b/ui/templates/includes/learning_resource.html @@ -45,6 +45,7 @@

{{ result.object.course.course_number }} {{ result.object.course.run }} + Preview

From a477a5ecb98139d2f0e96a49146feda09e9d2861 Mon Sep 17 00:00:00 2001 From: Shawn Milochik Date: Mon, 20 Jul 2015 21:54:02 -0400 Subject: [PATCH 05/38] Changed vocabulary term indexing from string to integer. Prevents any funny business with special characters. Incidentally saves space in Elasticsearch. Fixes #384 Replaces and closes #374. Replaces and closes #316. --- search/__init__.py | 8 +-- search/search_indexes.py | 15 ++--- search/tests/test_indexing.py | 6 +- ui/templates/includes/facet_panel.html | 16 ++--- ui/views.py | 86 +++++++++++++++++++------- 5 files changed, 86 insertions(+), 45 deletions(-) diff --git a/search/__init__.py b/search/__init__.py index e9e961eb..e4915b9c 100644 --- a/search/__init__.py +++ b/search/__init__.py @@ -20,8 +20,8 @@ def get_sqs(): # Add hard-coded facets. for facet in ("course", "run", "resource_type"): sqs = sqs.facet(facet) - # Add dynamic facets (from taxonomy). Values with spaces do not work, - # so use the slug. - for slug in Vocabulary.objects.all().values_list("slug", flat=True): - sqs = sqs.facet(slug) + # Add dynamic facets (from taxonomy). Certain characters cause problems, + # so use the primary key. + for vocabulary_id in Vocabulary.objects.all().values_list("id", flat=True): + sqs = sqs.facet(vocabulary_id) return sqs diff --git a/search/search_indexes.py b/search/search_indexes.py index f34efe4f..efe2d778 100644 --- a/search/search_indexes.py +++ b/search/search_indexes.py @@ -91,16 +91,13 @@ def prepare(self, obj): """ prepared = super(LearningResourceIndex, self).prepare(obj) for vocab in Vocabulary.objects.all(): - # Values with spaces do not work, so replace them with underscores. - # Slugify doesn't work because it adds hypens, which are also - # split by Elasticsearch. - terms = [ - term.label.replace(" ", "_") - for term in obj.terms.filter(vocabulary_id=vocab.id) - ] - prepared[vocab.slug] = terms + # Use the integer primary keys as index values. This saves space, + # and also avoids all issues dealing with "special" characters. + terms = set(obj.terms.filter(vocabulary_id=vocab.id).values_list( + 'id', flat=True)) + prepared[vocab.id] = terms # for faceted "_exact" in URL - prepared[vocab.slug + "_exact"] = terms # for faceted "exact" + prepared["{0}_exact".format(vocab.id)] = terms return prepared def prepare_repository(self, obj): # pylint: disable=no-self-use diff --git a/search/tests/test_indexing.py b/search/tests/test_indexing.py index 07d3ed71..29f93c30 100644 --- a/search/tests/test_indexing.py +++ b/search/tests/test_indexing.py @@ -23,13 +23,13 @@ def test_index_vocabulary(self): """ term = self.terms[0] self.assertTrue(self.count_faceted_results( - self.vocabulary.name, term.label) == 0) + self.vocabulary.id, term.id) == 0) self.resource.terms.add(term) self.assertTrue(self.count_faceted_results( - self.vocabulary.name, term.label) == 1) + self.vocabulary.id, term.id) == 1) self.resource.terms.remove(term) self.assertTrue(self.count_faceted_results( - self.vocabulary.name, term.label) == 0) + self.vocabulary.id, term.id) == 0) def test_strip_xml(self): """Indexed content_xml should have XML stripped.""" diff --git a/ui/templates/includes/facet_panel.html b/ui/templates/includes/facet_panel.html index 6e9b434e..b46a538e 100644 --- a/ui/templates/includes/facet_panel.html +++ b/ui/templates/includes/facet_panel.html @@ -108,28 +108,28 @@

-{% for name, vocab in vocabularies.items %} -{% if vocab %} +{% for vocab, terms in vocabularies.items %} +{% if terms %}
    - {% for term in vocab %} + {% for term in terms %}
  • - - - {{ term.1 }} + + {{ term.2 }}
  • {% endfor %}
diff --git a/ui/views.py b/ui/views.py index d2c1f97f..80804494 100644 --- a/ui/views.py +++ b/ui/views.py @@ -31,7 +31,7 @@ from roles.permissions import GroupTypes, RepoPermission from search import get_sqs from search.sorting import LoreSortingFields -from taxonomy.models import Vocabulary +from taxonomy.models import Vocabulary, Term from ui.forms import UploadForm, RepositoryForm log = logging.getLogger(__name__) @@ -123,6 +123,58 @@ def create_repo(request): ) +def get_vocabularies(facets): + """ + Parse facet information for the template. + It will return a dictionary that looks like this: + + { + (u'13', u'difficulty'): [(38, u'medium', 23), (17, u'hard', 19)], + (u'15', u'prerequisite'): [(44, u'yes', 23)] + } + + The keys are tuples with Vocabulary ID and name for the key, + and a list of tuples containing id, label, and count for the terms. + + This is for ease-of-use in the template, where the integer primary keys + are used for search links and the names/labels are used for display. + + Args: + facets (dict): facet ID with terms & term counts + Returns: + vocabularies (dict): dict of vocab info to term info + """ + + if "fields" not in facets: + return {} + vocab_ids = [] + term_ids = [] + for vocab_id, counts in facets["fields"].items(): + if not vocab_id.isdigit(): + continue + vocab_ids.append(int(vocab_id)) + for term_id, count in counts: + term_ids.append(term_id) + + vocabs = { + x: y for x, y in Vocabulary.objects.filter( + id__in=vocab_ids).values_list('id', 'name') + } + terms = { + x: y for x, y in Term.objects.filter( + id__in=term_ids).values_list('id', 'label') + } + vocabularies = {} + for vocabulary_id, term_data in facets["fields"].items(): + if not vocabulary_id.isdigit(): + continue + vocab = (vocabulary_id, vocabs[int(vocabulary_id)]) + vocabularies[vocab] = [] + for t_id, count in term_data: + vocabularies[vocab].append((t_id, terms[int(t_id)], count)) + return vocabularies + + class RepositoryView(FacetedSearchView): """Subclass of haystack.views.FacetedSearchView""" @@ -154,7 +206,6 @@ def dispatch(self, *args, **kwargs): def extra_context(self): """Add to the context.""" context = super(RepositoryView, self).extra_context() - vocabularies = Vocabulary.objects.all().values_list("slug", flat=True) params = dict(self.request.GET.copy()) qs_prefix = "?" # Chop out page number so we don't end up with @@ -175,25 +226,18 @@ def extra_context(self): ) qs_prefix = "?{0}&".format("&".join(qs_prefix)) - if "fields" in context["facets"]: - context.update({ - "repo": self.repo, - "perms_on_cur_repo": get_perms(self.request.user, self.repo), - # Add a non-slug version to the context for display. This - # turns a "two-tuple" into a "three-tuple." - "vocabularies": { - k: [x + (x[0].replace("_", " "),) for x in v] - for k, v in context["facets"]["fields"].items() - if k in vocabularies - }, - "qs_prefix": qs_prefix, - "sorting_options": { - "current": LoreSortingFields.get_sorting_option( - self.sortby), - "all": LoreSortingFields.all_sorting_options_but( - self.sortby) - } - }) + context.update({ + "repo": self.repo, + "perms_on_cur_repo": get_perms(self.request.user, self.repo), + "vocabularies": get_vocabularies(context["facets"]), + "qs_prefix": qs_prefix, + "sorting_options": { + "current": LoreSortingFields.get_sorting_option( + self.sortby), + "all": LoreSortingFields.all_sorting_options_but( + self.sortby) + } + }) return context def build_form(self, form_kwargs=None): From 6b21d321eab3459c854aa227d89f5ff1dd2e2269 Mon Sep 17 00:00:00 2001 From: George Schneeloch Date: Mon, 20 Jul 2015 16:52:19 -0400 Subject: [PATCH 06/38] Grouped REST tests by common endpoint --- rest/tests/base.py | 36 +- rest/tests/test_learning_resource.py | 453 +++++++++ .../{test_rest_members.py => test_members.py} | 173 +++- rest/tests/test_misc.py | 50 + rest/tests/test_repository.py | 321 +++++++ rest/tests/test_rest_authorization.py | 883 ------------------ .../{test_rest.py => test_vocabulary.py} | 658 ++++++------- 7 files changed, 1325 insertions(+), 1249 deletions(-) create mode 100644 rest/tests/test_learning_resource.py rename rest/tests/{test_rest_members.py => test_members.py} (71%) create mode 100644 rest/tests/test_misc.py create mode 100644 rest/tests/test_repository.py delete mode 100644 rest/tests/test_rest_authorization.py rename rest/tests/{test_rest.py => test_vocabulary.py} (62%) diff --git a/rest/tests/base.py b/rest/tests/base.py index aeb1a4b1..1eec5a77 100644 --- a/rest/tests/base.py +++ b/rest/tests/base.py @@ -12,10 +12,13 @@ ) from rest_framework.reverse import reverse from django.db.models import Count +from django.contrib.auth.models import User, Permission from importer.tasks import import_file from learningresources.tests.base import LoreTestCase from learningresources.api import get_resources +from roles.api import assign_user_to_repo_group +from roles.permissions import GroupTypes API_BASE = '/api/v1/' REPO_BASE = '/api/v1/repositories/' @@ -588,7 +591,38 @@ def import_course_tarball(self, repo): """ tarball_file = self.get_course_single_tarball() import_file( - tarball_file, self.repo.id, self.user.id) + tarball_file, repo.id, self.user.id) return get_resources(repo.id).annotate( count_assets=Count('static_assets') ).filter(count_assets__gt=0).first() + + +class RESTAuthTestCase(RESTTestCase): + """REST tests for authorization""" + + def setUp(self): + super(RESTAuthTestCase, self).setUp() + + # add_repo_user is another user with add_repo permission but who + # does not have access to self.repo + self.add_repo_user = User.objects.create_user( + username="creator_user", password=self.PASSWORD + ) + add_repo_perm = Permission.objects.get(codename=self.ADD_REPO_PERM) + self.add_repo_user.user_permissions.add(add_repo_perm) + + # curator_user doesn't have add_repo but have view_repo permission on + # self.repo + self.curator_user = User.objects.create_user( + username="curator_user", password=self.PASSWORD + ) + assign_user_to_repo_group( + self.curator_user, self.repo, GroupTypes.REPO_CURATOR) + + # author_user doesn't have manage_taxonomy permission + self.author_user = User.objects.create_user( + username="author_user", password=self.PASSWORD + ) + assign_user_to_repo_group( + self.author_user, self.repo, GroupTypes.REPO_AUTHOR + ) diff --git a/rest/tests/test_learning_resource.py b/rest/tests/test_learning_resource.py new file mode 100644 index 00000000..ef5e6142 --- /dev/null +++ b/rest/tests/test_learning_resource.py @@ -0,0 +1,453 @@ +""" +REST tests for learning resources and static assets +""" + +from __future__ import unicode_literals +import os + +from rest_framework.status import ( + HTTP_200_OK, + HTTP_400_BAD_REQUEST, + HTTP_403_FORBIDDEN, + HTTP_404_NOT_FOUND, + HTTP_405_METHOD_NOT_ALLOWED, +) + +from rest.tests.base import ( + RESTTestCase, + RESTAuthTestCase, + REPO_BASE, + as_json, + API_BASE, +) +from learningresources.api import get_resources +from learningresources.models import ( + LearningResource, + LearningResourceType, +) +from importer.tasks import import_file +from taxonomy.models import Vocabulary + + +# pylint: disable=invalid-name +class TestLearningResource(RESTTestCase): + """ + REST tests for learning resources and static assets + """ + + def test_immutable_fields_learning_resource(self): + """Test immutable fields for term""" + resource = self.import_course_tarball(self.repo) + lr_id = resource.id + + lr_dict = { + "id": 99, + "learning_resource_type": 4, + "static_assets": [3], + "title": "Getting Help", + "description": "description", + "content_xml": "...", + "materialized_path": + "/course/chapter[4]/sequential[1]/vertical[3]", + "url_path": "url_path", + "parent": 22, + "copyright": "copyright", + "xa_nr_views": 1, + "xa_nr_attempts": 2, + "xa_avg_grade": 3.0, + "xa_histogram_grade": 4.0, + "terms": [], + "preview_url": "", + } + + def assert_not_changed(new_dict): + """Check that fields have not changed""" + # These keys should be different since they are immutable or set by + # the serializer. + fields = ( + 'id', 'learning_resource_type', 'static_assets', 'title', + 'content_xml', 'materialized_path', 'url_path', 'parent', + 'copyright', 'xa_nr_views', 'xa_nr_attempts', 'xa_avg_grade', + 'xa_histogram_grade', 'preview_url' + ) + for field in fields: + self.assertNotEqual(lr_dict[field], new_dict[field]) + + assert_not_changed( + self.patch_learning_resource(self.repo.slug, lr_id, lr_dict, + skip_assert=True)) + assert_not_changed( + self.put_learning_resource(self.repo.slug, lr_id, lr_dict, + skip_assert=True)) + + def test_missing_learning_resource(self): + """Test for an invalid learning resource id""" + repo_slug1 = self.repo.slug + resource1 = self.import_course_tarball(self.repo) + lr1_id = resource1.id + + # import from a different course so it's not a duplicate course + zip_file = self.get_course_zip() + new_repo_dict = self.create_repository() + repo_slug2 = new_repo_dict['slug'] + repo_id2 = new_repo_dict['id'] + import_file( + zip_file, repo_id2, self.user.id) + resource2 = get_resources(repo_id2).first() + lr2_id = resource2.id + + # repo_slug1 should own lr1_id and repo_slug2 should own lr2_id + self.get_learning_resource(repo_slug1, lr1_id) + self.get_learning_resource(repo_slug2, lr1_id, + expected_status=HTTP_404_NOT_FOUND) + self.get_learning_resource(repo_slug1, lr2_id, + expected_status=HTTP_404_NOT_FOUND) + self.get_learning_resource(repo_slug2, lr2_id) + + def test_filefield_serialization(self): + """Make sure that URL output is turned on in settings""" + resource = self.import_course_tarball(self.repo) + static_assets = self.get_static_assets( + self.repo.slug, resource.id)['results'] + self.assertTrue(static_assets[0]['asset'].startswith("http")) + + def test_add_term_to_learning_resource(self): + """ + Add a term to a learning resource via PATCH + """ + + resource = self.import_course_tarball(self.repo) + lr_id = resource.id + + vocab1_slug = self.create_vocabulary(self.repo.slug)['slug'] + supported_term_slug = self.create_term( + self.repo.slug, vocab1_slug)['slug'] + + # This should change soon but for now we can't set this via API + Vocabulary.objects.get(slug=vocab1_slug).learning_resource_types.add( + resource.learning_resource_type + ) + + vocab_dict = dict(self.DEFAULT_VOCAB_DICT) + vocab_dict['name'] += " changed" + vocab2_slug = self.create_vocabulary( + self.repo.slug, vocab_dict)['slug'] + unsupported_term_slug = self.create_term( + self.repo.slug, vocab2_slug)['slug'] + + self.assertEqual([], self.get_learning_resource( + self.repo.slug, lr_id)['terms']) + + self.patch_learning_resource( + self.repo.slug, lr_id, {"terms": [supported_term_slug]}) + self.patch_learning_resource( + self.repo.slug, lr_id, {"terms": ["missing"]}, + expected_status=HTTP_400_BAD_REQUEST) + self.patch_learning_resource( + self.repo.slug, lr_id, {"terms": [unsupported_term_slug]}, + expected_status=HTTP_400_BAD_REQUEST) + + def test_learning_resource_types(self): + """ + Get from learning_resource_types + """ + base_url = "{}learning_resource_types/".format(API_BASE) + + resp = self.client.get(base_url) + self.assertEqual(HTTP_200_OK, resp.status_code) + types = as_json(resp) + + self.assertEqual(sorted([lrt.name for lrt + in LearningResourceType.objects.all()]), + sorted([t['name'] for t in types['results']])) + + # nothing besides GET, OPTION, HEAD allowed + resp = self.client.options(base_url) + self.assertEqual(HTTP_200_OK, resp.status_code) + resp = self.client.head(base_url) + self.assertEqual(HTTP_200_OK, resp.status_code) + resp = self.client.post(base_url, {}) + self.assertEqual(HTTP_405_METHOD_NOT_ALLOWED, resp.status_code) + resp = self.client.patch(base_url, {}) + self.assertEqual(HTTP_405_METHOD_NOT_ALLOWED, resp.status_code) + resp = self.client.put(base_url, {}) + self.assertEqual(HTTP_405_METHOD_NOT_ALLOWED, resp.status_code) + + # restricted to logged in users + self.logout() + resp = self.client.get(base_url) + self.assertEqual(HTTP_403_FORBIDDEN, resp.status_code) + + # but otherwise unrestricted + self.login(self.user_norepo) + resp = self.client.get(base_url) + self.assertEqual(HTTP_200_OK, resp.status_code) + types = as_json(resp) + + self.assertEqual(sorted([lrt.name for lrt + in LearningResourceType.objects.all()]), + sorted([t['name'] for t in types['results']])) + + def test_preview_url(self): + """ + Assert preview url behavior for learning resources + """ + learning_resource = LearningResource.objects.first() + expected_jump_to_id_url = ( + "https://www.sandbox.edx.org/courses/" + "test-org/infinity/Febtober/jump_to_id/url_name1" + ) + self.assertEqual( + expected_jump_to_id_url, + learning_resource.get_preview_url() + ) + + resource_dict = self.get_learning_resource( + self.repo.slug, learning_resource.id) + self.assertEqual( + expected_jump_to_id_url, resource_dict['preview_url']) + + learning_resource.url_name = None + self.assertEqual( + "https://www.sandbox.edx.org/courses/" + "test-org/infinity/Febtober/courseware", + learning_resource.get_preview_url() + ) + + +# pylint: disable=too-many-ancestors +class TestLearningResourceAuthorization(RESTAuthTestCase): + """ + REST tests for authorization of learning resources and static assets + """ + + def test_resource_get(self): + """Test retrieve learning resource""" + self.assertEqual( + 1, self.get_learning_resources(self.repo.slug)['count']) + + resource = self.import_course_tarball(self.repo) + lr_id = resource.id + + # author_user has view_repo permissions + self.logout() + self.login(self.author_user.username) + self.assertEqual( + 1 + self.toy_resource_count, + self.get_learning_resources(self.repo.slug)['count'] + ) + self.get_learning_resource(self.repo.slug, lr_id) + + # user_norepo has no view_repo permission + self.logout() + self.login(self.user_norepo.username) + self.get_learning_resources( + self.repo.slug, expected_status=HTTP_403_FORBIDDEN) + self.get_learning_resource( + self.repo.slug, lr_id, expected_status=HTTP_403_FORBIDDEN) + + # as anonymous + self.logout() + self.get_learning_resources( + self.repo.slug, expected_status=HTTP_403_FORBIDDEN) + self.get_learning_resource( + self.repo.slug, lr_id, expected_status=HTTP_403_FORBIDDEN) + + def test_resource_post(self): + """Test create learning resource""" + resp = self.client.post( + '{repo_base}{repo_slug}/learning_resources/'.format( + repo_base=REPO_BASE, + repo_slug=self.repo.slug, + ), + self.DEFAULT_LR_DICT + ) + self.assertEqual(resp.status_code, HTTP_405_METHOD_NOT_ALLOWED) + + def test_resource_delete(self): + """Test delete learning resource""" + resource = self.import_course_tarball(self.repo) + lr_id = resource.id + + resp = self.client.delete( + '{repo_base}{repo_slug}/learning_resources/{lr_id}/'.format( + repo_base=REPO_BASE, + repo_slug=self.repo.slug, + lr_id=lr_id, + ), + self.DEFAULT_LR_DICT + ) + self.assertEqual(resp.status_code, HTTP_405_METHOD_NOT_ALLOWED) + + def test_resource_put_patch(self): + """Test update learning resource""" + resource = self.import_course_tarball(self.repo) + lr_id = resource.id + + new_description_dict = {"description": "new description"} + + # as curator who has add_edit_metadata permission + self.logout() + self.login(self.curator_user.username) + self.patch_learning_resource( + self.repo.slug, lr_id, new_description_dict + ) + new_description_dict['description'] += "_" + new_description_dict['terms'] = [] + self.put_learning_resource( + self.repo.slug, lr_id, new_description_dict + ) + + # author does have add_edit_metadata permission + self.logout() + self.login(self.author_user.username) + new_description_dict['description'] += "_" + self.patch_learning_resource( + self.repo.slug, lr_id, new_description_dict + ) + new_description_dict['description'] += "_" + self.put_learning_resource( + self.repo.slug, lr_id, new_description_dict + ) + + # as anonymous + self.logout() + new_description_dict['description'] += "_" + self.patch_learning_resource( + self.repo.slug, lr_id, new_description_dict, + expected_status=HTTP_403_FORBIDDEN + ) + self.put_learning_resource( + self.repo.slug, lr_id, new_description_dict, + expected_status=HTTP_403_FORBIDDEN + ) + + def test_static_assets_get(self): + """Test for getting static assets from learning_resources""" + def get_resource_with_asset(type_name): + """ + Get a LearningResource with a StaticAsset. + """ + for resource in get_resources(self.repo.id): + if (resource.learning_resource_type.name != type_name or + resource.static_assets.count() == 0): + continue + return resource + self.import_course_tarball(self.repo) + # Most static assets within a course will be associated with multiple + # resources due to the hierarchy. For the test course, we know that + # there is no overlap between the html and video, making them + # suitable for this test. + resource1 = get_resource_with_asset("html") + resource2 = get_resource_with_asset("video") + static_asset1 = resource1.static_assets.first() + static_asset2 = resource2.static_assets.first() + lr1_id = resource1.id + + # add a second StaticAsset to another learning resource + resource2.static_assets.add(static_asset2) + lr2_id = resource2.id + # make sure the result for an asset contains a name and an url + resp = self.get_static_asset(self.repo.slug, lr1_id, static_asset1.id) + self.assertTrue('asset' in resp) + self.assertTrue('name' in resp) + self.assertTrue(static_asset1.asset.url in resp['asset']) + self.assertEqual( + resp['name'], + os.path.basename(static_asset1.asset.name) + ) + + # make sure static assets only show up in their proper places + self.get_static_asset(self.repo.slug, lr1_id, static_asset1.id) + self.assertEqual( + 2, self.get_static_assets(self.repo.slug, lr1_id)['count']) + self.get_static_asset(self.repo.slug, lr1_id, static_asset2.id, + expected_status=HTTP_404_NOT_FOUND) + self.get_static_asset(self.repo.slug, lr2_id, static_asset1.id, + expected_status=HTTP_404_NOT_FOUND) + self.get_static_asset(self.repo.slug, lr2_id, static_asset2.id) + self.assertEqual( + 1, self.get_static_assets(self.repo.slug, lr2_id)['count']) + + # author_user has view_repo permissions + self.logout() + self.login(self.author_user.username) + self.assertEqual( + 2, self.get_static_assets(self.repo.slug, lr1_id)['count']) + self.get_static_asset(self.repo.slug, lr1_id, static_asset1.id) + + # user_norepo has no view_repo permission + self.logout() + self.login(self.user_norepo.username) + self.get_static_assets(self.repo.slug, lr1_id, + expected_status=HTTP_403_FORBIDDEN) + self.get_static_asset(self.repo.slug, lr1_id, static_asset1.id, + expected_status=HTTP_403_FORBIDDEN) + + # as anonymous + self.logout() + self.get_static_assets(self.repo.slug, lr1_id, + expected_status=HTTP_403_FORBIDDEN) + self.get_static_asset(self.repo.slug, lr1_id, static_asset1.id, + expected_status=HTTP_403_FORBIDDEN) + + def test_static_assets_create(self): + """Test for creating static assets from learning_resources""" + lr_id = get_resources(self.repo.id).first().id + resp = self.client.post( + '{repo_base}{repo_slug}/learning_resources/' + '{lr_id}/static_assets/'.format( + repo_base=REPO_BASE, + repo_slug=self.repo.slug, + lr_id=lr_id, + ), + self.DEFAULT_LR_DICT + ) + self.assertEqual(resp.status_code, HTTP_405_METHOD_NOT_ALLOWED) + + def test_static_assets_put_patch(self): + """Test for updating static assets from learning_resources""" + resource = self.import_course_tarball(self.repo) + static_asset = resource.static_assets.first() + lr_id = resource.id + + resp = self.client.put( + '{repo_base}{repo_slug}/learning_resources/' + '{lr_id}/static_assets/{sa_id}/'.format( + repo_base=REPO_BASE, + repo_slug=self.repo.slug, + lr_id=lr_id, + sa_id=static_asset.id, + ), + self.DEFAULT_LR_DICT + ) + self.assertEqual(resp.status_code, HTTP_405_METHOD_NOT_ALLOWED) + resp = self.client.patch( + '{repo_base}{repo_slug}/learning_resources/' + '{lr_id}/static_assets/{sa_id}/'.format( + repo_base=REPO_BASE, + repo_slug=self.repo.slug, + lr_id=lr_id, + sa_id=static_asset.id, + ), + self.DEFAULT_LR_DICT + ) + self.assertEqual(resp.status_code, HTTP_405_METHOD_NOT_ALLOWED) + + def test_static_assets_delete(self): + """Test for deleting static assets from learning_resources""" + resource = self.import_course_tarball(self.repo) + static_asset = resource.static_assets.first() + lr_id = resource.id + + resp = self.client.delete( + '{repo_base}{repo_slug}/learning_resources/' + '{lr_id}/static_assets/{sa_id}/'.format( + repo_base=REPO_BASE, + repo_slug=self.repo.slug, + lr_id=lr_id, + sa_id=static_asset.id, + ), + self.DEFAULT_LR_DICT + ) + self.assertEqual(resp.status_code, HTTP_405_METHOD_NOT_ALLOWED) diff --git a/rest/tests/test_rest_members.py b/rest/tests/test_members.py similarity index 71% rename from rest/tests/test_rest_members.py rename to rest/tests/test_members.py index 9800156a..a81967f0 100644 --- a/rest/tests/test_rest_members.py +++ b/rest/tests/test_members.py @@ -8,6 +8,7 @@ from django.contrib.auth.models import User from rest_framework.status import ( HTTP_400_BAD_REQUEST, + HTTP_403_FORBIDDEN, HTTP_404_NOT_FOUND, ) @@ -17,7 +18,10 @@ list_users_in_repo, ) from roles.permissions import GroupTypes, BaseGroupTypes -from rest.tests.base import RESTTestCase +from rest.tests.base import ( + RESTTestCase, + RESTAuthTestCase, +) class TestMembers(RESTTestCase): @@ -579,3 +583,170 @@ def test_members_delete(self): group_type=BaseGroupTypes.ADMINISTRATORS ) self.assert_users_count(admins=1) + + +# pylint: disable=too-many-ancestors +class TestMembersAuthorization(RESTAuthTestCase): + """Tests for member authorization via REST""" + + def test_members_get(self): + """ + Tests for members. + Get requests: an user can see members if has at least basic permissions + """ + # add an user to all groups + for group_type in [GroupTypes.REPO_ADMINISTRATOR, + GroupTypes.REPO_CURATOR, GroupTypes.REPO_AUTHOR]: + assign_user_to_repo_group( + self.user, self.repo, group_type) + + self.logout() + # as anonymous + self.get_members(urlfor='base', repo_slug=self.repo.slug, + expected_status=HTTP_403_FORBIDDEN) + # list of all groups for an user + self.get_members(urlfor='users', repo_slug=self.repo.slug, + username=self.user.username, + expected_status=HTTP_403_FORBIDDEN) + for group_type in BaseGroupTypes.all_base_groups(): + # specific group for an user + self.get_members(urlfor='users', repo_slug=self.repo.slug, + username=self.user.username, + group_type=group_type, + expected_status=HTTP_403_FORBIDDEN) + # list of all users for a group + self.get_members(urlfor='groups', repo_slug=self.repo.slug, + group_type=group_type, + expected_status=HTTP_403_FORBIDDEN) + # specific user for a group + self.get_members(urlfor='groups', repo_slug=self.repo.slug, + username=self.user.username, + group_type=group_type, + expected_status=HTTP_403_FORBIDDEN) + + # any kind of user in the repo groups can retrieve infos + for user in [self.author_user.username, self.curator_user.username, + self.user.username]: + self.logout() + self.login(user) + # list of all groups for an user + self.get_members(urlfor='base', repo_slug=self.repo.slug) + # specific group for an user + self.get_members(urlfor='users', repo_slug=self.repo.slug, + username=self.user.username) + for group_type in BaseGroupTypes.all_base_groups(): + self.get_members(urlfor='users', repo_slug=self.repo.slug, + username=self.user.username, + group_type=group_type) + # list of all users for a group + self.get_members(urlfor='groups', repo_slug=self.repo.slug, + group_type=group_type) + # specific user for a group + self.get_members(urlfor='groups', repo_slug=self.repo.slug, + username=self.user.username, + group_type=group_type) + + def test_members_create(self): + """ + Tests for members. + Post requests: an user can create members only if s/he is admin + The only URLS where users can be assigned to group or vice versa are + /api/v1/repositories//members/groups//users/ + /api/v1/repositories//members/users//groups/ + """ + self.logout() + mem_dict_user = {'group_type': 'administrators'} + mem_dict_groups = {'username': self.user_norepo.username} + # as anonymous + self.create_member(urlfor='users', repo_slug=self.repo.slug, + mem_dict=mem_dict_user, username=self.user.username, + expected_status=HTTP_403_FORBIDDEN) + for group_type in BaseGroupTypes.all_base_groups(): + self.create_member(urlfor='groups', repo_slug=self.repo.slug, + mem_dict=mem_dict_groups, + group_type=group_type, + expected_status=HTTP_403_FORBIDDEN) + # as author + self.login(self.author_user.username) + self.create_member(urlfor='users', repo_slug=self.repo.slug, + mem_dict=mem_dict_user, username=self.user.username, + expected_status=HTTP_403_FORBIDDEN) + for group_type in BaseGroupTypes.all_base_groups(): + self.create_member(urlfor='groups', repo_slug=self.repo.slug, + mem_dict=mem_dict_groups, + group_type=group_type, + expected_status=HTTP_403_FORBIDDEN) + # as curator + self.logout() + self.login(self.curator_user.username) + self.create_member(urlfor='users', repo_slug=self.repo.slug, + mem_dict=mem_dict_user, username=self.user.username, + expected_status=HTTP_403_FORBIDDEN) + for group_type in BaseGroupTypes.all_base_groups(): + self.create_member(urlfor='groups', repo_slug=self.repo.slug, + mem_dict=mem_dict_groups, + group_type=group_type, + expected_status=HTTP_403_FORBIDDEN) + # as administrator + self.logout() + self.login(self.user.username) + self.create_member(urlfor='users', repo_slug=self.repo.slug, + mem_dict=mem_dict_user, username=self.user.username) + for group_type in BaseGroupTypes.all_base_groups(): + self.create_member(urlfor='groups', repo_slug=self.repo.slug, + mem_dict=mem_dict_groups, + group_type=group_type) + + def test_members_delete(self): + """ + Tests for members. + Delete requests: an user can delete members only if s/he is admin + The only URLS where users can be deleted from a group or vice versa are + /api/v1/repositories//members/groups//users/ + /api/v1/repositories//members/users//groups/ + """ + for group_type in BaseGroupTypes.all_base_groups(): + # as anonymous + self.logout() + self.delete_member(urlfor='users', repo_slug=self.repo.slug, + username=self.user.username, + group_type=group_type, + expected_status=HTTP_403_FORBIDDEN) + self.delete_member(urlfor='groups', repo_slug=self.repo.slug, + username=self.user.username, + group_type=group_type, + expected_status=HTTP_403_FORBIDDEN) + # as author + self.login(self.author_user.username) + self.delete_member(urlfor='users', repo_slug=self.repo.slug, + username=self.user.username, + group_type=group_type, + expected_status=HTTP_403_FORBIDDEN) + self.delete_member(urlfor='groups', repo_slug=self.repo.slug, + username=self.user.username, + group_type=group_type, + expected_status=HTTP_403_FORBIDDEN) + # as curator + self.logout() + self.login(self.curator_user.username) + self.delete_member(urlfor='users', repo_slug=self.repo.slug, + username=self.user.username, + group_type=group_type, + expected_status=HTTP_403_FORBIDDEN) + self.delete_member(urlfor='groups', repo_slug=self.repo.slug, + username=self.user.username, + group_type=group_type, + expected_status=HTTP_403_FORBIDDEN) + # different loop because the actual deletion can impact the other tests + for group_type in BaseGroupTypes.all_base_groups(): + # as administrator + # deleting a different username because deleting self from admin is + # a special case (handled in different tests) + self.logout() + self.login(self.user.username) + self.delete_member(urlfor='users', repo_slug=self.repo.slug, + username=self.author_user.username, + group_type=group_type) + self.delete_member(urlfor='groups', repo_slug=self.repo.slug, + username=self.author_user.username, + group_type=group_type) diff --git a/rest/tests/test_misc.py b/rest/tests/test_misc.py new file mode 100644 index 00000000..416a691c --- /dev/null +++ b/rest/tests/test_misc.py @@ -0,0 +1,50 @@ +""" +Unit tests for REST api +""" +from __future__ import unicode_literals + +from rest_framework.status import ( + HTTP_404_NOT_FOUND, +) + +from rest.tests.base import ( + RESTTestCase, + API_BASE, +) + + +class TestMisc(RESTTestCase): + """ + REST test + """ + + def test_root(self): + """ + Test root of API + """ + resp = self.client.get(API_BASE) + self.assertEqual(HTTP_404_NOT_FOUND, resp.status_code) + resp = self.client.post(API_BASE) + self.assertEqual(HTTP_404_NOT_FOUND, resp.status_code) + resp = self.client.head(API_BASE) + self.assertEqual(HTTP_404_NOT_FOUND, resp.status_code) + resp = self.client.patch(API_BASE) + self.assertEqual(HTTP_404_NOT_FOUND, resp.status_code) + resp = self.client.put(API_BASE) + self.assertEqual(HTTP_404_NOT_FOUND, resp.status_code) + resp = self.client.options(API_BASE) + self.assertEqual(HTTP_404_NOT_FOUND, resp.status_code) + + self.logout() + resp = self.client.get(API_BASE) + self.assertEqual(HTTP_404_NOT_FOUND, resp.status_code) + resp = self.client.post(API_BASE) + self.assertEqual(HTTP_404_NOT_FOUND, resp.status_code) + resp = self.client.head(API_BASE) + self.assertEqual(HTTP_404_NOT_FOUND, resp.status_code) + resp = self.client.patch(API_BASE) + self.assertEqual(HTTP_404_NOT_FOUND, resp.status_code) + resp = self.client.put(API_BASE) + self.assertEqual(HTTP_404_NOT_FOUND, resp.status_code) + resp = self.client.options(API_BASE) + self.assertEqual(HTTP_404_NOT_FOUND, resp.status_code) diff --git a/rest/tests/test_repository.py b/rest/tests/test_repository.py new file mode 100644 index 00000000..1a179e31 --- /dev/null +++ b/rest/tests/test_repository.py @@ -0,0 +1,321 @@ +""" +Repository REST tests +""" + +from __future__ import unicode_literals + +from rest_framework.status import ( + HTTP_200_OK, + HTTP_400_BAD_REQUEST, + HTTP_403_FORBIDDEN, + HTTP_404_NOT_FOUND, + HTTP_405_METHOD_NOT_ALLOWED, +) + +from rest.tests.base import ( + RESTAuthTestCase, + RESTTestCase, + REPO_BASE, + as_json, +) +from rest.serializers import RepositorySerializer +from learningresources.models import Repository + + +# pylint: disable=invalid-name +class TestRepository(RESTTestCase): + """Repository REST tests""" + + def test_repositories(self): + """ + Test for Repository + """ + repositories = self.get_repositories() + self.assertEqual(1, repositories['count']) + + self.create_repository() + repositories = self.get_repositories() + self.assertEqual(2, repositories['count']) + repo_slug = repositories['results'][1]['slug'] + + # test PUT and PATCH + input_dict = { + 'name': 'name', + 'description': 'description', + } + input_dict_changed = { + 'name': 'name', + 'description': 'changed description', + } + + # patch with missing slug + self.patch_repository( + "missing", {'name': 'rename'}, + expected_status=HTTP_404_NOT_FOUND + ) + + self.patch_repository( + repo_slug, {'name': 'rename'}, + expected_status=HTTP_405_METHOD_NOT_ALLOWED) + + # put with missing slug + self.put_repository("missing slug", input_dict, + expected_status=HTTP_404_NOT_FOUND) + + self.put_repository( + repo_slug, input_dict, + expected_status=HTTP_405_METHOD_NOT_ALLOWED) + + # verify change of data + self.put_repository( + repo_slug, input_dict_changed, + expected_status=HTTP_405_METHOD_NOT_ALLOWED) + + # never created a duplicate + self.assertEqual(2, Repository.objects.count()) + + # call to verify HTTP_OK + self.get_repository(repo_slug) + + # delete a repository + self.delete_repository(repo_slug, + expected_status=HTTP_405_METHOD_NOT_ALLOWED) + + repositories = self.get_repositories() + self.assertEqual(2, repositories['count']) + + vocab_slug = self.create_vocabulary(self.repo.slug)['slug'] + vocabularies = self.get_vocabularies(self.repo.slug) + self.assertEqual(1, vocabularies['count']) + + self.delete_repository(self.repo.slug, + expected_status=HTTP_405_METHOD_NOT_ALLOWED) + + self.delete_vocabulary(self.repo.slug, vocab_slug) + + vocabularies = self.get_vocabularies(self.repo.slug) + self.assertEqual(0, vocabularies['count']) + + self.delete_repository(self.repo.slug, + expected_status=HTTP_405_METHOD_NOT_ALLOWED) + + self.get_repositories() + self.get_repository("missing", expected_status=HTTP_404_NOT_FOUND) + + # as anonymous + self.logout() + self.get_repositories(expected_status=HTTP_403_FORBIDDEN) + self.get_repository("missing", expected_status=HTTP_403_FORBIDDEN) + + def test_repository_pagination(self): + """Test pagination for collections""" + + expected = [ + self.create_repository( + { + 'name': "name{i}".format(i=i), + "description": "description" + } + ) for i in range(40)] + + repositories = self.get_repositories() + + # 40 we created + self.repo + self.assertEqual(41, repositories['count']) + self.assertEqual( + [RepositorySerializer( + Repository.objects.get(id=x['id'])).data + for x in expected[:19]], + repositories['results'][1:20]) + + resp = self.client.get( + '{repo_base}?page=2'.format( + repo_base=REPO_BASE, + )) + self.assertEqual(HTTP_200_OK, resp.status_code) + repositories = as_json(resp) + self.assertEqual(41, repositories['count']) + self.assertEqual( + [RepositorySerializer(Repository.objects.get(id=x['id'])).data + for x in expected[19:39]], repositories['results']) + + def test_immutable_fields_repository(self): + """Test repository immutable fields""" + repo_dict = { + 'id': -1, + 'slug': 'sluggy', + 'name': 'other name', + 'description': 'description', + 'date_created': "2015-01-01", + 'created_by': self.user_norepo.id, + } + + def assert_not_changed(new_dict): + """Check that fields have not changed""" + # These keys should be different since they are immutable or set by + # the serializer. + for field in ('id', 'slug', 'date_created'): + self.assertNotEqual(repo_dict[field], new_dict[field]) + + # created_by is set internally and should not show up in output. + self.assertNotIn('created_by', new_dict) + repository = Repository.objects.get(slug=new_dict['slug']) + self.assertEqual(repository.created_by.id, self.user.id) + + assert_not_changed(self.create_repository(repo_dict, skip_assert=True)) + + +# pylint: disable=too-many-ancestors +class TestRepositoryAuthorization(RESTAuthTestCase): + """Repository authorization REST tests""" + + def test_repository_create(self): + """Test create repository authorization""" + # switch to add_repo_user + self.logout() + self.login(self.add_repo_user.username) + + # add_repo_user creates a repository + new_repo_slug = self.create_repository()['slug'] + + self.get_repository(new_repo_slug) + self.get_repository(self.repo.slug, expected_status=HTTP_403_FORBIDDEN) + repos = self.get_repositories() + self.assertEqual(1, repos['count']) + + # duplicate repository, now that we assigned another repo + # with same name to user + self.create_repository(expected_status=HTTP_400_BAD_REQUEST) + + # switch to curator_user + self.logout() + self.login(self.curator_user.username) + + self.create_repository({ + 'name': 'newname', + 'description': 'description', + }, expected_status=HTTP_403_FORBIDDEN) + + # switch to user with no permissions for repo + self.logout() + self.login(self.user_norepo.username) + + self.create_repository({ + 'name': 'newname', + 'description': 'description', + }, expected_status=HTTP_403_FORBIDDEN) + + # as anonymous + self.logout() + self.create_repository({ + 'name': 'newname', + 'description': 'description', + }, expected_status=HTTP_403_FORBIDDEN) + + def test_repository_patch_put(self): + """Test updating repository""" + # switch to add_repo_user + self.logout() + self.login(self.add_repo_user.username) + + input_dict = { + 'name': self.repo.name, + 'description': 'new description' + } + self.patch_repository(self.repo.slug, input_dict, + expected_status=HTTP_403_FORBIDDEN) + self.put_repository(self.repo.slug, input_dict, + expected_status=HTTP_403_FORBIDDEN) + + # switch to curator_user + self.logout() + self.login(self.curator_user.username) + self.patch_repository(self.repo.slug, input_dict, + expected_status=HTTP_405_METHOD_NOT_ALLOWED) + self.put_repository(self.repo.slug, input_dict, + expected_status=HTTP_405_METHOD_NOT_ALLOWED) + + # switch to user with no permissions for repo + self.logout() + self.login(self.user_norepo.username) + self.patch_repository(self.repo.slug, input_dict, + expected_status=HTTP_403_FORBIDDEN) + self.put_repository(self.repo.slug, input_dict, + expected_status=HTTP_403_FORBIDDEN) + + # as anonymous + self.logout() + self.patch_repository(self.repo.slug, input_dict, + expected_status=HTTP_403_FORBIDDEN) + self.put_repository(self.repo.slug, input_dict, + expected_status=HTTP_403_FORBIDDEN) + + def test_repository_delete(self): + """Test deleting repository""" + # switch to add_repo_user + self.logout() + self.login(self.add_repo_user.username) + + self.delete_repository(self.repo.slug, + expected_status=HTTP_403_FORBIDDEN) + + self.logout() + self.login(self.curator_user.username) + + # curator does have view_repo but nobody is allowed + # to delete repositories + self.delete_repository(self.repo.slug, + expected_status=HTTP_405_METHOD_NOT_ALLOWED) + + # switch to user with no permissions for repo + self.logout() + self.login(self.user_norepo.username) + self.delete_repository(self.repo.slug, + expected_status=HTTP_403_FORBIDDEN) + + # as anonymous + self.logout() + self.delete_repository(self.repo.slug, + expected_status=HTTP_403_FORBIDDEN) + + def test_repository_get(self): + """Test list and view repository""" + + # switch to add_repo_user + self.logout() + self.login(self.add_repo_user.username) + + # add_repo_user shouldn't be able to see any repositories yet + self.get_repository(self.repo.slug, expected_status=HTTP_403_FORBIDDEN) + repos = self.get_repositories() + self.assertEqual(0, repos['count']) + + # add_repo_user creates a repository + new_repo_slug = self.create_repository()['slug'] + + self.get_repository(new_repo_slug) + repos = self.get_repositories() + self.assertEqual(1, repos['count']) + + # switch to curator_user + self.logout() + self.login(self.curator_user.username) + + self.assertEqual(1, self.get_repositories()['count']) + self.get_repository(new_repo_slug, + expected_status=HTTP_403_FORBIDDEN) + self.get_repository(self.repo.slug) + + # switch to user_norepo + self.logout() + self.login(self.user_norepo.username) + + self.assertEqual(0, self.get_repositories()['count']) + self.get_repository(self.repo.slug, + expected_status=HTTP_403_FORBIDDEN) + + # as anonymous + self.logout() + self.get_repositories(expected_status=HTTP_403_FORBIDDEN) + self.get_repository(self.repo.slug, + expected_status=HTTP_403_FORBIDDEN) diff --git a/rest/tests/test_rest_authorization.py b/rest/tests/test_rest_authorization.py deleted file mode 100644 index aa5169ce..00000000 --- a/rest/tests/test_rest_authorization.py +++ /dev/null @@ -1,883 +0,0 @@ -""" -Tests for REST authorization -""" - -from __future__ import unicode_literals - -import logging -import os - -from rest_framework.status import ( - HTTP_400_BAD_REQUEST, - HTTP_403_FORBIDDEN, - HTTP_404_NOT_FOUND, - HTTP_405_METHOD_NOT_ALLOWED, -) -from django.contrib.auth.models import User, Permission - -from rest.tests.base import RESTTestCase, REPO_BASE -from learningresources.api import get_resources -from roles.api import assign_user_to_repo_group -from roles.permissions import GroupTypes, BaseGroupTypes - -log = logging.getLogger(__name__) - - -# pylint: disable=too-many-public-methods -class TestRestAuthorization(RESTTestCase): - """ - Tests for REST authorization - """ - - def setUp(self): - super(TestRestAuthorization, self).setUp() - - # add_repo_user is another user with add_repo permission but who - # does not have access to self.repo - self.add_repo_user = User.objects.create_user( - username="creator_user", password=self.PASSWORD - ) - add_repo_perm = Permission.objects.get(codename=self.ADD_REPO_PERM) - self.add_repo_user.user_permissions.add(add_repo_perm) - - # curator_user doesn't have add_repo but have view_repo permission on - # self.repo - self.curator_user = User.objects.create_user( - username="curator_user", password=self.PASSWORD - ) - assign_user_to_repo_group( - self.curator_user, self.repo, GroupTypes.REPO_CURATOR) - - # author_user doesn't have manage_taxonomy permission - self.author_user = User.objects.create_user( - username="author_user", password=self.PASSWORD - ) - assign_user_to_repo_group( - self.author_user, self.repo, GroupTypes.REPO_AUTHOR - ) - - def test_repository_create(self): - """Test create repository authorization""" - # switch to add_repo_user - self.logout() - self.login(self.add_repo_user.username) - - # add_repo_user creates a repository - new_repo_slug = self.create_repository()['slug'] - - self.get_repository(new_repo_slug) - self.get_repository(self.repo.slug, expected_status=HTTP_403_FORBIDDEN) - repos = self.get_repositories() - self.assertEqual(1, repos['count']) - - # duplicate repository, now that we assigned another repo - # with same name to user - self.create_repository(expected_status=HTTP_400_BAD_REQUEST) - - # switch to curator_user - self.logout() - self.login(self.curator_user.username) - - self.create_repository({ - 'name': 'newname', - 'description': 'description', - }, expected_status=HTTP_403_FORBIDDEN) - - # switch to user with no permissions for repo - self.logout() - self.login(self.user_norepo.username) - - self.create_repository({ - 'name': 'newname', - 'description': 'description', - }, expected_status=HTTP_403_FORBIDDEN) - - # as anonymous - self.logout() - self.create_repository({ - 'name': 'newname', - 'description': 'description', - }, expected_status=HTTP_403_FORBIDDEN) - - def test_repository_patch_put(self): - """Test updating repository""" - # switch to add_repo_user - self.logout() - self.login(self.add_repo_user.username) - - input_dict = { - 'name': self.repo.name, - 'description': 'new description' - } - self.patch_repository(self.repo.slug, input_dict, - expected_status=HTTP_403_FORBIDDEN) - self.put_repository(self.repo.slug, input_dict, - expected_status=HTTP_403_FORBIDDEN) - - # switch to curator_user - self.logout() - self.login(self.curator_user.username) - self.patch_repository(self.repo.slug, input_dict, - expected_status=HTTP_405_METHOD_NOT_ALLOWED) - self.put_repository(self.repo.slug, input_dict, - expected_status=HTTP_405_METHOD_NOT_ALLOWED) - - # switch to user with no permissions for repo - self.logout() - self.login(self.user_norepo.username) - self.patch_repository(self.repo.slug, input_dict, - expected_status=HTTP_403_FORBIDDEN) - self.put_repository(self.repo.slug, input_dict, - expected_status=HTTP_403_FORBIDDEN) - - # as anonymous - self.logout() - self.patch_repository(self.repo.slug, input_dict, - expected_status=HTTP_403_FORBIDDEN) - self.put_repository(self.repo.slug, input_dict, - expected_status=HTTP_403_FORBIDDEN) - - def test_repository_delete(self): - """Test deleting repository""" - # switch to add_repo_user - self.logout() - self.login(self.add_repo_user.username) - - self.delete_repository(self.repo.slug, - expected_status=HTTP_403_FORBIDDEN) - - self.logout() - self.login(self.curator_user.username) - - # curator does have view_repo but nobody is allowed - # to delete repositories - self.delete_repository(self.repo.slug, - expected_status=HTTP_405_METHOD_NOT_ALLOWED) - - # switch to user with no permissions for repo - self.logout() - self.login(self.user_norepo.username) - self.delete_repository(self.repo.slug, - expected_status=HTTP_403_FORBIDDEN) - - # as anonymous - self.logout() - self.delete_repository(self.repo.slug, - expected_status=HTTP_403_FORBIDDEN) - - def test_repository_get(self): - """Test list and view repository""" - - # switch to add_repo_user - self.logout() - self.login(self.add_repo_user.username) - - # add_repo_user shouldn't be able to see any repositories yet - self.get_repository(self.repo.slug, expected_status=HTTP_403_FORBIDDEN) - repos = self.get_repositories() - self.assertEqual(0, repos['count']) - - # add_repo_user creates a repository - new_repo_slug = self.create_repository()['slug'] - - self.get_repository(new_repo_slug) - repos = self.get_repositories() - self.assertEqual(1, repos['count']) - - # switch to curator_user - self.logout() - self.login(self.curator_user.username) - - self.assertEqual(1, self.get_repositories()['count']) - self.get_repository(new_repo_slug, - expected_status=HTTP_403_FORBIDDEN) - self.get_repository(self.repo.slug) - - # switch to user_norepo - self.logout() - self.login(self.user_norepo.username) - - self.assertEqual(0, self.get_repositories()['count']) - self.get_repository(self.repo.slug, - expected_status=HTTP_403_FORBIDDEN) - - # as anonymous - self.logout() - self.get_repositories(expected_status=HTTP_403_FORBIDDEN) - self.get_repository(self.repo.slug, - expected_status=HTTP_403_FORBIDDEN) - - def test_vocabulary_create(self): - """Test create vocabulary""" - self.logout() - self.login(self.curator_user.username) - - self.create_vocabulary(self.repo.slug) - self.assertEqual(1, self.get_vocabularies(self.repo.slug)['count']) - - # login as author which doesn't have manage_taxonomy permissions - self.logout() - self.login(self.author_user.username) - - vocab_dict = dict(self.DEFAULT_VOCAB_DICT) - vocab_dict['name'] = 'other_name' # prevent name collision - - # author does not have create access - self.create_vocabulary( - self.repo.slug, vocab_dict, - expected_status=HTTP_403_FORBIDDEN - ) - - # make sure at least view_repo is needed - self.logout() - self.login(self.user_norepo.username) - - self.create_vocabulary(self.repo.slug, vocab_dict, - expected_status=HTTP_403_FORBIDDEN) - - # as anonymous - self.logout() - self.create_vocabulary(self.repo.slug, vocab_dict, - expected_status=HTTP_403_FORBIDDEN) - - def test_vocabulary_put_patch(self): - """Test update vocabulary""" - vocab_slug = self.create_vocabulary(self.repo.slug)['slug'] - - self.logout() - self.login(self.curator_user.username) - - self.patch_vocabulary(self.repo.slug, vocab_slug, - self.DEFAULT_VOCAB_DICT) - self.put_vocabulary(self.repo.slug, vocab_slug, - self.DEFAULT_VOCAB_DICT) - - # login as author which doesn't have manage_taxonomy permissions - self.logout() - self.login(self.author_user.username) - - self.patch_vocabulary(self.repo.slug, vocab_slug, - self.DEFAULT_VOCAB_DICT, - expected_status=HTTP_403_FORBIDDEN) - self.put_vocabulary(self.repo.slug, vocab_slug, - self.DEFAULT_VOCAB_DICT, - expected_status=HTTP_403_FORBIDDEN) - - # make sure at least view_repo is needed - self.logout() - self.login(self.user_norepo.username) - self.patch_vocabulary(self.repo.slug, vocab_slug, - self.DEFAULT_VOCAB_DICT, - expected_status=HTTP_403_FORBIDDEN) - self.put_vocabulary(self.repo.slug, vocab_slug, - self.DEFAULT_VOCAB_DICT, - expected_status=HTTP_403_FORBIDDEN) - - # as anonymous - self.logout() - self.patch_vocabulary(self.repo.slug, vocab_slug, - self.DEFAULT_VOCAB_DICT, - expected_status=HTTP_403_FORBIDDEN) - self.put_vocabulary(self.repo.slug, vocab_slug, - self.DEFAULT_VOCAB_DICT, - expected_status=HTTP_403_FORBIDDEN) - - def test_vocabulary_delete(self): - """Test delete vocabulary""" - vocab_slug = self.create_vocabulary(self.repo.slug)['slug'] - - # login as author which doesn't have manage_taxonomy permissions - self.logout() - self.login(self.author_user.username) - - self.delete_vocabulary(self.repo.slug, vocab_slug, - expected_status=HTTP_403_FORBIDDEN) - - # curator does have manage_taxonomy permissions - self.logout() - self.login(self.curator_user.username) - self.delete_vocabulary(self.repo.slug, vocab_slug) - - # vocab is missing - self.create_term(self.repo.slug, vocab_slug, - expected_status=HTTP_404_NOT_FOUND) - self.delete_vocabulary(self.repo.slug, vocab_slug, - expected_status=HTTP_404_NOT_FOUND) - - # recreate vocab so we can delete it - vocab_slug = self.create_vocabulary(self.repo.slug)['slug'] - - # author has view_repo but delete not allowed - self.logout() - self.login(self.author_user.username) - self.delete_vocabulary(self.repo.slug, vocab_slug, - expected_status=HTTP_403_FORBIDDEN) - - # make sure at least view_repo is needed - self.logout() - self.login(self.user_norepo.username) - - self.delete_vocabulary(self.repo.slug, vocab_slug, - expected_status=HTTP_403_FORBIDDEN) - - # as anonymous - self.logout() - self.delete_vocabulary(self.repo.slug, vocab_slug, - expected_status=HTTP_403_FORBIDDEN) - - def test_vocabulary_get(self): - """Test get vocabulary and vocabularies""" - vocab_slug = self.create_vocabulary(self.repo.slug)['slug'] - - # author_user has view_repo permissions - self.logout() - self.login(self.author_user.username) - self.assertEqual(1, self.get_vocabularies(self.repo.slug)['count']) - self.get_vocabulary(self.repo.slug, vocab_slug) - - # user_norepo has no view_repo permission - self.logout() - self.login(self.user_norepo.username) - self.get_vocabularies( - self.repo.slug, expected_status=HTTP_403_FORBIDDEN) - self.get_vocabulary(self.repo.slug, vocab_slug, - expected_status=HTTP_403_FORBIDDEN) - - # as anonymous - self.logout() - self.get_vocabularies( - self.repo.slug, expected_status=HTTP_403_FORBIDDEN) - self.get_vocabulary(self.repo.slug, vocab_slug, - expected_status=HTTP_403_FORBIDDEN) - - def test_term_create(self): - """Test create term""" - vocab_slug = self.create_vocabulary(self.repo.slug)['slug'] - - # curator has manage_taxonomy permission - self.logout() - self.login(self.curator_user.username) - - self.create_term(self.repo.slug, vocab_slug) - self.assertEqual(1, self.get_terms( - self.repo.slug, vocab_slug)['count']) - - # login as author which doesn't have manage_taxonomy permissions - self.logout() - self.login(self.author_user.username) - - term_dict = dict(self.DEFAULT_TERM_DICT) - term_dict['label'] = 'other_name' # prevent name collision - - # author does not have create access - self.create_term( - self.repo.slug, vocab_slug, term_dict, - expected_status=HTTP_403_FORBIDDEN - ) - - # make sure at least view_repo is needed - self.logout() - self.login(self.user_norepo.username) - self.create_term( - self.repo.slug, vocab_slug, term_dict, - expected_status=HTTP_403_FORBIDDEN - ) - - # as anonymous - self.logout() - self.create_term( - self.repo.slug, vocab_slug, term_dict, - expected_status=HTTP_403_FORBIDDEN - ) - - def test_term_put_patch(self): - """Test update term""" - vocab_slug = self.create_vocabulary(self.repo.slug)['slug'] - term_slug = self.create_term(self.repo.slug, vocab_slug)['slug'] - - self.logout() - self.login(self.curator_user.username) - - self.patch_term(self.repo.slug, vocab_slug, term_slug, - self.DEFAULT_TERM_DICT) - self.put_term(self.repo.slug, vocab_slug, term_slug, - self.DEFAULT_TERM_DICT) - - # login as author which doesn't have manage_taxonomy permissions - self.logout() - self.login(self.author_user.username) - - self.patch_term(self.repo.slug, vocab_slug, term_slug, - self.DEFAULT_TERM_DICT, - expected_status=HTTP_403_FORBIDDEN) - self.put_term(self.repo.slug, vocab_slug, term_slug, - self.DEFAULT_TERM_DICT, - expected_status=HTTP_403_FORBIDDEN) - - # make sure at least view_repo is needed - self.logout() - self.login(self.user_norepo.username) - - self.patch_term(self.repo.slug, vocab_slug, term_slug, - self.DEFAULT_TERM_DICT, - expected_status=HTTP_403_FORBIDDEN) - self.put_term(self.repo.slug, vocab_slug, term_slug, - self.DEFAULT_TERM_DICT, - expected_status=HTTP_403_FORBIDDEN) - - # as anonymous - self.logout() - self.patch_term(self.repo.slug, vocab_slug, term_slug, - self.DEFAULT_TERM_DICT, - expected_status=HTTP_403_FORBIDDEN) - self.put_term(self.repo.slug, vocab_slug, term_slug, - self.DEFAULT_TERM_DICT, - expected_status=HTTP_403_FORBIDDEN) - - def test_term_delete(self): - """Test delete term""" - vocab_slug = self.create_vocabulary(self.repo.slug)['slug'] - term_slug = self.create_term(self.repo.slug, vocab_slug)['slug'] - - # login as author which doesn't have manage_taxonomy permissions - self.logout() - self.login(self.author_user.username) - - self.delete_term(self.repo.slug, vocab_slug, term_slug, - expected_status=HTTP_403_FORBIDDEN) - - # curator does have manage_taxonomy permissions - self.logout() - self.login(self.curator_user.username) - self.delete_term(self.repo.slug, vocab_slug, term_slug) - - # make sure at least view_repo is needed - self.logout() - self.login(self.user_norepo.username) - self.delete_term(self.repo.slug, vocab_slug, term_slug, - expected_status=HTTP_403_FORBIDDEN) - - # as anonymous - self.logout() - self.delete_term(self.repo.slug, vocab_slug, term_slug, - expected_status=HTTP_403_FORBIDDEN) - - def test_term_get(self): - """Test retrieve term""" - vocab_slug = self.create_vocabulary(self.repo.slug)['slug'] - term_slug = self.create_term(self.repo.slug, vocab_slug)['slug'] - - # author_user has view_repo permissions - self.logout() - self.login(self.author_user.username) - self.assertEqual(1, self.get_terms( - self.repo.slug, vocab_slug)['count']) - self.get_term(self.repo.slug, vocab_slug, term_slug) - - # user_norepo has no view_repo permission - self.logout() - self.login(self.user_norepo.username) - self.get_terms(self.repo.slug, vocab_slug, - expected_status=HTTP_403_FORBIDDEN) - self.get_term(self.repo.slug, vocab_slug, term_slug, - expected_status=HTTP_403_FORBIDDEN) - - # as anonymous - self.logout() - self.get_terms(self.repo.slug, vocab_slug, - expected_status=HTTP_403_FORBIDDEN) - self.get_term(self.repo.slug, vocab_slug, term_slug, - expected_status=HTTP_403_FORBIDDEN) - - def test_members_get(self): - """ - Tests for members. - Get requests: an user can see members if has at least basic permissions - """ - # add an user to all groups - for group_type in [GroupTypes.REPO_ADMINISTRATOR, - GroupTypes.REPO_CURATOR, GroupTypes.REPO_AUTHOR]: - assign_user_to_repo_group( - self.user, self.repo, group_type) - - self.logout() - # as anonymous - self.get_members(urlfor='base', repo_slug=self.repo.slug, - expected_status=HTTP_403_FORBIDDEN) - # list of all groups for an user - self.get_members(urlfor='users', repo_slug=self.repo.slug, - username=self.user.username, - expected_status=HTTP_403_FORBIDDEN) - for group_type in BaseGroupTypes.all_base_groups(): - # specific group for an user - self.get_members(urlfor='users', repo_slug=self.repo.slug, - username=self.user.username, - group_type=group_type, - expected_status=HTTP_403_FORBIDDEN) - # list of all users for a group - self.get_members(urlfor='groups', repo_slug=self.repo.slug, - group_type=group_type, - expected_status=HTTP_403_FORBIDDEN) - # specific user for a group - self.get_members(urlfor='groups', repo_slug=self.repo.slug, - username=self.user.username, - group_type=group_type, - expected_status=HTTP_403_FORBIDDEN) - - # any kind of user in the repo groups can retrieve infos - for user in [self.author_user.username, self.curator_user.username, - self.user.username]: - self.logout() - self.login(user) - # list of all groups for an user - self.get_members(urlfor='base', repo_slug=self.repo.slug) - # specific group for an user - self.get_members(urlfor='users', repo_slug=self.repo.slug, - username=self.user.username) - for group_type in BaseGroupTypes.all_base_groups(): - self.get_members(urlfor='users', repo_slug=self.repo.slug, - username=self.user.username, - group_type=group_type) - # list of all users for a group - self.get_members(urlfor='groups', repo_slug=self.repo.slug, - group_type=group_type) - # specific user for a group - self.get_members(urlfor='groups', repo_slug=self.repo.slug, - username=self.user.username, - group_type=group_type) - - def test_members_create(self): - """ - Tests for members. - Post requests: an user can create members only if s/he is admin - The only URLS where users can be assigned to group or vice versa are - /api/v1/repositories//members/groups//users/ - /api/v1/repositories//members/users//groups/ - """ - self.logout() - mem_dict_user = {'group_type': 'administrators'} - mem_dict_groups = {'username': self.user_norepo.username} - # as anonymous - self.create_member(urlfor='users', repo_slug=self.repo.slug, - mem_dict=mem_dict_user, username=self.user.username, - expected_status=HTTP_403_FORBIDDEN) - for group_type in BaseGroupTypes.all_base_groups(): - self.create_member(urlfor='groups', repo_slug=self.repo.slug, - mem_dict=mem_dict_groups, - group_type=group_type, - expected_status=HTTP_403_FORBIDDEN) - # as author - self.login(self.author_user.username) - self.create_member(urlfor='users', repo_slug=self.repo.slug, - mem_dict=mem_dict_user, username=self.user.username, - expected_status=HTTP_403_FORBIDDEN) - for group_type in BaseGroupTypes.all_base_groups(): - self.create_member(urlfor='groups', repo_slug=self.repo.slug, - mem_dict=mem_dict_groups, - group_type=group_type, - expected_status=HTTP_403_FORBIDDEN) - # as curator - self.logout() - self.login(self.curator_user.username) - self.create_member(urlfor='users', repo_slug=self.repo.slug, - mem_dict=mem_dict_user, username=self.user.username, - expected_status=HTTP_403_FORBIDDEN) - for group_type in BaseGroupTypes.all_base_groups(): - self.create_member(urlfor='groups', repo_slug=self.repo.slug, - mem_dict=mem_dict_groups, - group_type=group_type, - expected_status=HTTP_403_FORBIDDEN) - # as administrator - self.logout() - self.login(self.user.username) - self.create_member(urlfor='users', repo_slug=self.repo.slug, - mem_dict=mem_dict_user, username=self.user.username) - for group_type in BaseGroupTypes.all_base_groups(): - self.create_member(urlfor='groups', repo_slug=self.repo.slug, - mem_dict=mem_dict_groups, - group_type=group_type) - - def test_members_delete(self): - """ - Tests for members. - Delete requests: an user can delete members only if s/he is admin - The only URLS where users can be deleted from a group or vice versa are - /api/v1/repositories//members/groups//users/ - /api/v1/repositories//members/users//groups/ - """ - for group_type in BaseGroupTypes.all_base_groups(): - # as anonymous - self.logout() - self.delete_member(urlfor='users', repo_slug=self.repo.slug, - username=self.user.username, - group_type=group_type, - expected_status=HTTP_403_FORBIDDEN) - self.delete_member(urlfor='groups', repo_slug=self.repo.slug, - username=self.user.username, - group_type=group_type, - expected_status=HTTP_403_FORBIDDEN) - # as author - self.login(self.author_user.username) - self.delete_member(urlfor='users', repo_slug=self.repo.slug, - username=self.user.username, - group_type=group_type, - expected_status=HTTP_403_FORBIDDEN) - self.delete_member(urlfor='groups', repo_slug=self.repo.slug, - username=self.user.username, - group_type=group_type, - expected_status=HTTP_403_FORBIDDEN) - # as curator - self.logout() - self.login(self.curator_user.username) - self.delete_member(urlfor='users', repo_slug=self.repo.slug, - username=self.user.username, - group_type=group_type, - expected_status=HTTP_403_FORBIDDEN) - self.delete_member(urlfor='groups', repo_slug=self.repo.slug, - username=self.user.username, - group_type=group_type, - expected_status=HTTP_403_FORBIDDEN) - # different loop because the actual deletion can impact the other tests - for group_type in BaseGroupTypes.all_base_groups(): - # as administrator - # deleting a different username because deleting self from admin is - # a special case (handled in different tests) - self.logout() - self.login(self.user.username) - self.delete_member(urlfor='users', repo_slug=self.repo.slug, - username=self.author_user.username, - group_type=group_type) - self.delete_member(urlfor='groups', repo_slug=self.repo.slug, - username=self.author_user.username, - group_type=group_type) - - def test_resource_get(self): - """Test retrieve learning resource""" - self.assertEqual( - 1, self.get_learning_resources(self.repo.slug)['count']) - - resource = self.import_course_tarball(self.repo) - lr_id = resource.id - - # author_user has view_repo permissions - self.logout() - self.login(self.author_user.username) - self.assertEqual( - 1 + self.toy_resource_count, - self.get_learning_resources(self.repo.slug)['count'] - ) - self.get_learning_resource(self.repo.slug, lr_id) - - # user_norepo has no view_repo permission - self.logout() - self.login(self.user_norepo.username) - self.get_learning_resources( - self.repo.slug, expected_status=HTTP_403_FORBIDDEN) - self.get_learning_resource( - self.repo.slug, lr_id, expected_status=HTTP_403_FORBIDDEN) - - # as anonymous - self.logout() - self.get_learning_resources( - self.repo.slug, expected_status=HTTP_403_FORBIDDEN) - self.get_learning_resource( - self.repo.slug, lr_id, expected_status=HTTP_403_FORBIDDEN) - - def test_resource_post(self): - """Test create learning resource""" - resp = self.client.post( - '{repo_base}{repo_slug}/learning_resources/'.format( - repo_base=REPO_BASE, - repo_slug=self.repo.slug, - ), - self.DEFAULT_LR_DICT - ) - self.assertEqual(resp.status_code, HTTP_405_METHOD_NOT_ALLOWED) - - def test_resource_delete(self): - """Test delete learning resource""" - resource = self.import_course_tarball(self.repo) - lr_id = resource.id - - resp = self.client.delete( - '{repo_base}{repo_slug}/learning_resources/{lr_id}/'.format( - repo_base=REPO_BASE, - repo_slug=self.repo.slug, - lr_id=lr_id, - ), - self.DEFAULT_LR_DICT - ) - self.assertEqual(resp.status_code, HTTP_405_METHOD_NOT_ALLOWED) - - def test_resource_put_patch(self): - """Test update learning resource""" - resource = self.import_course_tarball(self.repo) - lr_id = resource.id - - new_description_dict = {"description": "new description"} - - # as curator who has add_edit_metadata permission - self.logout() - self.login(self.curator_user.username) - self.patch_learning_resource( - self.repo.slug, lr_id, new_description_dict - ) - new_description_dict['description'] += "_" - new_description_dict['terms'] = [] - self.put_learning_resource( - self.repo.slug, lr_id, new_description_dict - ) - - # author does have add_edit_metadata permission - self.logout() - self.login(self.author_user.username) - new_description_dict['description'] += "_" - self.patch_learning_resource( - self.repo.slug, lr_id, new_description_dict - ) - new_description_dict['description'] += "_" - self.put_learning_resource( - self.repo.slug, lr_id, new_description_dict - ) - - # as anonymous - self.logout() - new_description_dict['description'] += "_" - self.patch_learning_resource( - self.repo.slug, lr_id, new_description_dict, - expected_status=HTTP_403_FORBIDDEN - ) - self.put_learning_resource( - self.repo.slug, lr_id, new_description_dict, - expected_status=HTTP_403_FORBIDDEN - ) - - def test_static_assets_get(self): - """Test for getting static assets from learning_resources""" - def get_resource_with_asset(type_name): - """ - Get a LearningResource with a StaticAsset. - """ - for resource in get_resources(self.repo.id): - if (resource.learning_resource_type.name != type_name or - resource.static_assets.count() == 0): - continue - return resource - self.import_course_tarball(self.repo) - # Most static assets within a course will be associated with multiple - # resources due to the hierarchy. For the test course, we know that - # there is no overlap between the html and video, making them - # suitable for this test. - resource1 = get_resource_with_asset("html") - resource2 = get_resource_with_asset("video") - static_asset1 = resource1.static_assets.first() - static_asset2 = resource2.static_assets.first() - lr1_id = resource1.id - - # add a second StaticAsset to another learning resource - resource2.static_assets.add(static_asset2) - lr2_id = resource2.id - # make sure the result for an asset contains a name and an url - resp = self.get_static_asset(self.repo.slug, lr1_id, static_asset1.id) - self.assertTrue('asset' in resp) - self.assertTrue('name' in resp) - self.assertTrue(static_asset1.asset.url in resp['asset']) - self.assertEqual( - resp['name'], - os.path.basename(static_asset1.asset.name) - ) - - # make sure static assets only show up in their proper places - self.get_static_asset(self.repo.slug, lr1_id, static_asset1.id) - self.assertEqual( - 2, self.get_static_assets(self.repo.slug, lr1_id)['count']) - self.get_static_asset(self.repo.slug, lr1_id, static_asset2.id, - expected_status=HTTP_404_NOT_FOUND) - self.get_static_asset(self.repo.slug, lr2_id, static_asset1.id, - expected_status=HTTP_404_NOT_FOUND) - self.get_static_asset(self.repo.slug, lr2_id, static_asset2.id) - self.assertEqual( - 1, self.get_static_assets(self.repo.slug, lr2_id)['count']) - - # author_user has view_repo permissions - self.logout() - self.login(self.author_user.username) - self.assertEqual( - 2, self.get_static_assets(self.repo.slug, lr1_id)['count']) - self.get_static_asset(self.repo.slug, lr1_id, static_asset1.id) - - # user_norepo has no view_repo permission - self.logout() - self.login(self.user_norepo.username) - self.get_static_assets(self.repo.slug, lr1_id, - expected_status=HTTP_403_FORBIDDEN) - self.get_static_asset(self.repo.slug, lr1_id, static_asset1.id, - expected_status=HTTP_403_FORBIDDEN) - - # as anonymous - self.logout() - self.get_static_assets(self.repo.slug, lr1_id, - expected_status=HTTP_403_FORBIDDEN) - self.get_static_asset(self.repo.slug, lr1_id, static_asset1.id, - expected_status=HTTP_403_FORBIDDEN) - - def test_static_assets_create(self): - """Test for creating static assets from learning_resources""" - lr_id = get_resources(self.repo.id).first().id - resp = self.client.post( - '{repo_base}{repo_slug}/learning_resources/' - '{lr_id}/static_assets/'.format( - repo_base=REPO_BASE, - repo_slug=self.repo.slug, - lr_id=lr_id, - ), - self.DEFAULT_LR_DICT - ) - self.assertEqual(resp.status_code, HTTP_405_METHOD_NOT_ALLOWED) - - def test_static_assets_put_patch(self): - """Test for updating static assets from learning_resources""" - resource = self.import_course_tarball(self.repo) - static_asset = resource.static_assets.first() - lr_id = resource.id - - resp = self.client.put( - '{repo_base}{repo_slug}/learning_resources/' - '{lr_id}/static_assets/{sa_id}/'.format( - repo_base=REPO_BASE, - repo_slug=self.repo.slug, - lr_id=lr_id, - sa_id=static_asset.id, - ), - self.DEFAULT_LR_DICT - ) - self.assertEqual(resp.status_code, HTTP_405_METHOD_NOT_ALLOWED) - resp = self.client.patch( - '{repo_base}{repo_slug}/learning_resources/' - '{lr_id}/static_assets/{sa_id}/'.format( - repo_base=REPO_BASE, - repo_slug=self.repo.slug, - lr_id=lr_id, - sa_id=static_asset.id, - ), - self.DEFAULT_LR_DICT - ) - self.assertEqual(resp.status_code, HTTP_405_METHOD_NOT_ALLOWED) - - def test_static_assets_delete(self): - """Test for deleting static assets from learning_resources""" - resource = self.import_course_tarball(self.repo) - static_asset = resource.static_assets.first() - lr_id = resource.id - - resp = self.client.delete( - '{repo_base}{repo_slug}/learning_resources/' - '{lr_id}/static_assets/{sa_id}/'.format( - repo_base=REPO_BASE, - repo_slug=self.repo.slug, - lr_id=lr_id, - sa_id=static_asset.id, - ), - self.DEFAULT_LR_DICT - ) - self.assertEqual(resp.status_code, HTTP_405_METHOD_NOT_ALLOWED) diff --git a/rest/tests/test_rest.py b/rest/tests/test_vocabulary.py similarity index 62% rename from rest/tests/test_rest.py rename to rest/tests/test_vocabulary.py index d5a1951a..006cdc70 100644 --- a/rest/tests/test_rest.py +++ b/rest/tests/test_vocabulary.py @@ -1,6 +1,7 @@ """ -Unit tests for REST api +REST tests relating to vocabularies and terms """ + from __future__ import unicode_literals from rest_framework.status import ( @@ -11,114 +12,25 @@ HTTP_405_METHOD_NOT_ALLOWED, ) -from importer.tasks import import_file -from learningresources.models import ( - Repository, - LearningResource, - LearningResourceType, +from rest.tests.base import ( + RESTAuthTestCase, + RESTTestCase, + REPO_BASE, + as_json, ) -from learningresources.api import get_resources -from taxonomy.models import Vocabulary, Term - from rest.serializers import ( - RepositorySerializer, VocabularySerializer, TermSerializer, ) -from .base import ( - RESTTestCase, - REPO_BASE, - API_BASE, - as_json, -) +from taxonomy.models import Vocabulary, Term +from learningresources.models import LearningResourceType -class TestRest(RESTTestCase): +# pylint: disable=invalid-name +class TestVocabulary(RESTTestCase): """ - REST test + REST tests relating to vocabularies and terms """ - - def test_repositories(self): - """ - Test for Repository - """ - repositories = self.get_repositories() - self.assertEqual(1, repositories['count']) - - self.create_repository() - repositories = self.get_repositories() - self.assertEqual(2, repositories['count']) - repo_slug = repositories['results'][1]['slug'] - - # test PUT and PATCH - input_dict = { - 'name': 'name', - 'description': 'description', - } - input_dict_changed = { - 'name': 'name', - 'description': 'changed description', - } - - # patch with missing slug - self.patch_repository( - "missing", {'name': 'rename'}, - expected_status=HTTP_404_NOT_FOUND - ) - - self.patch_repository( - repo_slug, {'name': 'rename'}, - expected_status=HTTP_405_METHOD_NOT_ALLOWED) - - # put with missing slug - self.put_repository("missing slug", input_dict, - expected_status=HTTP_404_NOT_FOUND) - - self.put_repository( - repo_slug, input_dict, - expected_status=HTTP_405_METHOD_NOT_ALLOWED) - - # verify change of data - self.put_repository( - repo_slug, input_dict_changed, - expected_status=HTTP_405_METHOD_NOT_ALLOWED) - - # never created a duplicate - self.assertEqual(2, Repository.objects.count()) - - # call to verify HTTP_OK - self.get_repository(repo_slug) - - # delete a repository - self.delete_repository(repo_slug, - expected_status=HTTP_405_METHOD_NOT_ALLOWED) - - repositories = self.get_repositories() - self.assertEqual(2, repositories['count']) - - vocab_slug = self.create_vocabulary(self.repo.slug)['slug'] - vocabularies = self.get_vocabularies(self.repo.slug) - self.assertEqual(1, vocabularies['count']) - - self.delete_repository(self.repo.slug, - expected_status=HTTP_405_METHOD_NOT_ALLOWED) - - self.delete_vocabulary(self.repo.slug, vocab_slug) - - vocabularies = self.get_vocabularies(self.repo.slug) - self.assertEqual(0, vocabularies['count']) - - self.delete_repository(self.repo.slug, - expected_status=HTTP_405_METHOD_NOT_ALLOWED) - - self.get_repositories() - self.get_repository("missing", expected_status=HTTP_404_NOT_FOUND) - - # as anonymous - self.logout() - self.get_repositories(expected_status=HTTP_403_FORBIDDEN) - self.get_repository("missing", expected_status=HTTP_403_FORBIDDEN) - def test_vocabularies(self): """ Test for Vocabulary @@ -485,38 +397,6 @@ def test_delete_propagation(self): expected_status=HTTP_404_NOT_FOUND) self.get_term(self.repo.slug, vocab2_slug, term2_slug) - def test_repository_pagination(self): - """Test pagination for collections""" - - expected = [ - self.create_repository( - { - 'name': "name{i}".format(i=i), - "description": "description" - } - ) for i in range(40)] - - repositories = self.get_repositories() - - # 40 we created + self.repo - self.assertEqual(41, repositories['count']) - self.assertEqual( - [RepositorySerializer( - Repository.objects.get(id=x['id'])).data - for x in expected[:19]], - repositories['results'][1:20]) - - resp = self.client.get( - '{repo_base}?page=2'.format( - repo_base=REPO_BASE, - )) - self.assertEqual(HTTP_200_OK, resp.status_code) - repositories = as_json(resp) - self.assertEqual(41, repositories['count']) - self.assertEqual( - [RepositorySerializer(Repository.objects.get(id=x['id'])).data - for x in expected[19:39]], repositories['results']) - def test_vocabulary_pagination(self): """Test pagination for collections""" @@ -585,33 +465,6 @@ def test_term_pagination(self): self.assertEqual([TermSerializer(x).data for x in expected[20:40]], terms['results']) - # for some reason Pylint has issues with this name - # pylint: disable=invalid-name - def test_immutable_fields_repository(self): - """Test repository immutable fields""" - repo_dict = { - 'id': -1, - 'slug': 'sluggy', - 'name': 'other name', - 'description': 'description', - 'date_created': "2015-01-01", - 'created_by': self.user_norepo.id, - } - - def assert_not_changed(new_dict): - """Check that fields have not changed""" - # These keys should be different since they are immutable or set by - # the serializer. - for field in ('id', 'slug', 'date_created'): - self.assertNotEqual(repo_dict[field], new_dict[field]) - - # created_by is set internally and should not show up in output. - self.assertNotIn('created_by', new_dict) - repository = Repository.objects.get(slug=new_dict['slug']) - self.assertEqual(repository.created_by.id, self.user.id) - - assert_not_changed(self.create_repository(repo_dict, skip_assert=True)) - def test_immutable_fields_vocabulary(self): """Test immutable fields for vocabulary""" @@ -696,191 +549,6 @@ def assert_not_changed(new_dict): ) assert_not_changed(result) - def test_immutable_fields_learning_resource(self): - """Test immutable fields for term""" - self.import_course_tarball(self.repo) - resource = LearningResource.objects.first() - lr_id = resource.id - - lr_dict = { - "id": 99, - "learning_resource_type": 4, - "static_assets": [3], - "title": "Getting Help", - "description": "description", - "content_xml": "...", - "materialized_path": - "/course/chapter[4]/sequential[1]/vertical[3]", - "url_path": "url_path", - "parent": 22, - "copyright": "copyright", - "xa_nr_views": 1, - "xa_nr_attempts": 2, - "xa_avg_grade": 3.0, - "xa_histogram_grade": 4.0, - "terms": [], - "preview_url": "", - } - - def assert_not_changed(new_dict): - """Check that fields have not changed""" - # These keys should be different since they are immutable or set by - # the serializer. - fields = ( - 'id', 'learning_resource_type', 'static_assets', 'title', - 'content_xml', 'materialized_path', 'url_path', 'parent', - 'copyright', 'xa_nr_views', 'xa_nr_attempts', 'xa_avg_grade', - 'xa_histogram_grade', 'preview_url' - ) - for field in fields: - self.assertNotEqual(lr_dict[field], new_dict[field]) - - assert_not_changed( - self.patch_learning_resource(self.repo.slug, lr_id, lr_dict, - skip_assert=True)) - assert_not_changed( - self.put_learning_resource(self.repo.slug, lr_id, lr_dict, - skip_assert=True)) - - def test_missing_learning_resource(self): - """Test for an invalid learning resource id""" - repo_slug1 = self.repo.slug - resource1 = self.import_course_tarball(self.repo) - lr1_id = resource1.id - - # import from a different course so it's not a duplicate course - zip_file = self.get_course_zip() - new_repo_dict = self.create_repository() - repo_slug2 = new_repo_dict['slug'] - repo_id2 = new_repo_dict['id'] - import_file( - zip_file, repo_id2, self.user.id) - resource2 = get_resources(repo_id2).first() - lr2_id = resource2.id - - # repo_slug1 should own lr1_id and repo_slug2 should own lr2_id - self.get_learning_resource(repo_slug1, lr1_id) - self.get_learning_resource(repo_slug2, lr1_id, - expected_status=HTTP_404_NOT_FOUND) - self.get_learning_resource(repo_slug1, lr2_id, - expected_status=HTTP_404_NOT_FOUND) - self.get_learning_resource(repo_slug2, lr2_id) - - def test_filefield_serialization(self): - """Make sure that URL output is turned on in settings""" - resource = self.import_course_tarball(self.repo) - static_assets = self.get_static_assets( - self.repo.slug, resource.id)['results'] - self.assertTrue(static_assets[0]['asset'].startswith("http")) - - def test_root(self): - """ - Test root of API - """ - resp = self.client.get(API_BASE) - self.assertEqual(HTTP_404_NOT_FOUND, resp.status_code) - resp = self.client.post(API_BASE) - self.assertEqual(HTTP_404_NOT_FOUND, resp.status_code) - resp = self.client.head(API_BASE) - self.assertEqual(HTTP_404_NOT_FOUND, resp.status_code) - resp = self.client.patch(API_BASE) - self.assertEqual(HTTP_404_NOT_FOUND, resp.status_code) - resp = self.client.put(API_BASE) - self.assertEqual(HTTP_404_NOT_FOUND, resp.status_code) - resp = self.client.options(API_BASE) - self.assertEqual(HTTP_404_NOT_FOUND, resp.status_code) - - self.logout() - resp = self.client.get(API_BASE) - self.assertEqual(HTTP_404_NOT_FOUND, resp.status_code) - resp = self.client.post(API_BASE) - self.assertEqual(HTTP_404_NOT_FOUND, resp.status_code) - resp = self.client.head(API_BASE) - self.assertEqual(HTTP_404_NOT_FOUND, resp.status_code) - resp = self.client.patch(API_BASE) - self.assertEqual(HTTP_404_NOT_FOUND, resp.status_code) - resp = self.client.put(API_BASE) - self.assertEqual(HTTP_404_NOT_FOUND, resp.status_code) - resp = self.client.options(API_BASE) - self.assertEqual(HTTP_404_NOT_FOUND, resp.status_code) - - def test_add_term_to_learning_resource(self): - """ - Add a term to a learning resource via PATCH - """ - - resource = self.import_course_tarball(self.repo) - lr_id = resource.id - - vocab1_slug = self.create_vocabulary(self.repo.slug)['slug'] - supported_term_slug = self.create_term( - self.repo.slug, vocab1_slug)['slug'] - - # This should change soon but for now we can't set this via API - Vocabulary.objects.get(slug=vocab1_slug).learning_resource_types.add( - resource.learning_resource_type - ) - - vocab_dict = dict(self.DEFAULT_VOCAB_DICT) - vocab_dict['name'] += " changed" - vocab2_slug = self.create_vocabulary( - self.repo.slug, vocab_dict)['slug'] - unsupported_term_slug = self.create_term( - self.repo.slug, vocab2_slug)['slug'] - - self.assertEqual([], self.get_learning_resource( - self.repo.slug, lr_id)['terms']) - - self.patch_learning_resource( - self.repo.slug, lr_id, {"terms": [supported_term_slug]}) - self.patch_learning_resource( - self.repo.slug, lr_id, {"terms": ["missing"]}, - expected_status=HTTP_400_BAD_REQUEST) - self.patch_learning_resource( - self.repo.slug, lr_id, {"terms": [unsupported_term_slug]}, - expected_status=HTTP_400_BAD_REQUEST) - - def test_learning_resource_types(self): - """ - Get from learning_resource_types - """ - base_url = "{}learning_resource_types/".format(API_BASE) - - resp = self.client.get(base_url) - self.assertEqual(HTTP_200_OK, resp.status_code) - types = as_json(resp) - - self.assertEqual(sorted([lrt.name for lrt - in LearningResourceType.objects.all()]), - sorted([t['name'] for t in types['results']])) - - # nothing besides GET, OPTION, HEAD allowed - resp = self.client.options(base_url) - self.assertEqual(HTTP_200_OK, resp.status_code) - resp = self.client.head(base_url) - self.assertEqual(HTTP_200_OK, resp.status_code) - resp = self.client.post(base_url, {}) - self.assertEqual(HTTP_405_METHOD_NOT_ALLOWED, resp.status_code) - resp = self.client.patch(base_url, {}) - self.assertEqual(HTTP_405_METHOD_NOT_ALLOWED, resp.status_code) - resp = self.client.put(base_url, {}) - self.assertEqual(HTTP_405_METHOD_NOT_ALLOWED, resp.status_code) - - # restricted to logged in users - self.logout() - resp = self.client.get(base_url) - self.assertEqual(HTTP_403_FORBIDDEN, resp.status_code) - - # but otherwise unrestricted - self.login(self.user_norepo) - resp = self.client.get(base_url) - self.assertEqual(HTTP_200_OK, resp.status_code) - types = as_json(resp) - - self.assertEqual(sorted([lrt.name for lrt - in LearningResourceType.objects.all()]), - sorted([t['name'] for t in types['results']])) - def test_vocabulary_learning_resource_types(self): """ Test learning_resource_types field on vocabularies @@ -932,28 +600,290 @@ def test_vocabulary_learning_resource_types(self): self.get_vocabulary( self.repo.slug, vocab_slug)['learning_resource_types']) - def test_preview_url(self): - """ - Assert preview url behavior for learning resources - """ - learning_resource = LearningResource.objects.first() - expected_jump_to_id_url = ( - "https://www.sandbox.edx.org/courses/" - "test-org/infinity/Febtober/jump_to_id/url_name1" + +# pylint: disable=too-many-ancestors +class TestVocabularyAuthorization(RESTAuthTestCase): + """ + Tests relating to term and vocabulary authorization + """ + def test_vocabulary_create(self): + """Test create vocabulary""" + self.logout() + self.login(self.curator_user.username) + + self.create_vocabulary(self.repo.slug) + self.assertEqual(1, self.get_vocabularies(self.repo.slug)['count']) + + # login as author which doesn't have manage_taxonomy permissions + self.logout() + self.login(self.author_user.username) + + vocab_dict = dict(self.DEFAULT_VOCAB_DICT) + vocab_dict['name'] = 'other_name' # prevent name collision + + # author does not have create access + self.create_vocabulary( + self.repo.slug, vocab_dict, + expected_status=HTTP_403_FORBIDDEN ) - self.assertEqual( - expected_jump_to_id_url, - learning_resource.get_preview_url() + + # make sure at least view_repo is needed + self.logout() + self.login(self.user_norepo.username) + + self.create_vocabulary(self.repo.slug, vocab_dict, + expected_status=HTTP_403_FORBIDDEN) + + # as anonymous + self.logout() + self.create_vocabulary(self.repo.slug, vocab_dict, + expected_status=HTTP_403_FORBIDDEN) + + def test_vocabulary_put_patch(self): + """Test update vocabulary""" + vocab_slug = self.create_vocabulary(self.repo.slug)['slug'] + + self.logout() + self.login(self.curator_user.username) + + self.patch_vocabulary(self.repo.slug, vocab_slug, + self.DEFAULT_VOCAB_DICT) + self.put_vocabulary(self.repo.slug, vocab_slug, + self.DEFAULT_VOCAB_DICT) + + # login as author which doesn't have manage_taxonomy permissions + self.logout() + self.login(self.author_user.username) + + self.patch_vocabulary(self.repo.slug, vocab_slug, + self.DEFAULT_VOCAB_DICT, + expected_status=HTTP_403_FORBIDDEN) + self.put_vocabulary(self.repo.slug, vocab_slug, + self.DEFAULT_VOCAB_DICT, + expected_status=HTTP_403_FORBIDDEN) + + # make sure at least view_repo is needed + self.logout() + self.login(self.user_norepo.username) + self.patch_vocabulary(self.repo.slug, vocab_slug, + self.DEFAULT_VOCAB_DICT, + expected_status=HTTP_403_FORBIDDEN) + self.put_vocabulary(self.repo.slug, vocab_slug, + self.DEFAULT_VOCAB_DICT, + expected_status=HTTP_403_FORBIDDEN) + + # as anonymous + self.logout() + self.patch_vocabulary(self.repo.slug, vocab_slug, + self.DEFAULT_VOCAB_DICT, + expected_status=HTTP_403_FORBIDDEN) + self.put_vocabulary(self.repo.slug, vocab_slug, + self.DEFAULT_VOCAB_DICT, + expected_status=HTTP_403_FORBIDDEN) + + def test_vocabulary_delete(self): + """Test delete vocabulary""" + vocab_slug = self.create_vocabulary(self.repo.slug)['slug'] + + # login as author which doesn't have manage_taxonomy permissions + self.logout() + self.login(self.author_user.username) + + self.delete_vocabulary(self.repo.slug, vocab_slug, + expected_status=HTTP_403_FORBIDDEN) + + # curator does have manage_taxonomy permissions + self.logout() + self.login(self.curator_user.username) + self.delete_vocabulary(self.repo.slug, vocab_slug) + + # vocab is missing + self.create_term(self.repo.slug, vocab_slug, + expected_status=HTTP_404_NOT_FOUND) + self.delete_vocabulary(self.repo.slug, vocab_slug, + expected_status=HTTP_404_NOT_FOUND) + + # recreate vocab so we can delete it + vocab_slug = self.create_vocabulary(self.repo.slug)['slug'] + + # author has view_repo but delete not allowed + self.logout() + self.login(self.author_user.username) + self.delete_vocabulary(self.repo.slug, vocab_slug, + expected_status=HTTP_403_FORBIDDEN) + + # make sure at least view_repo is needed + self.logout() + self.login(self.user_norepo.username) + + self.delete_vocabulary(self.repo.slug, vocab_slug, + expected_status=HTTP_403_FORBIDDEN) + + # as anonymous + self.logout() + self.delete_vocabulary(self.repo.slug, vocab_slug, + expected_status=HTTP_403_FORBIDDEN) + + def test_vocabulary_get(self): + """Test get vocabulary and vocabularies""" + vocab_slug = self.create_vocabulary(self.repo.slug)['slug'] + + # author_user has view_repo permissions + self.logout() + self.login(self.author_user.username) + self.assertEqual(1, self.get_vocabularies(self.repo.slug)['count']) + self.get_vocabulary(self.repo.slug, vocab_slug) + + # user_norepo has no view_repo permission + self.logout() + self.login(self.user_norepo.username) + self.get_vocabularies( + self.repo.slug, expected_status=HTTP_403_FORBIDDEN) + self.get_vocabulary(self.repo.slug, vocab_slug, + expected_status=HTTP_403_FORBIDDEN) + + # as anonymous + self.logout() + self.get_vocabularies( + self.repo.slug, expected_status=HTTP_403_FORBIDDEN) + self.get_vocabulary(self.repo.slug, vocab_slug, + expected_status=HTTP_403_FORBIDDEN) + + def test_term_create(self): + """Test create term""" + vocab_slug = self.create_vocabulary(self.repo.slug)['slug'] + + # curator has manage_taxonomy permission + self.logout() + self.login(self.curator_user.username) + + self.create_term(self.repo.slug, vocab_slug) + self.assertEqual(1, self.get_terms( + self.repo.slug, vocab_slug)['count']) + + # login as author which doesn't have manage_taxonomy permissions + self.logout() + self.login(self.author_user.username) + + term_dict = dict(self.DEFAULT_TERM_DICT) + term_dict['label'] = 'other_name' # prevent name collision + + # author does not have create access + self.create_term( + self.repo.slug, vocab_slug, term_dict, + expected_status=HTTP_403_FORBIDDEN ) - resource_dict = self.get_learning_resource( - self.repo.slug, learning_resource.id) - self.assertEqual( - expected_jump_to_id_url, resource_dict['preview_url']) + # make sure at least view_repo is needed + self.logout() + self.login(self.user_norepo.username) + self.create_term( + self.repo.slug, vocab_slug, term_dict, + expected_status=HTTP_403_FORBIDDEN + ) - learning_resource.url_name = None - self.assertEqual( - "https://www.sandbox.edx.org/courses/" - "test-org/infinity/Febtober/courseware", - learning_resource.get_preview_url() + # as anonymous + self.logout() + self.create_term( + self.repo.slug, vocab_slug, term_dict, + expected_status=HTTP_403_FORBIDDEN ) + + def test_term_put_patch(self): + """Test update term""" + vocab_slug = self.create_vocabulary(self.repo.slug)['slug'] + term_slug = self.create_term(self.repo.slug, vocab_slug)['slug'] + + self.logout() + self.login(self.curator_user.username) + + self.patch_term(self.repo.slug, vocab_slug, term_slug, + self.DEFAULT_TERM_DICT) + self.put_term(self.repo.slug, vocab_slug, term_slug, + self.DEFAULT_TERM_DICT) + + # login as author which doesn't have manage_taxonomy permissions + self.logout() + self.login(self.author_user.username) + + self.patch_term(self.repo.slug, vocab_slug, term_slug, + self.DEFAULT_TERM_DICT, + expected_status=HTTP_403_FORBIDDEN) + self.put_term(self.repo.slug, vocab_slug, term_slug, + self.DEFAULT_TERM_DICT, + expected_status=HTTP_403_FORBIDDEN) + + # make sure at least view_repo is needed + self.logout() + self.login(self.user_norepo.username) + + self.patch_term(self.repo.slug, vocab_slug, term_slug, + self.DEFAULT_TERM_DICT, + expected_status=HTTP_403_FORBIDDEN) + self.put_term(self.repo.slug, vocab_slug, term_slug, + self.DEFAULT_TERM_DICT, + expected_status=HTTP_403_FORBIDDEN) + + # as anonymous + self.logout() + self.patch_term(self.repo.slug, vocab_slug, term_slug, + self.DEFAULT_TERM_DICT, + expected_status=HTTP_403_FORBIDDEN) + self.put_term(self.repo.slug, vocab_slug, term_slug, + self.DEFAULT_TERM_DICT, + expected_status=HTTP_403_FORBIDDEN) + + def test_term_delete(self): + """Test delete term""" + vocab_slug = self.create_vocabulary(self.repo.slug)['slug'] + term_slug = self.create_term(self.repo.slug, vocab_slug)['slug'] + + # login as author which doesn't have manage_taxonomy permissions + self.logout() + self.login(self.author_user.username) + + self.delete_term(self.repo.slug, vocab_slug, term_slug, + expected_status=HTTP_403_FORBIDDEN) + + # curator does have manage_taxonomy permissions + self.logout() + self.login(self.curator_user.username) + self.delete_term(self.repo.slug, vocab_slug, term_slug) + + # make sure at least view_repo is needed + self.logout() + self.login(self.user_norepo.username) + self.delete_term(self.repo.slug, vocab_slug, term_slug, + expected_status=HTTP_403_FORBIDDEN) + + # as anonymous + self.logout() + self.delete_term(self.repo.slug, vocab_slug, term_slug, + expected_status=HTTP_403_FORBIDDEN) + + def test_term_get(self): + """Test retrieve term""" + vocab_slug = self.create_vocabulary(self.repo.slug)['slug'] + term_slug = self.create_term(self.repo.slug, vocab_slug)['slug'] + + # author_user has view_repo permissions + self.logout() + self.login(self.author_user.username) + self.assertEqual(1, self.get_terms( + self.repo.slug, vocab_slug)['count']) + self.get_term(self.repo.slug, vocab_slug, term_slug) + + # user_norepo has no view_repo permission + self.logout() + self.login(self.user_norepo.username) + self.get_terms(self.repo.slug, vocab_slug, + expected_status=HTTP_403_FORBIDDEN) + self.get_term(self.repo.slug, vocab_slug, term_slug, + expected_status=HTTP_403_FORBIDDEN) + + # as anonymous + self.logout() + self.get_terms(self.repo.slug, vocab_slug, + expected_status=HTTP_403_FORBIDDEN) + self.get_term(self.repo.slug, vocab_slug, term_slug, + expected_status=HTTP_403_FORBIDDEN) From 1c509c67c02d7350c3f2983263391ae2195e9eb9 Mon Sep 17 00:00:00 2001 From: George Schneeloch Date: Tue, 21 Jul 2015 16:13:03 -0400 Subject: [PATCH 07/38] Fixed preview link not showing up in list view --- ui/templates/includes/learning_resource.html | 2 +- ui/tests/test_learningresources_views.py | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/ui/templates/includes/learning_resource.html b/ui/templates/includes/learning_resource.html index 76725c2c..4131ab6b 100644 --- a/ui/templates/includes/learning_resource.html +++ b/ui/templates/includes/learning_resource.html @@ -45,7 +45,7 @@

{{ result.object.course.course_number }} {{ result.object.course.run }} - Preview + Preview

diff --git a/ui/tests/test_learningresources_views.py b/ui/tests/test_learningresources_views.py index faf812b1..bc9b7c91 100644 --- a/ui/tests/test_learningresources_views.py +++ b/ui/tests/test_learningresources_views.py @@ -28,6 +28,7 @@ log = logging.getLogger(__name__) +# pylint: disable=too-many-public-methods class TestViews(LoreTestCase): """Hit each view.""" @@ -376,3 +377,12 @@ def test_serve_media(self): # this is in case of python 3.3 except AttributeError: # pragma: no cover imp.reload(ui.urls) + + def test_preview_url(self): + """Test that preview url shows up correctly""" + resp = self.client.get(self.repository_url, follow=True) + self.assertIn( + 'Preview', + resp.content.decode('utf-8')) From 8ccad3bf8e8cabf9bb4fde85cf32c08cd38e6ee4 Mon Sep 17 00:00:00 2001 From: Carson Gee Date: Tue, 21 Jul 2015 17:28:10 -0400 Subject: [PATCH 08/38] Updated Haystack to 2.4.0 - Removed automatic index update from deployment --- .travis.yml | 1 - requirements.txt | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 0338c491..0bb3df07 100644 --- a/.travis.yml +++ b/.travis.yml @@ -24,5 +24,4 @@ deploy: run: - "sleep 15" - "python manage.py migrate --noinput" - - "python manage.py update_index" - "python manage.py sync_permissions" diff --git a/requirements.txt b/requirements.txt index 346e19e3..343ec83c 100644 --- a/requirements.txt +++ b/requirements.txt @@ -4,7 +4,7 @@ Django==1.8.3 django-appconf==1.0.1 django-bootstrap3==6.1.0 django-compressor==1.5 -django-haystack==2.3.1 +django-haystack==2.4.0 djangorestframework==3.1.3 dj-database-url==0.3.0 dj-static==0.0.6 From 8b8c2fcd0d073eab7f73dba025098d293764e854 Mon Sep 17 00:00:00 2001 From: Giovanni Di Milia Date: Mon, 20 Jul 2015 11:03:32 -0400 Subject: [PATCH 09/38] Refactored permission check for listing view --- ui/tests/test_learningresources_views.py | 18 +++++++++++++----- ui/views.py | 19 ++++++++++++------- 2 files changed, 25 insertions(+), 12 deletions(-) diff --git a/ui/tests/test_learningresources_views.py b/ui/tests/test_learningresources_views.py index bc9b7c91..b2d4205f 100644 --- a/ui/tests/test_learningresources_views.py +++ b/ui/tests/test_learningresources_views.py @@ -86,15 +86,23 @@ def test_create_repo_post(self): body = resp.content.decode("utf-8") self.assertTrue(repo_name in body) - def test_listing_unauthorized(self): - """View listing page.""" - # Not authorized to view this repository... - body = self.assert_status_code( + def test_listing_not_found(self): + """View listing page, but repo does not exist.""" + self.assert_status_code( "/repositories/99/", + NOT_FOUND, + return_body=True + ) + + def test_listing_unauthorized(self): + """View listing page, but not authorized to view this repository.""" + self.logout() + self.login(self.USERNAME_NO_REPO) + self.assert_status_code( + self.repository_url, UNAUTHORIZED, return_body=True ) - self.assertTrue("unauthorized" in body) def test_listing_importcourse_perms(self): """ diff --git a/ui/views.py b/ui/views.py index 80804494..51e52378 100644 --- a/ui/views.py +++ b/ui/views.py @@ -13,7 +13,6 @@ from django.core.servers.basehttp import FileWrapper from django.core.urlresolvers import reverse from django.http import HttpResponse, Http404, StreamingHttpResponse -from django.http.response import HttpResponseForbidden from django.shortcuts import ( render, redirect, @@ -24,7 +23,11 @@ from guardian.shortcuts import get_perms from learningresources.api import ( - get_repo, get_repos, get_resource, NotFound, + get_repo, + get_repos, + get_resource, + NotFound, + PermissionDenied as LorePermissionDenied, ) from learningresources.models import Repository, StaticAsset from roles.api import assign_user_to_repo_group @@ -182,13 +185,15 @@ class RepositoryView(FacetedSearchView): # We need the extra kwarg. def __call__(self, request, repo_slug): # Get arguments from the URL - # pylint: disable=attribute-defined-outside-init # It's a subclass of an external class, so we don't have # repo_slug in __init__. - repos = get_repos(request.user.id) - if repo_slug not in set([x.slug for x in repos]): - return HttpResponseForbidden("unauthorized") - self.repo = [x for x in repos if x.slug == repo_slug][0] + # pylint: disable=attribute-defined-outside-init + try: + self.repo = get_repo(repo_slug, request.user.id) + except NotFound: + raise Http404() + except LorePermissionDenied: + raise PermissionDenied('unauthorized') # get sorting from params if it's there sortby = dict(request.GET.copy()).get('sortby', []) if (len(sortby) > 0 and From 5d12e6f685ac23ec755d8cbd5d3e61fa53b45ed1 Mon Sep 17 00:00:00 2001 From: George Schneeloch Date: Wed, 22 Jul 2015 15:10:43 -0400 Subject: [PATCH 10/38] Refactored code for reloading module into a function --- ui/tests/test_learningresources_views.py | 28 +++--------------------- 1 file changed, 3 insertions(+), 25 deletions(-) diff --git a/ui/tests/test_learningresources_views.py b/ui/tests/test_learningresources_views.py index b2d4205f..8450b51c 100644 --- a/ui/tests/test_learningresources_views.py +++ b/ui/tests/test_learningresources_views.py @@ -4,8 +4,6 @@ from __future__ import unicode_literals -import imp -import importlib import logging import os @@ -18,6 +16,7 @@ from roles.api import assign_user_to_repo_group, remove_user_from_repo_group from roles.permissions import GroupTypes from search.sorting import LoreSortingFields +from six.moves import reload_module # pylint: disable=import-error from learningresources.tests.base import LoreTestCase @@ -358,33 +357,12 @@ def test_serve_media(self): DEFAULT_FILE_STORAGE=('storages.backends' '.s3boto.S3BotoStorage') ): - # force the reload of the urls - # python 2.7, 3.3 and 3.4 have different ways to reload modules - # 2.7 uses the builtin function reload - # 3.3 uses imp.reload - # 3.4 uses importlib.reload - try: - reload(ui.urls) # pylint: disable=undefined-variable - # this is in case of python 3.4 - except NameError: # pragma: no cover - try: - importlib.reload(ui.urls) - # this is in case of python 3.3 - except AttributeError: # pragma: no cover - imp.reload(ui.urls) + reload_module(ui.urls) # the view is not available any more resp = self.client.get(static_asset_url) self.assertEqual(resp.status_code, NOT_FOUND) # force the reload of the urls again to be sure to have everything back - try: - reload(ui.urls) # pylint: disable=undefined-variable - # this is in case of python 3.4 - except NameError: # pragma: no cover - try: - importlib.reload(ui.urls) - # this is in case of python 3.3 - except AttributeError: # pragma: no cover - imp.reload(ui.urls) + reload_module(ui.urls) def test_preview_url(self): """Test that preview url shows up correctly""" From aed872084694c944a635b8a9170c4ee876437458 Mon Sep 17 00:00:00 2001 From: George Schneeloch Date: Thu, 23 Jul 2015 11:34:42 -0400 Subject: [PATCH 11/38] Removed export view which isn't used anymore --- ui/templates/includes/learning_resource.html | 2 +- ui/tests/test_learningresources_views.py | 23 ----------------- ui/urls.py | 6 +---- ui/views.py | 27 +------------------- 4 files changed, 3 insertions(+), 55 deletions(-) diff --git a/ui/templates/includes/learning_resource.html b/ui/templates/includes/learning_resource.html index 4131ab6b..c91e2087 100644 --- a/ui/templates/includes/learning_resource.html +++ b/ui/templates/includes/learning_resource.html @@ -26,7 +26,7 @@

- + {% if result.object.title %} {{ result.object.title }} {% else %} diff --git a/ui/tests/test_learningresources_views.py b/ui/tests/test_learningresources_views.py index 8450b51c..6a220058 100644 --- a/ui/tests/test_learningresources_views.py +++ b/ui/tests/test_learningresources_views.py @@ -9,7 +9,6 @@ from django.conf import settings from django.core.files.storage import default_storage -from django.core.urlresolvers import reverse import ui.urls from learningresources.models import Repository, StaticAsset @@ -231,28 +230,6 @@ def test_access_without_login(self): self.assertEqual(len(response.redirect_chain), 2) self.assertTrue(302 in response.redirect_chain[0]) - def test_export_good(self): - """Get raw XML of something I should be allowed to see.""" - url = reverse("export", args=(self.repo.slug, self.resource.id)) - resp = self.client.get(url, follow=True) - body = resp.content.decode("utf-8") - self.assertTrue(resp.status_code == HTTP_OK) - self.assertTrue(self.resource.content_xml in body) - - def test_export_no_permission(self): - """Get raw XML of something I should not be allowed to see.""" - url = reverse("export", args=(self.repo.slug, self.resource.id)) - self.logout() - self.login(self.USERNAME_NO_REPO) - resp = self.client.get(url, follow=True) - self.assertTrue(resp.status_code == UNAUTHORIZED) - - def test_export_nonexistent(self): - """Get raw XML of something than does not exist.""" - url = reverse("export", args=(self.repo.slug, 999999)) - resp = self.client.get(url, follow=True) - self.assertTrue(resp.status_code == NOT_FOUND) - def test_repo_url(self): """Hit repo site normally.""" resp = self.client.get(self.repository_url, follow=True) diff --git a/ui/urls.py b/ui/urls.py index 7444de4d..731ad410 100644 --- a/ui/urls.py +++ b/ui/urls.py @@ -23,7 +23,7 @@ from search.forms import SearchForm from ui.views import ( - welcome, create_repo, export, + welcome, create_repo, upload, RepositoryView, serve_media ) import rest.urls as rest_urls @@ -46,10 +46,6 @@ ), name='repositories' ), - url(r'^repositories/(?P[-\w]+)/' - r'learningresources/(?P\d+)/$', - export, - name='export'), url(r'^repositories/(?P[-\w]+)/import/$', upload, name='upload'), ] diff --git a/ui/views.py b/ui/views.py index 51e52378..af9f248e 100644 --- a/ui/views.py +++ b/ui/views.py @@ -12,7 +12,7 @@ from django.core.exceptions import PermissionDenied from django.core.servers.basehttp import FileWrapper from django.core.urlresolvers import reverse -from django.http import HttpResponse, Http404, StreamingHttpResponse +from django.http import Http404, StreamingHttpResponse from django.shortcuts import ( render, redirect, @@ -25,7 +25,6 @@ from learningresources.api import ( get_repo, get_repos, - get_resource, NotFound, PermissionDenied as LorePermissionDenied, ) @@ -259,30 +258,6 @@ def build_form(self, form_kwargs=None): return super(RepositoryView, self).build_form(form_kwargs) -# pylint: disable=unused-argument -# repo_slug argument will be used by the decorator to protect the view -@login_required -@permission_required_or_403( - # pylint: disable=protected-access - # the following string is "learningresources.import_course" - # (unless the Repository model has been moved) - '{}.{}'.format( - Repository._meta.app_label, - RepoPermission.view_repo[0] - ), - (Repository, 'slug', 'repo_slug') -) -def export(request, repo_slug, resource_id): - """Dump LearningResource as XML""" - try: - return HttpResponse( - get_resource(resource_id, request.user.id).content_xml, - content_type='text/xml' - ) - except NotFound: - raise Http404() - - @login_required def serve_media(request, media_path): """ From bc9787e08ca1cfe6e1cb4197a233917729c917aa Mon Sep 17 00:00:00 2001 From: Giovanni Di Milia Date: Tue, 21 Jul 2015 11:58:00 -0400 Subject: [PATCH 12/38] Added description path to listing page --- importer/api/__init__.py | 21 +- learningresources/api.py | 44 ++++- learningresources/management/__init__.py | 0 .../management/commands/__init__.py | 0 .../commands/update_description_paths.py | 22 +++ .../migrations/0006_move_datestamp_data.py | 1 + ...8_remove_staticasset_learning_resources.py | 1 + .../0012_learningresource_description_path.py | 21 ++ .../0013_populate_description_path.py | 27 +++ learningresources/models.py | 1 + learningresources/tests/base.py | 23 ++- learningresources/tests/test_api.py | 181 ++++++++++++++---- ui/templates/includes/learning_resource.html | 3 + ui/tests/test_learningresources_views.py | 12 ++ 14 files changed, 310 insertions(+), 47 deletions(-) create mode 100644 learningresources/management/__init__.py create mode 100644 learningresources/management/commands/__init__.py create mode 100644 learningresources/management/commands/update_description_paths.py create mode 100644 learningresources/migrations/0012_learningresource_description_path.py create mode 100644 learningresources/migrations/0013_populate_description_path.py diff --git a/importer/api/__init__.py b/importer/api/__init__.py index fcf915b2..31aa51ce 100644 --- a/importer/api/__init__.py +++ b/importer/api/__init__.py @@ -18,8 +18,12 @@ from xbundle import XBundle, DESCRIPTOR_TAGS from learningresources.api import ( - create_course, create_resource, import_static_assets, - create_static_asset, get_video_sub, + create_course, + create_resource, + import_static_assets, + create_static_asset, + get_video_sub, + join_description_paths, ) from learningresources.models import StaticAsset, course_asset_basepath @@ -123,11 +127,11 @@ def import_course(bundle, repo_id, user_id, static_dir): user_id=user_id, ) import_static_assets(course, static_dir) - import_children(course, src, None) + import_children(course, src, None, '') return course -def import_children(course, element, parent): +def import_children(course, element, parent, parent_dpath): """ Create LearningResource instances for each element of an XML tree. @@ -137,16 +141,21 @@ def import_children(course, element, parent): element (lxml.etree): XML element within xbundle parent (learningresources.models.LearningResource): Parent LearningResource + parent_dpath (unicode): parent description path Returns: None """ + # pylint: disable=too-many-locals + title = element.attrib.get("display_name", "MISSING") mpath = etree.ElementTree(element).getpath(element) + dpath = join_description_paths(parent_dpath, title) resource = create_resource( course=course, parent=parent, resource_type=element.tag, - title=element.attrib.get("display_name", "MISSING"), + title=title, content_xml=etree.tostring(element), mpath=mpath, url_name=element.attrib.get("url_name", None), + dpath=dpath, ) target = "/static/" if element.tag == "video": @@ -185,4 +194,4 @@ def import_children(course, element, parent): for child in element.getchildren(): if child.tag in DESCRIPTOR_TAGS: - import_children(course, child, resource) + import_children(course, child, resource, dpath) diff --git a/learningresources/api.py b/learningresources/api.py index d9c1dad8..125ab032 100644 --- a/learningresources/api.py +++ b/learningresources/api.py @@ -79,7 +79,8 @@ def create_course(org, repo_id, course_number, run, user_id): # pylint: disable=too-many-arguments def create_resource( - course, parent, resource_type, title, content_xml, mpath, url_name + course, parent, resource_type, title, content_xml, mpath, url_name, + dpath ): """ Create a learning resource. @@ -93,6 +94,8 @@ def create_resource( content_xml (unicode): XML mpath (unicode): Materialized path url_name (unicode): Resource identifier + dpath (unicode): Description path + Returns: resource (learningresources.models.LearningResource): New LearningResource @@ -104,6 +107,7 @@ def create_resource( "content_xml": content_xml, "materialized_path": mpath, "url_name": url_name, + "description_path": dpath, } if parent is not None: params["parent_id"] = parent.id @@ -301,3 +305,41 @@ def import_static_assets(course, path): name = join(root, name).replace(path + sep, '', 1) django_file.name = name create_static_asset(course.id, django_file) + + +def join_description_paths(*args): + """ + Helper function to format the description path. + Args: + args (unicode): description path + Returns: + unicode: Formatted dpath + """ + return ' / '.join([dpath for dpath in args if dpath != '']) + + +def update_description_path(resource, force_parent_update=False): + """ + Updates the specified learning resource description path + based on the current title and the parent's description path + Args: + resource (learningresources.models.LearningResource): LearningResource + force_parent_update (boolean): force parent update + Returns: + None + """ + description_path = '' + if resource.parent is None: + description_path = join_description_paths(resource.title) + else: + # if the parent doesn't have a description_path update first the parent + if resource.parent.description_path == '' or force_parent_update: + update_description_path(resource.parent, force_parent_update) + # the current description path is + # the parent's one plus the current title + description_path = join_description_paths( + resource.parent.description_path, + resource.title + ) + resource.description_path = description_path + resource.save() diff --git a/learningresources/management/__init__.py b/learningresources/management/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/learningresources/management/commands/__init__.py b/learningresources/management/commands/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/learningresources/management/commands/update_description_paths.py b/learningresources/management/commands/update_description_paths.py new file mode 100644 index 00000000..5171438d --- /dev/null +++ b/learningresources/management/commands/update_description_paths.py @@ -0,0 +1,22 @@ +""" +Shell command to synchronize permissions and apply the latest to all groups +""" + +from __future__ import unicode_literals + +from django.core.management.base import BaseCommand + +from learningresources.api import update_description_path +from learningresources.models import LearningResource + + +class Command(BaseCommand): + """ + Command for sync_permissions + """ + help = "Synchronizes and updates the description_path" + + def handle(self, *args, **options): + """Command handler""" + for learning_resource in LearningResource.objects.all(): + update_description_path(learning_resource) diff --git a/learningresources/migrations/0006_move_datestamp_data.py b/learningresources/migrations/0006_move_datestamp_data.py index ca5b6065..649826ed 100644 --- a/learningresources/migrations/0006_move_datestamp_data.py +++ b/learningresources/migrations/0006_move_datestamp_data.py @@ -15,6 +15,7 @@ def transfer_datetimes(apps, schema_editor): Repository = apps.get_model("learningresources", "Repository") Repository.objects.all().update(date_created=F("create_date")) + class Migration(migrations.Migration): dependencies = [ diff --git a/learningresources/migrations/0008_remove_staticasset_learning_resources.py b/learningresources/migrations/0008_remove_staticasset_learning_resources.py index ce5017c1..7ff92cf4 100644 --- a/learningresources/migrations/0008_remove_staticasset_learning_resources.py +++ b/learningresources/migrations/0008_remove_staticasset_learning_resources.py @@ -5,6 +5,7 @@ # pylint: skip-file + class Migration(migrations.Migration): dependencies = [ diff --git a/learningresources/migrations/0012_learningresource_description_path.py b/learningresources/migrations/0012_learningresource_description_path.py new file mode 100644 index 00000000..d9642026 --- /dev/null +++ b/learningresources/migrations/0012_learningresource_description_path.py @@ -0,0 +1,21 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import models, migrations + +# pylint: skip-file + + +class Migration(migrations.Migration): + + dependencies = [ + ('learningresources', '0011_learningresource_url_name'), + ] + + operations = [ + migrations.AddField( + model_name='learningresource', + name='description_path', + field=models.TextField(blank=True), + ), + ] diff --git a/learningresources/migrations/0013_populate_description_path.py b/learningresources/migrations/0013_populate_description_path.py new file mode 100644 index 00000000..0fb0f6aa --- /dev/null +++ b/learningresources/migrations/0013_populate_description_path.py @@ -0,0 +1,27 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import models, migrations +from django.db.models import F + +from learningresources.api import update_description_path + +# pylint: skip-file + + +def populate_description_paths(apps, schema_editor): + """Populate the description path field""" + LearningResource = apps.get_model("learningresources", "LearningResource") + for learning_resource in LearningResource.objects.all(): # pragma: no cover + update_description_path(learning_resource) # pragma: no cover + + +class Migration(migrations.Migration): + + dependencies = [ + ('learningresources', '0012_learningresource_description_path'), + ] + + operations = [ + migrations.RunPython(populate_description_paths) + ] diff --git a/learningresources/models.py b/learningresources/models.py index ebb2fb86..3ceb096f 100644 --- a/learningresources/models.py +++ b/learningresources/models.py @@ -125,6 +125,7 @@ class LearningResource(BaseModel): description = models.TextField(blank=True) content_xml = models.TextField() materialized_path = models.TextField() + description_path = models.TextField(blank=True) url_path = models.TextField() parent = models.ForeignKey('self', null=True, blank=True) copyright = models.TextField() diff --git a/learningresources/tests/base.py b/learningresources/tests/base.py index 8b57c43b..7ef0eabe 100644 --- a/learningresources/tests/base.py +++ b/learningresources/tests/base.py @@ -20,7 +20,12 @@ from django.test.testcases import TestCase import haystack -from learningresources.api import create_repo, create_course, create_resource +from learningresources.api import ( + create_repo, + create_course, + create_resource, + update_description_path +) from learningresources.models import Repository, StaticAsset from roles.api import assign_user_to_repo_group from roles.permissions import GroupTypes @@ -61,17 +66,19 @@ def create_resource(self, **kwargs): """Creates a learning resource with extra fields""" learn_res = create_resource( course=self.course, - parent=None, - resource_type=kwargs.get('resource_type') or "example", - title=kwargs.get('title') or "other silly example", - content_xml=kwargs.get('content_xml') or "other blah", - mpath=kwargs.get('mpath') or "/otherblah", - url_name=kwargs.get('url_name') or None + parent=kwargs.get('parent'), + resource_type=kwargs.get('resource_type', "example"), + title=kwargs.get('title', "other silly example"), + content_xml=kwargs.get('content_xml', "other blah"), + mpath=kwargs.get('mpath', "/otherblah"), + url_name=kwargs.get('url_name'), + dpath='' ) learn_res.xa_nr_views = kwargs.get('xa_nr_views', 0) learn_res.xa_nr_attempts = kwargs.get('xa_nr_attempts', 0) learn_res.xa_avg_grade = kwargs.get('xa_avg_grade', 0) learn_res.save() + update_description_path(learn_res) return learn_res def setUp(self): @@ -99,7 +106,7 @@ def setUp(self): run="Febtober", user_id=self.user.id, ) - self.resource = create_resource( + self.resource = self.create_resource( course=self.course, parent=None, resource_type="example", diff --git a/learningresources/tests/test_api.py b/learningresources/tests/test_api.py index bf589ec8..37894b51 100644 --- a/learningresources/tests/test_api.py +++ b/learningresources/tests/test_api.py @@ -8,25 +8,18 @@ from django.core.files import File from django.core.files.storage import default_storage +from django.core.management import call_command from django.db.utils import IntegrityError import tempfile -from learningresources.api import ( - create_course, - create_repo, - create_static_asset, - get_resource, - get_repo, - get_repos, - NotFound, - PermissionDenied, -) +from importer.api import import_course_from_file +from learningresources import api from learningresources.models import ( Course, LearningResource, static_asset_basepath, ) -from importer.api import import_course_from_file +from mock import patch from .base import LoreTestCase log = logging.getLogger(__name__) @@ -65,19 +58,20 @@ def test_create(self): Create a course. """ before = course_count() - course = create_course(**self.kwargs) + course = api.create_course(**self.kwargs) after = course_count() self.assertTrue(after == before + 1) self.assertTrue(course is not None) # Can't create a duplicate. with self.assertRaises(ValueError): - create_course(**self.kwargs) + api.create_course(**self.kwargs) self.assertTrue(course_count() == after) # NOT NULL constraint fails on org with self.assertRaises(IntegrityError) as ex: - create_course(None, self.repo.id, "course", "run", self.user.id) + api.create_course( + None, self.repo.id, "course", "run", self.user.id) self.assertIn("org", ex.exception.args[0]) @@ -98,7 +92,7 @@ def test_get_resource(self): resource_id = LearningResource.objects.all()[0].id self.assertTrue( isinstance( - get_resource(resource_id, self.user.id), + api.get_resource(resource_id, self.user.id), LearningResource ) ) @@ -106,14 +100,14 @@ def test_get_resource(self): def test_get_missing_resource(self): """Get a resource""" resource_id = LearningResource.objects.all().order_by("-id")[0].id + 1 - with self.assertRaises(NotFound): - get_resource(resource_id, self.user.id) + with self.assertRaises(api.NotFound): + api.get_resource(resource_id, self.user.id) def test_get_no_permission_resource(self): """Get a resource""" resource_id = LearningResource.objects.all()[0].id - with self.assertRaises(PermissionDenied): - get_resource(resource_id, self.user_norepo.id) + with self.assertRaises(api.PermissionDenied): + api.get_resource(resource_id, self.user_norepo.id) class TestRepoAPI(LoreTestCase): @@ -123,16 +117,16 @@ class TestRepoAPI(LoreTestCase): def test_get_repo(self): """repo does not exist""" # this should not fail - get_repo(self.repo.slug, self.user.id) + api.get_repo(self.repo.slug, self.user.id) self.assertRaises( - NotFound, - get_repo, + api.NotFound, + api.get_repo, "nonexistent_repo", self.user.id ) self.assertRaises( - PermissionDenied, - get_repo, + api.PermissionDenied, + api.get_repo, self.repo.slug, self.user_norepo.id ) @@ -140,20 +134,20 @@ def test_get_repo(self): def test_get_repos(self): """test get_repos""" self.assertEqual([self.repo], - list(get_repos(self.user.id))) + list(api.get_repos(self.user.id))) self.assertEqual([], - list(get_repos(self.user_norepo.id))) - with self.assertRaises(PermissionDenied): - get_repos(-1) + list(api.get_repos(self.user_norepo.id))) + with self.assertRaises(api.PermissionDenied): + api.get_repos(-1) def test_invalid_create_repo(self): """Create a repository with empty name and description""" with self.assertRaises(IntegrityError) as ex: - create_repo(None, "description", self.user.id) + api.create_repo(None, "description", self.user.id) self.assertIn("name", ex.exception.args[0]) with self.assertRaises(IntegrityError) as ex: - create_repo("name", None, self.user.id) + api.create_repo("name", None, self.user.id) self.assertIn("description", ex.exception.args[0]) @@ -177,7 +171,7 @@ def test_create_static_asset(self): with tempfile.TemporaryFile() as temp: temp.write(file_contents) test_file = File(temp, name=basename) - asset = create_static_asset(self.course.id, test_file) + asset = api.create_static_asset(self.course.id, test_file) self.addCleanup(default_storage.delete, asset.asset) self.assertFalse(test_file.closed) self.assertEqual(asset.course, self.course) @@ -190,5 +184,128 @@ def test_create_static_asset(self): test_file.close() test_file.name = 'new_name' with self.assertRaises(ValueError): - asset = create_static_asset(self.course.id, test_file) + asset = api.create_static_asset(self.course.id, test_file) self.assertEqual(file_contents, asset.asset.read()) + + +class TestDescriptionPath(LoreTestCase): + """ + Tests the Python API for the description paths + """ + def test_join_description_path(self): + """Test for join_description_path""" + self.assertEqual( + api.join_description_paths(''), + '' + ) + self.assertEqual( + api.join_description_paths('', 'foo'), + 'foo' + ) + self.assertEqual( + api.join_description_paths('foo', ''), + 'foo' + ) + self.assertEqual( + api.join_description_paths('foo', '', 'bar'), + 'foo / bar' + ) + self.assertEqual( + api.join_description_paths('foo', 'bar', 'b az'), + 'foo / bar / b az' + ) + + def test_update_description_path(self): + """Tests for update_description_path""" + # after created a resource without parent has the description path + # equal to the title + self.assertIsNone(self.resource.parent) + self.assertEqual( + self.resource.title, + self.resource.description_path + ) + # changing the title does not update the description path automatically + self.resource.title = '123 xyz' + self.resource.save() + self.assertNotEqual( + self.resource.title, + self.resource.description_path + ) + # update the description path + api.update_description_path(self.resource) + self.assertEqual( + self.resource.title, + self.resource.description_path + ) + # create a child resource + child_res = self.create_resource( + parent=self.resource + ) + # the description path is the combination of the child resource title + # and the parent description path + self.assertEqual( + child_res.description_path, + api.join_description_paths( + self.resource.description_path, + child_res.title + ) + ) + # change both resources title + self.resource.title = '1234 xyza' + self.resource.save() + child_res.title = 'foo 1234' + child_res.save() + # note: child_res.parent and self.resource are 2 different instances + # of the same record, but they need to be refreshed separately + # after a change made to one of them + child_res.parent.refresh_from_db() + # update the description path of the child + # will not update the parent's one + api.update_description_path(child_res) + self.assertNotEqual( + self.resource.title, + self.resource.description_path + ) + self.assertEqual( + child_res.description_path, + api.join_description_paths( + self.resource.description_path, + child_res.title + ) + ) + # but the parent's update can be forced + api.update_description_path(child_res, force_parent_update=True) + self.resource.refresh_from_db() + self.assertEqual( + self.resource.title, + self.resource.description_path + ) + self.assertEqual( + child_res.description_path, + api.join_description_paths( + self.resource.description_path, + child_res.title + ) + ) + # removing the description path of the parent + self.resource.description_path = '' + self.resource.title = '999 new title' + self.resource.save() + child_res.parent.refresh_from_db() + # the update of the child will update the parent without forcing it + api.update_description_path(child_res) + self.resource.refresh_from_db() + self.assertEqual( + self.resource.title, + self.resource.description_path + ) + + def test_update_description_path_command(self): + """ + test the update_description_paths via manage.py + """ + # pylint: disable=invalid-name, no-self-use + with patch.object(api, 'update_description_path') as mock_method: + mock_method.return_value = None + call_command('update_description_paths') + mock_method.assert_called_with(self.resource) diff --git a/ui/templates/includes/learning_resource.html b/ui/templates/includes/learning_resource.html index 4131ab6b..08e22b44 100644 --- a/ui/templates/includes/learning_resource.html +++ b/ui/templates/includes/learning_resource.html @@ -35,6 +35,9 @@

+
+ {{ result.object.description_path }} +
{% if result.object.description %} {{ result.object.description }} diff --git a/ui/tests/test_learningresources_views.py b/ui/tests/test_learningresources_views.py index 8450b51c..a4995c80 100644 --- a/ui/tests/test_learningresources_views.py +++ b/ui/tests/test_learningresources_views.py @@ -321,6 +321,18 @@ def test_listing_with_sorting(self): body ) + def test_description_path(self): + """Tests that the description path is in the listing page""" + dpath_html = '{0}'.format( + self.resource.description_path + ) + body = self.assert_status_code( + self.repository_url, + HTTP_OK, + return_body=True + ) + self.assertIn(dpath_html, body) + def test_serve_media(self): """Hit serve media""" self.assertEqual( From cdf14ff22cfa55dfc4a76485b9e0e99d0781bba9 Mon Sep 17 00:00:00 2001 From: Amir Qayyum Khan Date: Fri, 10 Jul 2015 16:06:44 +0500 Subject: [PATCH 13/38] Extended tests for manage_taxonomies.jsx file --- ui/jstests/test_manage_taxonomies.jsx | 868 +++++++++++++++++++++++++- ui/static/ui/js/manage_taxonomies.jsx | 9 +- 2 files changed, 853 insertions(+), 24 deletions(-) diff --git a/ui/jstests/test_manage_taxonomies.jsx b/ui/jstests/test_manage_taxonomies.jsx index d86997ea..45de2737 100644 --- a/ui/jstests/test_manage_taxonomies.jsx +++ b/ui/jstests/test_manage_taxonomies.jsx @@ -5,13 +5,10 @@ define(['QUnit', 'jquery', 'setup_manage_taxonomies', 'reactaddons', var VocabularyComponent = ManageTaxonomies.VocabularyComponent; var waitForAjax = TestUtils.waitForAjax; - - var addTermResponse = { - "id": 9, - "slug": "aa", - "label": "aa", - "weight": 1 - }; + var TermComponent = ManageTaxonomies.TermComponent; + var AddTermsComponent = ManageTaxonomies.AddTermsComponent; + var AddVocabulary = ManageTaxonomies.AddVocabulary; + var TaxonomyComponent = ManageTaxonomies.TaxonomyComponent; var vocabulary = { "id": 1, "slug": "difficulty", @@ -20,6 +17,9 @@ define(['QUnit', 'jquery', 'setup_manage_taxonomies', 'reactaddons', "vocabulary_type": "f", "required": false, "weight": 2147483647, + "learning_resource_types": [ + "course" + ], "terms": [ { "id": 1, @@ -35,49 +35,297 @@ define(['QUnit', 'jquery', 'setup_manage_taxonomies', 'reactaddons', } ] }; - function reportError(msg) { - if (msg) { - console.error(msg); - } + /** + * Common assertions for Add terms, use in term and taxonomy + * components tests + * @param {QUnit.assert} assert - For assertions + * @param {ReactComponent} component - Root DOM object in react class + * @return {ReactComponent} devItems - DOM object of class 'panel-body' + * */ + function assertAddTermCommon(assert, component) { + // check term title + var listOfLinks = React.addons.TestUtils. + scryRenderedDOMComponentsWithTag( + component, + 'a' + ); + assert.equal(listOfLinks.length, 2); + var linkHeader = React.findDOMNode(listOfLinks[0]); + assert.equal(linkHeader.innerHTML, vocabulary.name); + //test items + var devItems = React.addons.TestUtils. + findRenderedDOMComponentWithClass( + component, + 'panel-body' + ); + var itemList = React.addons.TestUtils. + scryRenderedDOMComponentsWithTag( + devItems, + 'li' + ); + assert.equal(itemList.length, 2); + return devItems; } QUnit.module('Test taxonomies panel', { beforeEach: function() { + var learningResourceTypes = { + "count": 8, + "next": null, + "previous": null, + "results": [ + { + "name": "course" + }, + { + "name": "chapter" + }, + { + "name": "sequential" + }, + { + "name": "vertical" + }, + { + "name": "html" + }, + { + "name": "video" + }, + { + "name": "discussion" + }, + { + "name": "problem" + } + ] + }; + var listOfVocabularies = { + "count": 1, + "next": null, + "previous": null, + "results": [ + { + "id": 1, + "slug": "difficulty", + "name": "difficulty", + "description": "easy", + "vocabulary_type": "f", + "required": false, + "weight": 2147483647, + "terms": vocabulary.terms + } + ] + }; + var listOfTerms = { + "count": 2, + "next": null, + "previous": null, + "results": vocabulary.terms + }; + var term = { + "id": 9, + "slug": "test", + "label": "test", + "weight": 1 + }; + var duplicateVocabularyResponse = { + "non_field_errors":[ + "The fields repository, name must make a unique set." + ] + }; TestUtils.setup(); TestUtils.initMockjax({ - url: "/api/v1/repositories/*/vocabularies/*/terms/", - responseText: addTermResponse, + url: "/api/v1/repositories/repo/vocabularies/easy/terms/", + type: "POST", + status: 400 + }); + TestUtils.initMockjax({ + url: "/api/v1/repositories/repo/vocabularies/difficulty/terms/", + responseText: term, type: "POST" }); + TestUtils.initMockjax({ + url: "/api/v1/repositories/repo/vocabularies/difficulty/terms/", + responseText: listOfTerms, + type: "GET" + }); + TestUtils.initMockjax({ + url: "/api/v1/repositories/repo/vocabularies/", + contentType: "application/json; charset=utf-8", + responseText: vocabulary, + dataType: 'json', + type: "POST" + }); + TestUtils.initMockjax({ + url: "/api/v1/repositories/repo2/vocabularies/", + type: "POST", + status: 400 + }); + TestUtils.initMockjax({ + url: "/api/v1/repositories/repo3/vocabularies/", + contentType: "application/json; charset=utf-8", + responseText: duplicateVocabularyResponse, + dataType: 'json', + type: "POST", + status: 400 + }); + TestUtils.initMockjax({ + url: "/api/v1/repositories/repo/vocabularies/", + contentType: "application/json; charset=utf-8", + responseText: listOfVocabularies, + dataType: 'json', + type: "GET" + }); + TestUtils.initMockjax({ + url: "/api/v1/learning_resource_types/", + contentType: "application/json; charset=utf-8", + responseText: learningResourceTypes, + dataType: 'json', + type: "GET" + }); }, afterEach: function() { TestUtils.cleanup(); } }); + QUnit.test('Assert that TermComponent renders properly', + function(assert) { + assert.ok(TermComponent, "class object not found"); + var term = { + "id": 9, + "slug": "test", + "label": "test", + "weight": 1 + }; + var termComponentRendered = React.addons.TestUtils. + renderIntoDocument( + + ); + + var labelComponent = React.addons.TestUtils. + findRenderedDOMComponentWithTag( + termComponentRendered, + 'label' + ); + //testing label value render + var label = labelComponent.getDOMNode(); + assert.equal(label.innerHTML, term.label); + } + ); + QUnit.test('Assert that VocabularyComponent renders properly', function(assert) { assert.ok(VocabularyComponent, "class object not found"); - var done = assert.async(); var addTermCalled = 0; var addTerm = function() { addTermCalled += 1; }; - + var reportError = function() {}; var afterMount = function(component) { var node = React.findDOMNode(component); var textbox = $(node).find("input")[0]; - React.addons.TestUtils.Simulate.keyUp(textbox, {key: "x"}); - assert.equal(addTermCalled, 0); + // check term title + var listOfLinks = React.addons.TestUtils. + scryRenderedDOMComponentsWithTag( + component, + 'a' + ); + assert.equal(listOfLinks.length, 2); + var linkHeader = React.findDOMNode(listOfLinks[0]); + assert.equal(linkHeader.innerHTML, vocabulary.name); + + //test items + var devItems = React.addons.TestUtils. + findRenderedDOMComponentWithClass( + component, + 'panel-body' + ); + var itemList = React.addons.TestUtils. + scryRenderedDOMComponentsWithTag( + devItems, + 'li' + ); + assert.equal(itemList.length, 2); + //test enter text in input text + var inputNode = React.addons.TestUtils. + findRenderedDOMComponentWithTag( + component, + 'input' + ); + React.addons.TestUtils.Simulate.change( + inputNode, + {target: {value: 'test12'}} + ); + component.forceUpdate(function() { + assert.equal( + 'test12', + component.state.newTermLabel + ); + React.addons.TestUtils.Simulate.keyUp(textbox, {key: "x"}); + assert.equal(addTermCalled, 0); + + React.addons.TestUtils.Simulate.keyUp(textbox, {key: "Enter"}); + waitForAjax(1, function() { + assert.equal(addTermCalled, 1); + done(); + }); + }); + }; + React.addons.TestUtils. + renderIntoDocument( + + ); + } + ); + + QUnit.test('Assert that ajax fail VocabularyComponent', + function(assert) { + var done = assert.async(); + var vocabulary = { + "id": 2, + "slug": "easy", + "name": "easy", + "description": "easy", + "vocabulary_type": "m", + "required": false, + "weight": 2147483647, + "terms": [] + }; + var addTermCalled = 0; + var errorMessage; + var addTerm = function() { + addTermCalled += 1; + }; + var reportError = function(msg) { + errorMessage = msg; + }; + var afterMount = function(component) { + // wait for calls to populate form + var node = React.findDOMNode(component); + var textbox = $(node).find("input")[0]; React.addons.TestUtils.Simulate.keyUp(textbox, {key: "Enter"}); - waitForAjax(1, function() { - assert.equal(addTermCalled, 1); + waitForAjax(1, function () { + assert.equal(addTermCalled, 0); + assert.equal(//Error is caused by a 400 status code + errorMessage, + "Error occurred while adding new term." + ); done(); }); }; - React.addons.TestUtils. renderIntoDocument( + ); + } + ); + + QUnit.test('Assert that AddVocabulary work properly', + function(assert) { + assert.ok(AddVocabulary, "class object not found"); + var vocabularies = [ + { + "vocabulary": vocabulary, + "terms": vocabulary.terms + } + ]; + var learningResourceTypes = ['course', 'chapter', 'sequential', + 'vertical', 'html', 'video', 'discussion', 'problem']; + var saveVocabularyResponse; + var done = assert.async(); + var updateParent = function(data) { + saveVocabularyResponse = data; + }; + var afterMount = function(component) { + assert.equal( + component.state.name, + '' + ); + assert.equal( + component.state.description, + '' + ); + assert.equal( + component.state.vocabularyType, + 'm' + ); + assert.equal( + component.state.learningResourceTypes.length, + 0 + ); + var formNode = React.addons.TestUtils. + findRenderedDOMComponentWithClass( + component, + 'form-horizontal' + ); + assert.ok(formNode); + //test form submission + var inputNodes = React.addons.TestUtils. + scryRenderedDOMComponentsWithTag( + formNode, + 'input' + ); + assert.equal(inputNodes.length, 12); + var inputVocabularyName = inputNodes[0]; + var inputVocabularyDesc = inputNodes[1]; + var radioTagStyle = inputNodes[11]; + + React.addons.TestUtils.Simulate.change( + inputVocabularyName, {target: {value: "TestA"}} + ); + React.addons.TestUtils.Simulate.change( + inputVocabularyDesc, {target: {value: "TestA"}} + ); + React.addons.TestUtils.Simulate.change( + radioTagStyle, + {target: {value: 'f'}} + ); + component.forceUpdate(function() { + assert.equal(component.state.name, "TestA"); + assert.equal(component.state.vocabularyType, "f"); + assert.equal(component.state.description, "TestA"); + assert.equal( + component.state.learningResourceTypes.length, + 0 + ); + React.addons.TestUtils.Simulate.submit(formNode); + waitForAjax(1, function() { + assert.equal( + saveVocabularyResponse.id, + vocabulary.id + ); + //testing state is reset + assert.equal( + component.state.name, + '' + ); + assert.equal( + component.state.description, + '' + ); + assert.equal( + component.state.vocabularyType, + 'm' + ); + assert.equal( + component.state.learningResourceTypes.length, + 0 + ); + var inputNodes = React.addons.TestUtils. + scryRenderedDOMComponentsWithTag( + formNode, + 'input' + ); + var inputVocabularyName = inputNodes[0]; + var inputVocabularyDesc = inputNodes[1]; + var checkboxCourse = inputNodes[2]; + var radioTagStyle = inputNodes[11]; + + React.addons.TestUtils.Simulate.change( + inputVocabularyName, {target: {value: "TestB"}} + ); + React.addons.TestUtils.Simulate.change( + inputVocabularyDesc, {target: {value: "TestB"}} + ); + React.addons.TestUtils.Simulate.change( + checkboxCourse, + {target: {value: 'course', checked: true}} + ); + React.addons.TestUtils.Simulate.change( + radioTagStyle, + {target: {value: 'f', checked: true}} + ); + component.forceUpdate(function() { + assert.equal(component.state.name, "TestB"); + assert.equal(component.state.description, "TestB"); + assert.equal(component.state.vocabularyType, "f"); + assert.equal( + component.state.learningResourceTypes[0], + 'course' + ); + React.addons.TestUtils.Simulate.submit(formNode); + waitForAjax(1, function() { + //testing state is reset + assert.equal( + component.state.name, + '' + ); + assert.equal( + component.state.description, + '' + ); + assert.equal( + component.state.vocabularyType, + 'm' + ); + assert.equal( + component.state.learningResourceTypes.length, + 0 + ); + assert.equal( + saveVocabularyResponse.id, + vocabulary.id + ); + React.addons.TestUtils.Simulate.change( + inputVocabularyName, {target: {value: "TestC"}} + ); + React.addons.TestUtils.Simulate.change( + inputVocabularyDesc, {target: {value: "TestC"}} + ); + React.addons.TestUtils.Simulate.change( + checkboxCourse, + {target: {value: 'course', checked: true}} + ); + React.addons.TestUtils.Simulate.change( + radioTagStyle, + {target: {value: 'f', checked: true}} + ); + + component.forceUpdate(function() { + assert.equal(component.state.name, "TestC"); + assert.equal(component.state.description, "TestC"); + assert.equal(component.state.vocabularyType, "f"); + assert.equal( + component.state.learningResourceTypes[0], + 'course' + ); + //uncheck checkbox + React.addons.TestUtils.Simulate.change( + checkboxCourse, + {target: {value: 'course', checked: false}} + ); + component.forceUpdate(function() { + assert.equal(component.state.name, "TestC"); + assert.equal(component.state.description, "TestC"); + assert.equal(component.state.vocabularyType, "f"); + //assert after uncheck course, learningResourceTypes is empty + assert.equal( + component.state.learningResourceTypes.length, + 0 + ); + React.addons.TestUtils.Simulate.submit(formNode); + waitForAjax(1, function() { + //testing state is reset + assert.equal( + component.state.name, + '' + ); + assert.equal( + component.state.description, + '' + ); + assert.equal( + component.state.vocabularyType, + 'm' + ); + assert.equal( + component.state.learningResourceTypes.length, + 0 + ); + assert.equal( + saveVocabularyResponse.id, + vocabulary.id + ); + done(); + }); + }); + }); + }); + }); + }); + }); + }; + React.addons.TestUtils. + renderIntoDocument( + + ); + } + ); + + QUnit.test('Assert that ajax fail in AddVocabulary', + function(assert) { + assert.ok(AddVocabulary, "class object not found"); + var vocabularies = [ + { + "vocabulary": vocabulary, + "terms": vocabulary.terms + } + ]; + var done = assert.async(); + var updateParent = function() {}; + var afterMount = function(component) { + var formNode = React.addons.TestUtils. + findRenderedDOMComponentWithClass( + component, + 'form-horizontal' + ); + assert.ok(formNode); + //test form submission + var inputNodes = React.addons.TestUtils. + scryRenderedDOMComponentsWithTag( + formNode, + 'input' + ); + assert.equal(inputNodes.length, 4); + var inputVocabularyName = inputNodes[0]; + var inputVocabularyDesc = inputNodes[1]; + var checkboxCourse = inputNodes[2]; + + React.addons.TestUtils.Simulate.change( + inputVocabularyName, {target: {value: "TestB"}} + ); + React.addons.TestUtils.Simulate.change( + inputVocabularyDesc, {target: {value: "TestB"}} + ); + React.addons.TestUtils.Simulate.change( + checkboxCourse, + {target: {value: 'course', checked: true}} + ); + React.addons.TestUtils.Simulate.submit(formNode); + waitForAjax(1, function() { + assert.equal(//Error is caused by a 400 status code + component.state.errorMessage, + "There was a problem adding the Vocabulary." + ); + done(); + }); + }; + React.addons.TestUtils. + renderIntoDocument( + + ); + } + ); + + QUnit.test('Assert that ajax fail in AddVocabulary: Duplicate Vocabulary', + function(assert) { + assert.ok(AddVocabulary, "class object not found"); + var vocabularies = [ + { + "vocabulary": vocabulary, + "terms": vocabulary.terms + } + ]; + var done = assert.async(); + var updateParent = function() {}; + var afterMount = function(component) { + var formNode = React.addons.TestUtils. + findRenderedDOMComponentWithClass( + component, + 'form-horizontal' + ); + assert.ok(formNode); + //test form submission + var inputNodes = React.addons.TestUtils. + scryRenderedDOMComponentsWithTag( + formNode, + 'input' + ); + assert.equal(inputNodes.length, 4); + var inputVocabularyName = inputNodes[0]; + var inputVocabularyDesc = inputNodes[1]; + var checkboxCourse = inputNodes[2]; + + React.addons.TestUtils.Simulate.change( + inputVocabularyName, {target: {value: "TestB"}} + ); + React.addons.TestUtils.Simulate.change( + inputVocabularyDesc, {target: {value: "TestB"}} + ); + React.addons.TestUtils.Simulate.change( + checkboxCourse, + {target: {value: 'course', checked: true}} + ); + React.addons.TestUtils.Simulate.submit(formNode); + waitForAjax(1, function() { + assert.equal(//Error is caused by a 400 status code + component.state.errorMessage, + 'A Vocabulary named "" already exists.' + + ' Please choose a different name.' + ); + done(); + }); + }; + React.addons.TestUtils. + renderIntoDocument( + + ); + } + ); + + QUnit.test('Assert that add term works in TaxonomyComponent', + function(assert) { + assert.ok(TaxonomyComponent, "class object not found"); + var vocabularies = [ + { + "vocabulary": vocabulary, + "terms": vocabulary.terms + } + ]; + var vocabularyWithoutTerms = { + "id": 2, + "slug": "difficulty2", + "name": "difficulty2", + "description": "easy", + "vocabulary_type": "f", + "required": false, + "weight": 2147483647, + }; + var done = assert.async(); + var afterMount = function(component) { + assert.equal( + component.state.vocabularies.length, + 0 + ); + assert.equal( + component.state.learningResourceTypes.length, + 0 + ); + waitForAjax(2, function() { + assert.equal( + component.state.vocabularies.length, + 1 + ); + assert.equal( + component.state.vocabularies[0].terms.length, + 2 + ); + assertAddTermCommon(assert, component); + // Adding second Vocabulary to make sure term added to correct vocabulary + component.addVocabulary(vocabularyWithoutTerms); + component.forceUpdate(function() { + assert.equal( + component.state.vocabularies.length, + 2 + ); + var inputGroup = React.addons.TestUtils. + scryRenderedDOMComponentsWithClass( + component, + 'input-group' + ); + assert.equal(inputGroup.length, 2); + var textbox = React.addons.TestUtils. + findRenderedDOMComponentWithTag( + inputGroup[0], + 'input' + ); + assert.equal( + component.state.vocabularies[0].terms.length, + 2 + ); + React.addons.TestUtils.Simulate.keyUp(textbox, {key: "Enter"}); + waitForAjax(1, function() { + assert.equal( + component.state.vocabularies[0].terms.length, + 3 + ); + done(); + }); + }); + }); + }; + React.addons.TestUtils.renderIntoDocument + ( + + ); + } + ); + QUnit.test('Assert that add vocabulary works in TaxonomyComponent', + function(assert) { + assert.ok(TaxonomyComponent, "class object not found"); + var vocabularies = [ + { + "vocabulary": vocabulary, + "terms": vocabulary.terms + } + ]; + var vocabularyWithoutTerms = { + "id": 2, + "slug": "difficulty2", + "name": "difficulty2", + "description": "easy", + "vocabulary_type": "f", + "required": false, + "weight": 2147483647, + }; + var done = assert.async(); + var afterMount = function(component) { + assert.equal( + component.state.vocabularies.length, + 0 + ); + assert.equal( + component.state.learningResourceTypes.length, + 0 + ); + waitForAjax(2, function() { + assert.equal( + component.state.vocabularies.length, + 1 + ); + assert.equal( + component.state.learningResourceTypes.length, + 8 + ); + var formNode = React.addons.TestUtils. + findRenderedDOMComponentWithClass( + component, + 'form-horizontal' + ); + assert.ok(formNode); + //test form submission + var inputNodes = React.addons.TestUtils. + scryRenderedDOMComponentsWithTag( + formNode, + 'input' + ); + var inputVocabularyName = inputNodes[0]; + var inputVocabularyDesc = inputNodes[1]; + var checkboxCourse = inputNodes[2]; + + React.addons.TestUtils.Simulate.change( + inputVocabularyName, {target: {value: "TestA"}} + ); + React.addons.TestUtils.Simulate.change( + inputVocabularyDesc, {target: {value: "TestA"}} + ); + React.addons.TestUtils.Simulate.change( + checkboxCourse, + {target: {value: 'course', checked: true}} + ); + React.addons.TestUtils.Simulate.submit(formNode); + waitForAjax(1, function() { + assert.equal( + component.state.vocabularies.length, + 2 + ); + component.addVocabulary(vocabularyWithoutTerms); + component.forceUpdate(function() { + assert.equal( + component.state.vocabularies.length, + 3 + ); + done(); + }); + }); + }); + }; + React.addons.TestUtils.renderIntoDocument + ( + + ); + } + ); }); diff --git a/ui/static/ui/js/manage_taxonomies.jsx b/ui/static/ui/js/manage_taxonomies.jsx index 05cf2355..246281ca 100644 --- a/ui/static/ui/js/manage_taxonomies.jsx +++ b/ui/static/ui/js/manage_taxonomies.jsx @@ -177,8 +177,7 @@ define('setup_manage_taxonomies', ['reactaddons', 'lodash', 'jquery', 'utils'], }).fail(function(data) { var jsonData = data.responseJSON; var i = 0; - if (jsonData.non_field_errors && - jsonData.non_field_errors.length > 0) { + if (jsonData && jsonData.non_field_errors) { for (i = 0; i < jsonData.non_field_errors.length ; i++) { if (jsonData.non_field_errors[i] === 'The fields repository, name must make a unique set.') { @@ -341,7 +340,6 @@ define('setup_manage_taxonomies', ['reactaddons', 'lodash', 'jquery', 'utils'], var types = _.map(learningResourceTypes, function(type) { return type.name; }); - thiz.setState({learningResourceTypes: types}); Utils.getVocabulariesAndTerms(thiz.props.repoSlug).then( function(vocabularies) { @@ -353,12 +351,15 @@ define('setup_manage_taxonomies', ['reactaddons', 'lodash', 'jquery', 'utils'], } ); }); - } }); return { 'VocabularyComponent': VocabularyComponent, + 'TermComponent': TermComponent, + 'AddTermsComponent': AddTermsComponent, + 'AddVocabulary': AddVocabulary, + 'TaxonomyComponent': TaxonomyComponent, 'loader' : function (repoSlug) { React.render( , From b221aa7b1344abf043a7d8e79defb08d27b4a421 Mon Sep 17 00:00:00 2001 From: Carson Gee Date: Wed, 22 Jul 2015 12:33:51 -0400 Subject: [PATCH 14/38] Added docker support for running worker or Web process by environment --- Dockerfile | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/Dockerfile b/Dockerfile index 3dafaf8a..077bcd70 100644 --- a/Dockerfile +++ b/Dockerfile @@ -45,7 +45,12 @@ ENV PATH /src/node_modules/.bin:/node/node_modules/.bin:$PATH # Set pip cache folder, as it is breaking pip when it is on a shared volume ENV XDG_CACHE_HOME /tmp/.cache -EXPOSE 8070 +# Gather static +RUN ./manage.py collectstatic --noinput USER mitodl -CMD uwsgi uwsgi.ini + +# Set and expose port for uwsgi config +EXPOSE 8070 +ENV PORT 8070 +CMD if [ -n "$WORKER" ]; then celery -A lore worker; else uwsgi uwsgi.ini; fi From 6bffff899bb94acfe6929911665a1084b8c07106 Mon Sep 17 00:00:00 2001 From: George Schneeloch Date: Fri, 24 Jul 2015 12:33:36 -0400 Subject: [PATCH 15/38] Changed response vocabulary name to match input and avoid key collision --- ui/jstests/test_manage_taxonomies.jsx | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/ui/jstests/test_manage_taxonomies.jsx b/ui/jstests/test_manage_taxonomies.jsx index 45de2737..e8f4723b 100644 --- a/ui/jstests/test_manage_taxonomies.jsx +++ b/ui/jstests/test_manage_taxonomies.jsx @@ -892,6 +892,20 @@ define(['QUnit', 'jquery', 'setup_manage_taxonomies', 'reactaddons', checkboxCourse, {target: {value: 'course', checked: true}} ); + TestUtils.replaceMockjax({ + url: "/api/v1/repositories/repo/vocabularies/", + type: "POST", + responseText: { + "id": 2, + "slug": "test-a", + "name": "TestA", + "description": "TestA", + "vocabulary_type": "m", + "required": false, + "weight": 1, + "terms": [] + } + }); React.addons.TestUtils.Simulate.submit(formNode); waitForAjax(1, function() { assert.equal( From aa1718a55492951e9eca7faf2b146549cd9c898f Mon Sep 17 00:00:00 2001 From: George Schneeloch Date: Wed, 15 Jul 2015 17:35:23 -0400 Subject: [PATCH 16/38] Added shopping cart for export --- apiary.apib | 100 +++++-- learningresources/models.py | 2 +- learningresources/tests/base.py | 2 +- learningresources/tests/test_models.py | 1 - lore/celery.py | 2 +- lore/wsgi.py | 2 +- pylintrc | 2 +- rest/permissions.py | 13 + rest/serializers.py | 10 +- rest/tests/base.py | 139 ++++++++- rest/tests/test_learning_resource.py | 286 +++++++++++++++++-- rest/tests/test_members.py | 1 - rest/tests/test_repository.py | 2 - rest/tests/test_vocabulary.py | 2 - rest/urls.py | 11 + rest/views.py | 142 ++++++++- roles/tests/test_api.py | 2 +- roles/tests/test_permissions.py | 2 +- taxonomy/tests/test_api.py | 1 - ui/static/ui/css/slide-drawer.css | 72 ++--- ui/static/ui/js/listing.js | 95 +++++- ui/static/ui/js/lr_exports.jsx | 82 ++++++ ui/static/ui/js/require_config.js | 1 + ui/static/ui/js/utils.jsx | 2 +- ui/templates/includes/exports_panel.html | 15 + ui/templates/includes/learning_resource.html | 12 + ui/templates/repository.html | 8 + ui/views.py | 4 +- 28 files changed, 886 insertions(+), 127 deletions(-) create mode 100644 ui/static/ui/js/lr_exports.jsx create mode 100644 ui/templates/includes/exports_panel.html diff --git a/apiary.apib b/apiary.apib index e73175d4..aa4cf571 100644 --- a/apiary.apib +++ b/apiary.apib @@ -1,5 +1,5 @@ FORMAT: 1A -HOST: http://lore.odl.mit.edu/ +HOST: http://lore.odl.mit.edu/api/v1/ # LORE @@ -134,15 +134,16 @@ Create a repository by providing its JSON representation. "create_date": "2015-06-22" } -## Group Learningresources +## Group LearningResources -## Learningresource Collection [/repositories/{repo_slug}/learning_resources/{?type_name}] +## LearningResource Collection [/repositories/{repo_slug}/learning_resources/{?type_name}] + Parameters + repo_slug: `physics-1` (string, required) - slug for the repository + type_name: `chapter` (string, optional) - type of learning resource + + ids: `1,9` (string, optional) - comma separated list of LearningResource ids. Will filter out LearningResources not in this list. -### List Learningresources [GET] +### List LearningResources [GET] + Response 200 (application/json) @@ -175,13 +176,13 @@ Create a repository by providing its JSON representation. ] } -## Learningresource [/repositories/{repo_slug}/learning_resources/{learningresource_id}/] +## LearningResource [/repositories/{repo_slug}/learning_resources/{learningresource_id}/] + Parameters + repo_slug: `physics-1` (string, required) - slug for the repository - + learningresource_id: `8` (number, required) - ID of the Learningresource + + learningresource_id: `8` (number, required) - ID of the LearningResource -### View a Learningresource [GET] +### View a LearningResource [GET] + Response 200 (application/json) @@ -204,7 +205,7 @@ Create a repository by providing its JSON representation. "preview_url": "https://www.example.com/courses..." } -### Partially Update a Learningresource [PATCH] +### Partially Update a LearningResource [PATCH] + Request (application/json) @@ -233,7 +234,7 @@ Create a repository by providing its JSON representation. "preview_url": "https://www.example.com/courses..." } -### Update a Learningresource [PUT] +### Update a LearningResource [PUT] + Request (application/json) @@ -263,15 +264,80 @@ Create a repository by providing its JSON representation. "preview_url": "https://www.example.com/courses..." } -## Group Staticassets +## Group LearningResourceExports -## Staticassets Collection [/repositories/{repo_slug}/learning_resources/{learningresource_id}/static_assets/] +List of LearningResources to be exported. This is stored per user and is deleted on logout. + +## LearningResourceExport Collection [/repositories/{repo_slug}/learning_resource_exports/{username}/] + ++ Parameters + + repo_slug: `physics-1` (string, required) - slug for the repository + + username: `sarah` (string, required) - owner of exports (must be logged in user) + +### List LearningResourceExports [GET] + ++ Response 200 (application/json) + + + Body + + { + "count": 1, + "next": null, + "previous": null, + "results": [ + { + "id": 13 + } + ] + } + +### Delete LearningResourceExports [DELETE] + +Clears the export list for this repository. + ++ Response 204 (application/json) + +### Add a LearningResource ID to Export list [POST] + ++ Request (application/json) + + { + "id": 15 + } + ++ Response 201 (application/json) + + { + "id": 15 + } + +## LearningResourceExport [/repositories/{repo_slug}/learning_resource_exports/{username}/] + ++ Parameters + + repo_slug: `physics-1` (string, required) - slug for the repository + + username: `sarah` (string, required) - owner of exports (must be logged in user) + +### View a LearningResourceExport [GET] + ++ Response 200 (application/json) + + { + "id": 13 + } + +### Delete an ID from the LearningResource export list [DELETE] + ++ Response 204 (application/json) + +## Group StaticAssets + +## StaticAssets Collection [/repositories/{repo_slug}/learning_resources/{learningresource_id}/static_assets/] + Parameters + repo_slug: `physics-1` (string, required) - slug for the repository - + learningresource_id: `1` (number, required) - ID of the Learningresource + + learningresource_id: `1` (number, required) - ID of the LearningResource -### List All Staticassets [GET] +### List All StaticAssets [GET] + Response 200 (application/json) @@ -287,14 +353,14 @@ Create a repository by providing its JSON representation. ] } -## Staticasset [/repositories/{repo_slug}/learning_resources/{learningresource_id}/static_assets/{static_asset_id}/] +## StaticAsset [/repositories/{repo_slug}/learning_resources/{learningresource_id}/static_assets/{static_asset_id}/] + Parameters + repo_slug: `physics-1` (string, required) - slug for the repository - + learningresource_id: `1` (number, required) - ID of the Learningresource - + static_asset_id: `1` (number, required) - ID of the Staticasset + + learningresource_id: `1` (number, required) - ID of the LearningResource + + static_asset_id: `1` (number, required) - ID of the StaticAsset -### View a Staticasset [GET] +### View a StaticAsset [GET] + Response 200 (application/json) diff --git a/learningresources/models.py b/learningresources/models.py index 3ceb096f..5dad5f04 100644 --- a/learningresources/models.py +++ b/learningresources/models.py @@ -84,7 +84,7 @@ class Course(BaseModel): imported_by = models.ForeignKey(User) class Meta: - # pylint: disable=invalid-name,missing-docstring,too-few-public-methods + # pylint: disable=missing-docstring,too-few-public-methods unique_together = ("repository", "org", "course_number", "run") diff --git a/learningresources/tests/base.py b/learningresources/tests/base.py index 7ef0eabe..2e84d58b 100644 --- a/learningresources/tests/base.py +++ b/learningresources/tests/base.py @@ -30,7 +30,7 @@ from roles.api import assign_user_to_repo_group from roles.permissions import GroupTypes -log = logging.getLogger(__name__) # pylint: disable=invalid-name +log = logging.getLogger(__name__) class LoreTestCase(TestCase): diff --git a/learningresources/tests/test_models.py b/learningresources/tests/test_models.py index 56497799..d8a59585 100644 --- a/learningresources/tests/test_models.py +++ b/learningresources/tests/test_models.py @@ -75,7 +75,6 @@ def __init__(self, course): class TestModels(LoreTestCase): """Tests for learningresources models""" - # pylint: disable=invalid-name def test_unicode(self): """Test for __unicode__ on LearningResourceType""" diff --git a/lore/celery.py b/lore/celery.py index 8adb954b..87e3aeff 100644 --- a/lore/celery.py +++ b/lore/celery.py @@ -15,7 +15,7 @@ log = logging.getLogger(__name__) -async = Celery('lore') # pylint: disable=invalid-name +async = Celery('lore') # Using a string here means the worker will not have to # pickle the object when using Windows. diff --git a/lore/wsgi.py b/lore/wsgi.py index de3c6ea0..d3ce15e9 100644 --- a/lore/wsgi.py +++ b/lore/wsgi.py @@ -16,4 +16,4 @@ os.environ.setdefault("DJANGO_SETTINGS_MODULE", "lore.settings") -application = Cling(get_wsgi_application()) # pylint: disable=invalid-name +application = Cling(get_wsgi_application()) diff --git a/pylintrc b/pylintrc index 0e8e20de..c9a3144f 100644 --- a/pylintrc +++ b/pylintrc @@ -13,4 +13,4 @@ ignored-modules= six.moves, [MESSAGES CONTROL] -disable = no-member, old-style-class, no-init, too-few-public-methods, abstract-method +disable = no-member, old-style-class, no-init, too-few-public-methods, abstract-method, invalid-name, too-many-ancestors diff --git a/rest/permissions.py b/rest/permissions.py index b9569588..7d054888 100644 --- a/rest/permissions.py +++ b/rest/permissions.py @@ -200,3 +200,16 @@ def has_permission(self, request, view): raise Http404() return True + + +class ViewLearningResourceExportPermission(BasePermission): + """ + Checks that username mentioned is the same one as the request. + In the future we may want to let users see each other's exports + but for now this just enforces the user seeing their own exports. + """ + + def has_permission(self, request, view): + username = view.kwargs['username'] + + return request.user.username == username diff --git a/rest/serializers.py b/rest/serializers.py index affb3d45..df1c85c0 100644 --- a/rest/serializers.py +++ b/rest/serializers.py @@ -19,6 +19,7 @@ SlugRelatedField, FileField, SerializerMethodField, + IntegerField, ) from rest.util import LambdaDefault, RequiredBooleanField @@ -222,12 +223,12 @@ def validate_terms(self, terms): name=resource_type.name).exists(): raise ValidationError( "Term {} is not supported " - "for this learning resource".format(term.label)) + "for this LearningResource".format(term.label)) return terms @staticmethod def get_preview_url(obj): - """Construct preview URL for learning resource""" + """Construct preview URL for LearningResource.""" return obj.get_preview_url() @@ -260,3 +261,8 @@ def get_name(static_asset_obj): run=static_asset_obj.course.run, ) return static_asset_obj.asset.name.replace(basepath, '') + + +class LearningResourceExportSerializer(Serializer): + """Serializer for exporting id for LearningResource.""" + id = IntegerField() diff --git a/rest/tests/base.py b/rest/tests/base.py index 1eec5a77..06acd1fa 100644 --- a/rest/tests/base.py +++ b/rest/tests/base.py @@ -13,6 +13,7 @@ from rest_framework.reverse import reverse from django.db.models import Count from django.contrib.auth.models import User, Permission +from django.shortcuts import get_object_or_404 from importer.tasks import import_file from learningresources.tests.base import LoreTestCase @@ -479,7 +480,7 @@ def delete_member(self, urlfor, repo_slug, self.assertEqual(resp.status_code, expected_status) def get_learning_resources(self, repo_slug, expected_status=HTTP_200_OK): - """Get learning resources""" + """Get LearningResources.""" url = '{repo_base}{repo_slug}/learning_resources/'.format( repo_base=REPO_BASE, repo_slug=repo_slug, @@ -492,7 +493,7 @@ def get_learning_resources(self, repo_slug, expected_status=HTTP_200_OK): def get_learning_resource(self, repo_slug, learning_resource_id, expected_status=HTTP_200_OK): - """Get a learning resource""" + """Get a LearningResource.""" url = '{repo_base}{repo_slug}/learning_resources/{lr_id}/'.format( repo_base=REPO_BASE, repo_slug=repo_slug, @@ -507,7 +508,7 @@ def get_learning_resource(self, repo_slug, learning_resource_id, def patch_learning_resource( self, repo_slug, learning_resource_id, lr_dict, expected_status=HTTP_200_OK, skip_assert=False): - """Update a learning resource""" + """Update a LearningResource.""" resp = self.client.patch( '{repo_base}{repo_slug}/' 'learning_resources/{lr_id}/'.format( @@ -529,7 +530,7 @@ def patch_learning_resource( def put_learning_resource( self, repo_slug, learning_resource_id, lr_dict, expected_status=HTTP_200_OK, skip_assert=False): - """Update a learning resource""" + """Update a LearningResource.""" resp = self.client.put( '{repo_base}{repo_slug}/' 'learning_resources/{lr_id}/'.format( @@ -550,7 +551,7 @@ def put_learning_resource( def get_static_assets(self, repo_slug, learning_resource_id, expected_status=HTTP_200_OK): - """Get static assets for learning resource""" + """Get static assets for LearningResource.""" url = ( '{repo_base}{repo_slug}/' 'learning_resources/{lr_id}/static_assets/'.format( @@ -568,7 +569,7 @@ def get_static_assets(self, repo_slug, learning_resource_id, def get_static_asset(self, repo_slug, learning_resource_id, static_asset_id, expected_status=HTTP_200_OK): - """Get a static asset""" + """Get a StaticAsset.""" url = ( '{repo_base}{repo_slug}/learning_resources/{lr_id}/' 'static_assets/{sa_id}/'.format( @@ -587,7 +588,7 @@ def get_static_asset(self, repo_slug, learning_resource_id, def import_course_tarball(self, repo): """ Import course.xml into repo and return first LearningResource - which has any StaticAssets + which has any StaticAssets. """ tarball_file = self.get_course_single_tarball() import_file( @@ -596,6 +597,130 @@ def import_course_tarball(self, repo): count_assets=Count('static_assets') ).filter(count_assets__gt=0).first() + def get_learning_resource_exports(self, repo_slug, username=None, + expected_status=HTTP_200_OK): + """ + Get ids for LearningResource exports. + """ + if username is None: + user_id = self.client.session['_auth_user_id'] + username = get_object_or_404(User, id=user_id).username + + url = ( + "{repo_base}{repo_slug}/" + "learning_resource_exports/{username}/".format( + repo_base=REPO_BASE, + repo_slug=repo_slug, + username=username, + ) + ) + self.assert_options_head(url, expected_status=expected_status) + resp = self.client.get(url) + self.assertEqual(expected_status, resp.status_code) + if expected_status == HTTP_200_OK: + return as_json(resp) + + def delete_learning_resource_exports(self, repo_slug, username=None, + expected_status=HTTP_204_NO_CONTENT): + """ + Delete all exports in shopping cart for this repo. + """ + if username is None: + user_id = self.client.session['_auth_user_id'] + username = get_object_or_404(User, id=user_id).username + + url = ( + "{repo_base}{repo_slug}/" + "learning_resource_exports/{username}/".format( + repo_base=REPO_BASE, + repo_slug=repo_slug, + username=username, + ) + ) + resp = self.client.delete(url) + self.assertEqual(expected_status, resp.status_code) + + def get_learning_resource_export(self, repo_slug, learning_resource_id, + username=None, + expected_status=HTTP_200_OK): + """ + Get id for LearningResource export. + """ + if username is None: + user_id = self.client.session['_auth_user_id'] + username = get_object_or_404(User, id=user_id).username + + url = ( + "{repo_base}{repo_slug}/" + "learning_resource_exports/{username}/{lr_id}/".format( + repo_base=REPO_BASE, + repo_slug=repo_slug, + username=username, + lr_id=learning_resource_id, + ) + ) + resp = self.client.get(url) + self.assertEqual(expected_status, resp.status_code) + if expected_status == HTTP_200_OK: + return as_json(resp) + + def delete_learning_resource_export(self, repo_slug, learning_resource_id, + username=None, + expected_status=HTTP_204_NO_CONTENT): + """ + Delete one export in shopping cart for this repo. + """ + if username is None: + user_id = self.client.session['_auth_user_id'] + username = get_object_or_404(User, id=user_id).username + + url = ( + "{repo_base}{repo_slug}/" + "learning_resource_exports/{username}/{lr_id}/".format( + repo_base=REPO_BASE, + repo_slug=repo_slug, + username=username, + lr_id=learning_resource_id, + ) + ) + resp = self.client.delete(url) + self.assertEqual(expected_status, resp.status_code) + + def create_learning_resource_export(self, repo_slug, input_dict, + username=None, + expected_status=HTTP_201_CREATED, + skip_assert=False): + """ + Add a LearningResource to shopping cart to be exported. + """ + if username is None: + user_id = self.client.session['_auth_user_id'] + username = get_object_or_404(User, id=user_id).username + + url = ( + "{repo_base}{repo_slug}/" + "learning_resource_exports/{username}/".format( + repo_base=REPO_BASE, + repo_slug=repo_slug, + username=username, + ) + ) + resp = self.client.post(url, input_dict) + self.assertEqual(expected_status, resp.status_code) + if resp.status_code == HTTP_201_CREATED: + result_dict = as_json(resp) + if not skip_assert: + for key, value in input_dict.items(): + self.assertEqual(value, result_dict[key]) + self.assertIn(reverse( + 'learning-resource-export-detail', kwargs={ + 'repo_slug': repo_slug, + 'username': username, + 'lr_id': result_dict['id'], + } + ), resp['Location']) + return result_dict + class RESTAuthTestCase(RESTTestCase): """REST tests for authorization""" diff --git a/rest/tests/test_learning_resource.py b/rest/tests/test_learning_resource.py index ef5e6142..4a470a4d 100644 --- a/rest/tests/test_learning_resource.py +++ b/rest/tests/test_learning_resource.py @@ -1,5 +1,5 @@ """ -REST tests for learning resources and static assets +REST tests for LearningResources and static assets """ from __future__ import unicode_literals @@ -24,15 +24,17 @@ from learningresources.models import ( LearningResource, LearningResourceType, + Repository, ) from importer.tasks import import_file from taxonomy.models import Vocabulary +from roles.permissions import GroupTypes +from roles.api import assign_user_to_repo_group -# pylint: disable=invalid-name class TestLearningResource(RESTTestCase): """ - REST tests for learning resources and static assets + REST tests for LearningResources and StaticAssets """ def test_immutable_fields_learning_resource(self): @@ -81,7 +83,7 @@ def assert_not_changed(new_dict): skip_assert=True)) def test_missing_learning_resource(self): - """Test for an invalid learning resource id""" + """Test for an invalid LearningResource id.""" repo_slug1 = self.repo.slug resource1 = self.import_course_tarball(self.repo) lr1_id = resource1.id @@ -104,8 +106,37 @@ def test_missing_learning_resource(self): expected_status=HTTP_404_NOT_FOUND) self.get_learning_resource(repo_slug2, lr2_id) + def test_learning_resource_filter(self): + """Test for filtering LearningResources by ids.""" + + def get_filtered(ids, expected_status=HTTP_200_OK): + """Return list of LearningResources in shopping cart.""" + url_base = "{repo_base}{repo_slug}/learning_resources/".format( + repo_base=REPO_BASE, + repo_slug=self.repo.slug, + ) + resp = self.client.get("{url_base}?id={ids}".format( + url_base=url_base, + ids=",".join([str(s) for s in ids]) + )) + self.assertEqual(expected_status, resp.status_code) + if expected_status == HTTP_200_OK: + return sorted([x['id'] for x in as_json(resp)['results']]) + + self.import_course_tarball(self.repo) + self.assertEqual([], get_filtered([])) + get_filtered(["not-a-number"], + expected_status=HTTP_400_BAD_REQUEST) + self.assertEqual([], get_filtered([-1])) + + all_ids = list(get_resources( + self.repo.id).values_list('id', flat=True)) + self.assertEqual(sorted(all_ids), get_filtered(all_ids)) + self.assertEqual( + sorted(all_ids[:5]), get_filtered(all_ids[:5])) + def test_filefield_serialization(self): - """Make sure that URL output is turned on in settings""" + """Make sure that URL output is turned on in settings.""" resource = self.import_course_tarball(self.repo) static_assets = self.get_static_assets( self.repo.slug, resource.id)['results'] @@ -113,7 +144,7 @@ def test_filefield_serialization(self): def test_add_term_to_learning_resource(self): """ - Add a term to a learning resource via PATCH + Add a term to a LearningResource via PATCH. """ resource = self.import_course_tarball(self.repo) @@ -149,7 +180,7 @@ def test_add_term_to_learning_resource(self): def test_learning_resource_types(self): """ - Get from learning_resource_types + Get from learning_resource_types. """ base_url = "{}learning_resource_types/".format(API_BASE) @@ -190,7 +221,7 @@ def test_learning_resource_types(self): def test_preview_url(self): """ - Assert preview url behavior for learning resources + Assert preview url behavior for LearningResources. """ learning_resource = LearningResource.objects.first() expected_jump_to_id_url = ( @@ -214,15 +245,88 @@ def test_preview_url(self): learning_resource.get_preview_url() ) + def test_learning_resource_exports_invalid_methods(self): + """ + Test invalid methods for session-based shopping cart + for LearningResource exports. + """ + + collection_url = ( + "{repo_base}{repo_slug}/learning_resource_exports/{user}/".format( + repo_base=REPO_BASE, + repo_slug=self.repo.slug, + user=self.user.username + ) + ) + lr_id = LearningResource.objects.first().id + detail_url = ( + "{repo_base}{repo_slug}/learning_resource_exports/" + "{user}/{lr_id}/".format( + repo_base=REPO_BASE, + repo_slug=self.repo.slug, + user=self.user.username, + lr_id=lr_id, + ) + ) + + # PUT and PATCH not supported + resp = self.client.patch(collection_url, {}) + self.assertEqual(HTTP_405_METHOD_NOT_ALLOWED, resp.status_code) + resp = self.client.put(collection_url, {}) + self.assertEqual(HTTP_405_METHOD_NOT_ALLOWED, resp.status_code) + resp = self.client.patch(detail_url, {}) + self.assertEqual(HTTP_405_METHOD_NOT_ALLOWED, resp.status_code) + resp = self.client.put(detail_url, {}) + self.assertEqual(HTTP_405_METHOD_NOT_ALLOWED, resp.status_code) + + # POST not supported on detail url + resp = self.client.post(detail_url, {}) + self.assertEqual(HTTP_405_METHOD_NOT_ALLOWED, resp.status_code) + + def test_learning_resource_exports_anonymous(self): + """Make sure anonymous users are forbidden.""" + lr_id = get_resources(self.repo.id).first().id + self.logout() + + self.get_learning_resource_export( + self.repo.slug, lr_id, username="sarah", + expected_status=HTTP_403_FORBIDDEN) + self.get_learning_resource_exports( + self.repo.slug, username="sarah", + expected_status=HTTP_403_FORBIDDEN) + self.create_learning_resource_export( + self.repo.slug, {"id": lr_id}, username="sarah", + expected_status=HTTP_403_FORBIDDEN) + self.delete_learning_resource_export( + self.repo.slug, lr_id, + username="sarah", + expected_status=HTTP_403_FORBIDDEN) + self.delete_learning_resource_exports( + self.repo.slug, username="sarah", + expected_status=HTTP_403_FORBIDDEN) + + def test_learning_resource_exports_not_a_number(self): + """ + Don't cause 500 error for non-numeric ids. + """ + lr_id = "notanumber" + + self.get_learning_resource_export( + self.repo.slug, lr_id, expected_status=HTTP_404_NOT_FOUND) + self.create_learning_resource_export( + self.repo.slug, {"id": lr_id}, + expected_status=HTTP_400_BAD_REQUEST) + self.delete_learning_resource_export( + self.repo.slug, lr_id, expected_status=HTTP_404_NOT_FOUND) + -# pylint: disable=too-many-ancestors class TestLearningResourceAuthorization(RESTAuthTestCase): """ - REST tests for authorization of learning resources and static assets + REST tests for authorization of LearningResources and static assets. """ def test_resource_get(self): - """Test retrieve learning resource""" + """Test retrieve LearningResource.""" self.assertEqual( 1, self.get_learning_resources(self.repo.slug)['count']) @@ -254,7 +358,7 @@ def test_resource_get(self): self.repo.slug, lr_id, expected_status=HTTP_403_FORBIDDEN) def test_resource_post(self): - """Test create learning resource""" + """Test create LearningResource.""" resp = self.client.post( '{repo_base}{repo_slug}/learning_resources/'.format( repo_base=REPO_BASE, @@ -265,7 +369,7 @@ def test_resource_post(self): self.assertEqual(resp.status_code, HTTP_405_METHOD_NOT_ALLOWED) def test_resource_delete(self): - """Test delete learning resource""" + """Test delete LearningResource.""" resource = self.import_course_tarball(self.repo) lr_id = resource.id @@ -280,7 +384,7 @@ def test_resource_delete(self): self.assertEqual(resp.status_code, HTTP_405_METHOD_NOT_ALLOWED) def test_resource_put_patch(self): - """Test update learning resource""" + """Test update LearningResource.""" resource = self.import_course_tarball(self.repo) lr_id = resource.id @@ -323,7 +427,7 @@ def test_resource_put_patch(self): ) def test_static_assets_get(self): - """Test for getting static assets from learning_resources""" + """Test for getting StaticAssets from LearningResources.""" def get_resource_with_asset(type_name): """ Get a LearningResource with a StaticAsset. @@ -344,7 +448,7 @@ def get_resource_with_asset(type_name): static_asset2 = resource2.static_assets.first() lr1_id = resource1.id - # add a second StaticAsset to another learning resource + # add a second StaticAsset to another LearningResource resource2.static_assets.add(static_asset2) lr2_id = resource2.id # make sure the result for an asset contains a name and an url @@ -392,7 +496,7 @@ def get_resource_with_asset(type_name): expected_status=HTTP_403_FORBIDDEN) def test_static_assets_create(self): - """Test for creating static assets from learning_resources""" + """Test for creating StaticAsset from learning_resources.""" lr_id = get_resources(self.repo.id).first().id resp = self.client.post( '{repo_base}{repo_slug}/learning_resources/' @@ -406,7 +510,7 @@ def test_static_assets_create(self): self.assertEqual(resp.status_code, HTTP_405_METHOD_NOT_ALLOWED) def test_static_assets_put_patch(self): - """Test for updating static assets from learning_resources""" + """Test for updating StaticAssets from learning_resources.""" resource = self.import_course_tarball(self.repo) static_asset = resource.static_assets.first() lr_id = resource.id @@ -435,7 +539,7 @@ def test_static_assets_put_patch(self): self.assertEqual(resp.status_code, HTTP_405_METHOD_NOT_ALLOWED) def test_static_assets_delete(self): - """Test for deleting static assets from learning_resources""" + """Test for deleting StaticAssets from learning_resources.""" resource = self.import_course_tarball(self.repo) static_asset = resource.static_assets.first() lr_id = resource.id @@ -451,3 +555,147 @@ def test_static_assets_delete(self): self.DEFAULT_LR_DICT ) self.assertEqual(resp.status_code, HTTP_405_METHOD_NOT_ALLOWED) + + def test_learning_resource_exports(self): + """Test for creating LearningResource ids in session shopping cart.""" + + # set up our data + self.logout() + self.login(self.add_repo_user) + + repo_slug1 = self.repo.slug + repo_slug2 = self.create_repository()['slug'] + repo2 = Repository.objects.get(slug=repo_slug2) + + # 'add_repo_user' will be able to see both repos + assign_user_to_repo_group( + self.add_repo_user, self.repo, GroupTypes.REPO_ADMINISTRATOR) + self.import_course_tarball(repo2) + + repo1_lrid1 = get_resources(self.repo.id).all()[0].id + repo2_lrid1 = get_resources(repo2.id).all()[0].id + repo2_lrid2 = get_resources(repo2.id).all()[1].id + + self.logout() + self.login(self.user) + + # make sure we start with an empty shopping cart + self.assertEqual([], + self.get_learning_resource_exports( + repo_slug1)['results']) + self.create_learning_resource_export(self.repo.slug, { + 'id': repo1_lrid1 + }) + self.assertEqual([{'id': repo1_lrid1}], + self.get_learning_resource_exports( + repo_slug1)['results']) + + # user doesn't have access to repo2 + self.get_learning_resource_exports( + repo_slug2, expected_status=HTTP_403_FORBIDDEN) + + self.logout() + self.login(self.add_repo_user) + + # make sure that session is not shared between users + # user has an id in repo1 but add_repo_user does not + self.assertEqual([], self.get_learning_resource_exports( + repo_slug1 + )['results']) + + # don't store duplicates + self.create_learning_resource_export(repo_slug1, {"id": repo1_lrid1}) + self.create_learning_resource_export(repo_slug1, {"id": repo1_lrid1}) + self.assertEqual( + [{"id": repo1_lrid1}], + self.get_learning_resource_exports( + repo_slug1 + )['results']) + + # make sure export ids are confined to their own repos + self.create_learning_resource_export(repo_slug2, { + 'id': repo2_lrid1 + }) + # not affected by POST in previous line + self.assertEqual([{'id': repo1_lrid1}], + self.get_learning_resource_exports( + repo_slug1)['results']) + # contains the new item + self.assertEqual([{'id': repo2_lrid1}], + self.get_learning_resource_exports( + repo_slug2)['results']) + + # make sure we can't add export ids that don't belong to the repo + self.create_learning_resource_export(repo_slug1, { + 'id': repo2_lrid1 + }, expected_status=HTTP_403_FORBIDDEN) + + # make sure detail view will 404 if out of bounds + self.assertEqual(repo1_lrid1, self.get_learning_resource_export( + repo_slug1, repo1_lrid1)['id']) + self.get_learning_resource_export( + repo_slug1, repo2_lrid1, + expected_status=HTTP_404_NOT_FOUND) + self.get_learning_resource_export( + repo_slug2, repo1_lrid1, + expected_status=HTTP_404_NOT_FOUND) + self.assertEqual(repo2_lrid1, self.get_learning_resource_export( + repo_slug2, repo2_lrid1)['id']) + + self.create_learning_resource_export(repo_slug2, {"id": repo2_lrid1}) + self.create_learning_resource_export(repo_slug2, {"id": repo2_lrid2}) + + # finally, delete all of the things + self.assertEqual( + 2, self.get_learning_resource_exports(repo_slug2)['count']) + self.delete_learning_resource_exports(repo_slug2) + self.assertEqual( + 0, self.get_learning_resource_exports(repo_slug2)['count']) + + # make sure deleting an empty list doesn't cause problems + self.delete_learning_resource_exports(repo_slug2) + self.assertEqual( + 0, self.get_learning_resource_exports(repo_slug2)['count']) + + # repo1 is unaffected + self.assertEqual( + 1, self.get_learning_resource_exports(repo_slug1)['count']) + self.delete_learning_resource_export(repo_slug1, repo1_lrid1) + self.get_learning_resource_export( + repo_slug1, repo1_lrid1, expected_status=HTTP_404_NOT_FOUND) + + self.logout() + self.login(self.user) + + # Populate shopping cart one more time. + self.create_learning_resource_export(repo_slug1, {"id": repo1_lrid1}) + + self.logout() + self.login(self.user) + + # After logout and login session was cleared + self.assertEqual( + 0, self.get_learning_resource_exports(repo_slug1)['count']) + + def test_export_with_incorrect_username(self): + """Make sure we enforce the username in the URL.""" + resp = self.client.get( + "{repo_base}{repo_slug}/learning_resource_exports/missing/".format( + repo_base=REPO_BASE, + repo_slug=self.repo.slug, + )) + self.assertEqual(HTTP_403_FORBIDDEN, resp.status_code) + + resource = self.repo.course_set.first().learningresource_set.first() + self.create_learning_resource_export(self.repo.slug, { + "id": resource.id + }) + resp = self.client.get( + "{repo_base}{repo_slug}/learning_resource_exports/missing/" + "{lr_id}/".format( + repo_base=REPO_BASE, + repo_slug=self.repo.slug, + lr_id=resource.id, + ) + ) + self.assertEqual(HTTP_403_FORBIDDEN, resp.status_code) diff --git a/rest/tests/test_members.py b/rest/tests/test_members.py index a81967f0..8a421f34 100644 --- a/rest/tests/test_members.py +++ b/rest/tests/test_members.py @@ -585,7 +585,6 @@ def test_members_delete(self): self.assert_users_count(admins=1) -# pylint: disable=too-many-ancestors class TestMembersAuthorization(RESTAuthTestCase): """Tests for member authorization via REST""" diff --git a/rest/tests/test_repository.py b/rest/tests/test_repository.py index 1a179e31..b3b400d5 100644 --- a/rest/tests/test_repository.py +++ b/rest/tests/test_repository.py @@ -22,7 +22,6 @@ from learningresources.models import Repository -# pylint: disable=invalid-name class TestRepository(RESTTestCase): """Repository REST tests""" @@ -165,7 +164,6 @@ def assert_not_changed(new_dict): assert_not_changed(self.create_repository(repo_dict, skip_assert=True)) -# pylint: disable=too-many-ancestors class TestRepositoryAuthorization(RESTAuthTestCase): """Repository authorization REST tests""" diff --git a/rest/tests/test_vocabulary.py b/rest/tests/test_vocabulary.py index 006cdc70..7fb962cb 100644 --- a/rest/tests/test_vocabulary.py +++ b/rest/tests/test_vocabulary.py @@ -26,7 +26,6 @@ from learningresources.models import LearningResourceType -# pylint: disable=invalid-name class TestVocabulary(RESTTestCase): """ REST tests relating to vocabularies and terms @@ -601,7 +600,6 @@ def test_vocabulary_learning_resource_types(self): self.repo.slug, vocab_slug)['learning_resource_types']) -# pylint: disable=too-many-ancestors class TestVocabularyAuthorization(RESTAuthTestCase): """ Tests relating to term and vocabulary authorization diff --git a/rest/urls.py b/rest/urls.py index 4ca64c8b..fe3f21f6 100644 --- a/rest/urls.py +++ b/rest/urls.py @@ -23,6 +23,8 @@ LearningResourceTypeList, StaticAssetList, StaticAssetDetail, + LearningResourceExportList, + LearningResourceExportDetail, ) REPOSITORY_MEMBERS_URL = r'^repositories/(?P[-\w]+)/members/' @@ -30,6 +32,9 @@ REPOSITORY_RESOURCE_URL = ( r'^repositories/(?P[-\w]+)/learning_resources/' ) +REPOSITORY_EXPORTS_URL = ( + r'^repositories/(?P[-\w]+)/learning_resource_exports/' +) urlpatterns = [ url(r'^api-auth/', include('rest_framework.urls', @@ -96,6 +101,12 @@ r'(?P\d+)/static_assets/(?P\d+)/$', StaticAssetDetail.as_view(), name='static-asset-detail'), + url(REPOSITORY_EXPORTS_URL + r'(?P[-\w]+)/$', + LearningResourceExportList.as_view(), + name='learning-resource-export-list'), + url(REPOSITORY_EXPORTS_URL + r'(?P[-\w]+)/(?P\d+)/$', + LearningResourceExportDetail.as_view(), + name='learning-resource-export-detail'), url("^learning_resource_types/$", LearningResourceTypeList.as_view(), name='learning-resource-type-list'), ] diff --git a/rest/views.py b/rest/views.py index 35d91407..bdc31866 100644 --- a/rest/views.py +++ b/rest/views.py @@ -6,7 +6,9 @@ from django.http.response import Http404 from django.contrib.auth.models import User +from django.core.exceptions import PermissionDenied from rest_framework import status +from rest_framework.exceptions import ValidationError from rest_framework.generics import ( ListAPIView, ListCreateAPIView, @@ -35,6 +37,7 @@ GroupSerializer, LearningResourceTypeSerializer, LearningResourceSerializer, + LearningResourceExportSerializer, StaticAssetSerializer, ) from rest.permissions import ( @@ -46,6 +49,7 @@ ManageTaxonomyPermission, ManageRepoMembersPermission, ViewLearningResourcePermission, + ViewLearningResourceExportPermission, ViewStaticAssetPermission, ) from rest.util import CheckValidMemberParamMixin @@ -58,10 +62,12 @@ ) from learningresources.api import ( get_repos, + get_resource, ) +EXPORTS_KEY = 'learning_resource_exports' + -# pylint: disable=too-many-ancestors class RepositoryList(ListCreateAPIView): """REST list view for Repository""" serializer_class = RepositorySerializer @@ -118,7 +124,7 @@ class VocabularyList(ListCreateAPIView): def get_queryset(self): """Filter vocabularies by repository ownership and optionally - by learning resource type""" + by LearningResource type""" queryset = Vocabulary.objects.filter( repository__slug=self.kwargs['repo_slug'] ) @@ -410,10 +416,18 @@ class LearningResourceList(ListAPIView): ) def get_queryset(self): - """Get queryset for a learning resource""" - return LearningResource.objects.filter( + """Get queryset for a LearningResource""" + queryset = LearningResource.objects.filter( course__repository__slug=self.kwargs['repo_slug'] ) + id_value = self.request.query_params.get('id', None) + if id_value is not None: + try: + id_list = [int(x) for x in id_value.split(',') if len(x) > 0] + except ValueError: + raise ValidationError("id is not a number") + queryset = queryset.filter(id__in=id_list) + return queryset class LearningResourceDetail(RetrieveUpdateAPIView): @@ -428,7 +442,7 @@ class LearningResourceDetail(RetrieveUpdateAPIView): ) def get_queryset(self): - """Get queryset for a learning resource""" + """Get queryset for a LearningResource""" return LearningResource.objects.filter( id=self.kwargs['lr_id']) @@ -444,7 +458,7 @@ class StaticAssetList(ListAPIView): lookup_url_kwarg = 'sa_id' def get_queryset(self): - """Get queryset for static assets for a particular learning resource""" + """Get queryset for static assets for a particular LearningResource""" return LearningResource.objects.get( id=self.kwargs['lr_id'] ).static_assets.filter() @@ -461,7 +475,121 @@ class StaticAssetDetail(RetrieveAPIView): lookup_url_kwarg = 'sa_id' def get_queryset(self): - """Get queryset for static assets for a particular learning resource""" + """Get queryset for static assets for a particular LearningResource""" return StaticAsset.objects.filter( id=self.kwargs['sa_id'] ) + + +class LearningResourceExportList(ListCreateAPIView): + """REST list view for exports""" + + serializer_class = LearningResourceExportSerializer + permission_classes = ( + ViewLearningResourceExportPermission, + ViewRepoPermission, + IsAuthenticated, + ) + + def get_queryset(self): + repo_slug = self.kwargs['repo_slug'] + try: + exports = self.request.session[ + EXPORTS_KEY][repo_slug] + except KeyError: + exports = [] + return [{"id": lr_id} for lr_id in exports] + + def create(self, request, *args, **kwargs): + try: + lr_id = int(request.data['id']) + except ValueError: + raise ValidationError("LearningResource id must be a number") + repo_slug = self.kwargs['repo_slug'] + + learning_resource = get_resource(lr_id, self.request.user.id) + if learning_resource.course.repository.slug != repo_slug: + raise PermissionDenied() + + if EXPORTS_KEY not in self.request.session: + self.request.session[EXPORTS_KEY] = {} + + if repo_slug not in self.request.session[EXPORTS_KEY]: + self.request.session[EXPORTS_KEY][repo_slug] = [] + + exports = self.request.session[EXPORTS_KEY][repo_slug] + if lr_id not in exports: + exports.append(lr_id) + self.request.session.modified = True + + url = reverse('learning-resource-export-detail', + kwargs={'repo_slug': self.kwargs['repo_slug'], + 'username': self.kwargs['username'], + 'lr_id': lr_id}) + headers = {"Location": url} + return Response( + {"id": lr_id}, status=status.HTTP_201_CREATED, headers=headers) + + # pylint: disable=unused-argument + def delete(self, request, *args, **kwargs): + """Delete all ids in session for this repository and user""" + try: + del self.request.session[EXPORTS_KEY][self.kwargs['repo_slug']] + self.request.session.modified = True + except KeyError: + # doesn't exist, no need to delete + pass + return Response( + status=status.HTTP_204_NO_CONTENT, + content_type="text" + ) + + +class LearningResourceExportDetail(RetrieveDestroyAPIView): + """ + Detail resource for a LearningResource id for export + """ + serializer_class = LearningResourceExportSerializer + permission_classes = ( + ViewLearningResourceExportPermission, + ViewLearningResourcePermission, + IsAuthenticated, + ) + lookup_field = 'id' + lookup_url_kwarg = 'lr_id' + + def get_object(self): + """ + Retrieve an export id from the list. + """ + try: + lr_id = int(self.kwargs['lr_id']) + except ValueError: + raise Http404 + + repo_slug = self.kwargs['repo_slug'] + try: + exports = self.request.session[ + EXPORTS_KEY][repo_slug] + except KeyError: + raise Http404 + + if lr_id not in exports: + raise Http404 + + return {"id": lr_id} + + def perform_destroy(self, instance): + """ + Delete an export id from our export list. + """ + lr_id = instance['id'] + repo_slug = self.kwargs['repo_slug'] + try: + exports = self.request.session[ + EXPORTS_KEY][repo_slug] + if lr_id in exports: + exports.remove(lr_id) + self.request.session.modified = True + except KeyError: + raise Http404 diff --git a/roles/tests/test_api.py b/roles/tests/test_api.py index 07362c97..7f1f6289 100644 --- a/roles/tests/test_api.py +++ b/roles/tests/test_api.py @@ -25,7 +25,7 @@ class TestRoleApi(LoreTestCase): the repository model where it is called """ # there are too many self assignment in the init: disabling the check - # pylint: disable=too-many-instance-attributes, invalid-name + # pylint: disable=too-many-instance-attributes # pylint: disable=protected-access def __init__(self, *args, **kwargs): super(TestRoleApi, self).__init__(*args, **kwargs) diff --git a/roles/tests/test_permissions.py b/roles/tests/test_permissions.py index 1d1d715e..b133d394 100644 --- a/roles/tests/test_permissions.py +++ b/roles/tests/test_permissions.py @@ -18,7 +18,7 @@ class TestRolePermission(TestCase): """ Tests for the permissions """ - # pylint: disable=invalid-name, protected-access + # pylint: disable=protected-access def test_base_group_types(self): """ Checks basic repo group types diff --git a/taxonomy/tests/test_api.py b/taxonomy/tests/test_api.py index 86b7c143..d644f415 100644 --- a/taxonomy/tests/test_api.py +++ b/taxonomy/tests/test_api.py @@ -4,7 +4,6 @@ from __future__ import unicode_literals # pylint: disable=too-many-instance-attributes -# pylint: disable=invalid-name from learningresources.tests.base import LoreTestCase from learningresources.api import ( diff --git a/ui/static/ui/css/slide-drawer.css b/ui/static/ui/css/slide-drawer.css index f9b77e9b..9dc70f83 100644 --- a/ui/static/ui/css/slide-drawer.css +++ b/ui/static/ui/css/slide-drawer.css @@ -43,7 +43,9 @@ html, body { } .cd-panel, -.cd-panel-members { +.cd-panel-2, +.cd-panel-members, +.cd-panel-exports { position: fixed; top: 0; left: 0; @@ -56,7 +58,9 @@ html, body { z-index: 1000; } .cd-panel::after, -.cd-panel-members::after { +.cd-panel-2::after, +.cd-panel-members::after, +.cd-panel-exports::after { /* overlay layer */ position: absolute; top: 0; @@ -70,27 +74,35 @@ html, body { transition: background 0.3s 0.3s; } .cd-panel.is-visible, -.cd-panel-members.is-visible { +.cd-panel-2.is-visible, +.cd-panel-members.is-visible, +.cd-panel-exports.is-visible { visibility: visible; -webkit-transition: visibility 0s 0s; -moz-transition: visibility 0s 0s; transition: visibility 0s 0s; } .cd-panel.is-visible::after, -.cd-panel-members.is-visible::after { +.cd-panel-2.is-visible::after, +.cd-panel-members.is-visible::after, +.cd-panel-exports.is-visible::after { background: rgba(0, 0, 0, 0.6); -webkit-transition: background 0.3s 0s; -moz-transition: background 0.3s 0s; transition: background 0.3s 0s; } .cd-panel.is-visible .cd-panel-close::before, -.cd-panel-members.is-visible .cd-panel-close::before { +.cd-panel-2.is-visible .cd-panel-close::before, +.cd-panel-members.is-visible .cd-panel-close::before, +.cd-panel-exports.is-visible .cd-panel-close::before { -webkit-animation: cd-close-1 0.6s 0.3s; -moz-animation: cd-close-1 0.6s 0.3s; animation: cd-close-1 0.6s 0.3s; } .cd-panel.is-visible .cd-panel-close::after, -.cd-panel-members.is-visible .cd-panel-close::after { +.cd-panel-2.is-visible .cd-panel-close::after, +.cd-panel-members.is-visible .cd-panel-close::after, +.cd-panel-exports.is-visible .cd-panel-close::after { -webkit-animation: cd-close-2 0.6s 0.3s; -moz-animation: cd-close-2 0.6s 0.3s; animation: cd-close-2 0.6s 0.3s; @@ -100,54 +112,6 @@ html, body { margin-bottom: 15px; } -.cd-panel-2 { - position: fixed; - top: 0; - left: 0; - height: 100%; - width: 100%; - visibility: hidden; - -webkit-transition: visibility 0s 0.6s; - -moz-transition: visibility 0s 0.6s; - transition: visibility 0s 0.6s; - z-index: 1000; -} -.cd-panel-2::after { - /* overlay layer */ - position: absolute; - top: 0; - left: 0; - width: 100%; - height: 100%; - background: transparent; - cursor: pointer; - -webkit-transition: background 0.3s 0.3s; - -moz-transition: background 0.3s 0.3s; - transition: background 0.3s 0.3s; -} -.cd-panel-2.is-visible { - visibility: visible; - -webkit-transition: visibility 0s 0s; - -moz-transition: visibility 0s 0s; - transition: visibility 0s 0s; -} -.cd-panel-2.is-visible::after { - background: rgba(0, 0, 0, 0.6); - -webkit-transition: background 0.3s 0s; - -moz-transition: background 0.3s 0s; - transition: background 0.3s 0s; -} -.cd-panel-2.is-visible .cd-panel-close::before { - -webkit-animation: cd-close-1 0.6s 0.3s; - -moz-animation: cd-close-1 0.6s 0.3s; - animation: cd-close-1 0.6s 0.3s; -} -.cd-panel-2.is-visible .cd-panel-close::after { - -webkit-animation: cd-close-2 0.6s 0.3s; - -moz-animation: cd-close-2 0.6s 0.3s; - animation: cd-close-2 0.6s 0.3s; -} - @-webkit-keyframes cd-close-1 { 0%, 50% { -webkit-transform: rotate(0); diff --git a/ui/static/ui/js/listing.js b/ui/static/ui/js/listing.js index b37c072e..cffa161b 100644 --- a/ui/static/ui/js/listing.js +++ b/ui/static/ui/js/listing.js @@ -1,9 +1,9 @@ -define( - ['jquery', 'setup_manage_taxonomies', 'facets', +define('listing', + ['jquery', 'lodash', 'setup_manage_taxonomies', 'facets', 'learning_resources', 'static_assets', 'utils', - 'bootstrap', 'icheck', 'csrf'], - function($, setupManageTaxonomies, facets, LearningResources, StaticAssets, - Utils) { + 'lr_exports', 'bootstrap', 'icheck', 'csrf'], + function($, _, setupManageTaxonomies, facets, LearningResources, StaticAssets, + Utils, Exports) { 'use strict'; facets.setupFacets(window); var EMAIL_EXTENSION = '@mit.edu'; @@ -114,6 +114,24 @@ define( } }); + // Open exports panel. + var loggedInUsername = $("#export_button").data("logged-in-user"); + + $('#export_button').on('click', function(event) { + event.preventDefault(); + $('.cd-panel-exports').addClass('is-visible'); + Exports.loader(repoSlug, loggedInUsername, $("#exports_content")[0]); + }); + + // Close exports panel. + $('.cd-panel-exports').on('click', function(event) { + if ($(event.target).is('.cd-panel-exports') || + $(event.target).is('.cd-panel-close')) { + $('.cd-panel-exports').removeClass('is-visible'); + event.preventDefault(); + } + }); + //open the lateral panel for members $('.btn-members').on('click', function(event) { event.preventDefault(); @@ -213,6 +231,73 @@ define( }); }); + var updateCheckDisplay = function(node) { + if ($(node).data("selected")) { + $(node).find("i").attr("class", "fa fa-check"); + } else { + $(node).find("i").attr("class", "fa fa-arrow-right"); + } + }; + + /** + * Update export count badge. + * + * @param {number} direction Add this value to + * the existing count before rendering. + */ + var updateExportCount = function(direction) { + var oldCount = $("#export_button").data("export-count"); + var newCount = oldCount + direction; + + $("#export_button .badge").remove(); + if (newCount > 0) { + var $badge = $(""); + $badge.append(_.escape(newCount)); + $("#export_button").append($badge); + } + $("#export_button").data("export-count", newCount); + }; + _.each($(".link-export"), function(node) { + updateCheckDisplay(node); + }); + updateExportCount(0); + + // Set up click handlers for export links. + $(".link-export").click(function(event) { + event.preventDefault(); + + var learningResourceId = $(this).data("learningresource-id"); + var selected = $(this).data("selected") === true; + $(this).data("selected", !selected); + + if (selected) { + // Remove an item from export cart. + $.ajax({ + type: "DELETE", + url: "/api/v1/repositories/" + repoSlug + + "/learning_resource_exports/" + loggedInUsername + "/" + + learningResourceId + "/" + }).fail(function() { + // Revert export count change. + updateExportCount(1); + }); + updateExportCount(-1); + } else { + // Add an item to export cart. + $.post("/api/v1/repositories/" + repoSlug + + "/learning_resource_exports/" + loggedInUsername + "/", + { + id: learningResourceId + }).fail(function () { + // Revert export count change. + updateExportCount(-1); + }); + updateExportCount(1); + } + + updateCheckDisplay(this); + }); + var repoSlug = $("#repo_slug").val(); setupManageTaxonomies.loader(repoSlug); }); diff --git a/ui/static/ui/js/lr_exports.jsx b/ui/static/ui/js/lr_exports.jsx new file mode 100644 index 00000000..a6856ba9 --- /dev/null +++ b/ui/static/ui/js/lr_exports.jsx @@ -0,0 +1,82 @@ +define("lr_exports", + ['reactaddons', 'jquery', 'lodash', 'utils'], function (React, $, _, Utils) { + 'use strict'; + + /** + * A React component which shows the list of exports + * and provides a way to export these learning resources in bulk. + */ + var ExportsComponent = React.createClass({ + componentDidMount: function() { + var thiz = this; + Utils.getCollection("/api/v1/repositories/" + thiz.props.repoSlug + + "/learning_resource_exports/" + thiz.props.loggedInUser + "/" + ).then(function (ids) { + ids = _.map(ids, function (obj) { + return obj.id; + }); + if (ids.length > 0) { + return Utils.getCollection("/api/v1/repositories/" + + thiz.props.repoSlug + + "/learning_resources/?id=" + encodeURIComponent(ids.join(",")) + ); + } else { + return []; + } + }) + .then(function (learningResources) { + thiz.setState({exports: learningResources}); + }) + .fail(function() { + thiz.setState({ + messageText: undefined, + errorText: "Unable to load exports" + }); + }); + }, + render: function() { + var errorBox = null; + if (this.state.errorText !== undefined) { + errorBox =
+ {this.state.errorText} +
; + } + var messageBox = null; + if (this.state.messageText !== undefined) { + messageBox =
+ {this.state.messageText} +
; + } + + var exports = _.map(this.state.exports, function(ex) { + return
  • {ex.title}
  • ; + }); + + return
    + {errorBox} + {messageBox} +
      + {exports} +
    +
    ; + }, + getInitialState: function() { + return { + errorText: undefined, + messageText: undefined, + exports: [] + }; + } + }); + + return { + ExportsComponent: ExportsComponent, + loader: function(repoSlug, loggedInUser, container) { + React.unmountComponentAtNode(container); + React.render( + , + container + ); + } + }; +}); diff --git a/ui/static/ui/js/require_config.js b/ui/static/ui/js/require_config.js index 24c91467..3edf5ace 100644 --- a/ui/static/ui/js/require_config.js +++ b/ui/static/ui/js/require_config.js @@ -14,6 +14,7 @@ var REQUIRE_PATHS = { setup_manage_taxonomies: "../ui/js/manage_taxonomies.jsx?noext", learning_resources: "../ui/js/learning_resources.jsx?noext", static_assets: "../ui/js/static_assets.jsx?noext", + lr_exports: "../ui/js/lr_exports.jsx?noext", utils: "../ui/js/utils.jsx?noext", }; var SHIMS = { diff --git a/ui/static/ui/js/utils.jsx b/ui/static/ui/js/utils.jsx index 0ca04a93..7c80c8d1 100644 --- a/ui/static/ui/js/utils.jsx +++ b/ui/static/ui/js/utils.jsx @@ -34,7 +34,7 @@ define("utils", ["jquery", "lodash"], function ($, _) { getVocabulariesAndTerms: function (repoSlug, learningResourceType) { var url = '/api/v1/repositories/' + repoSlug + '/vocabularies/'; if (learningResourceType) { - url += "?type_name=" + encodeURI(learningResourceType); + url += "?type_name=" + encodeURIComponent(learningResourceType); } return _getCollection(url).then(function(vocabs) { diff --git a/ui/templates/includes/exports_panel.html b/ui/templates/includes/exports_panel.html new file mode 100644 index 00000000..2789d13a --- /dev/null +++ b/ui/templates/includes/exports_panel.html @@ -0,0 +1,15 @@ + +
    +
    +

    Exports

    + Close +
    +
    +
    +
    +
    + +
    + +
    diff --git a/ui/templates/includes/learning_resource.html b/ui/templates/includes/learning_resource.html index decb7a38..32cd1481 100644 --- a/ui/templates/includes/learning_resource.html +++ b/ui/templates/includes/learning_resource.html @@ -53,4 +53,16 @@

    + diff --git a/ui/templates/repository.html b/ui/templates/repository.html index 671800c4..1bf69d01 100644 --- a/ui/templates/repository.html +++ b/ui/templates/repository.html @@ -48,6 +48,10 @@ {% endif %}
    +
    +
    + +
    {% for result in page.object_list %} {% include "includes/learning_resource.html" %} @@ -77,6 +81,7 @@ {% if 'manage_repo_users' in perms_on_cur_repo %} {% include "includes/members_panel.html" %} {% endif %} +{% include "includes/exports_panel.html" %}