2u/add view tests to optimizer

cms/djangoapps/contentstore/docs/how-tos/test_course_related_view_auth.rst

@@ -0,0 +1,60 @@
+==============================================
+How to test View Auth for course-related views
+==============================================
+
+What to test
+------------
+Each view endpoint that exposes an internal API endpoint - like in files in the rest_api folder - must
+be tested for the following.
+
+- Only authenticated users can access the endpoint.
+- Only users with the correct permissions (authorization) can access the endpoint.
+- All data and params that are part of the request are properly validated.
+
+How to test
+-----------
+A lot of these tests can be easily implemented by inheriting from the `AuthorizeStaffTestCase`.
+This parent class assumes that the view is for a specific course and that only users who have access
+to the course can access the view. (They are either staff or instructors for the course, or global admin).
+
+Here is an example of how to test a view that requires a user to be authenticated and have access to a course.
+
+.. code-block:: python
+
+    from cms.djangoapps.contentstore.tests.test_utils import AuthorizeStaffTestCase
+    from django.test import TestCase
+    from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
+    from django.urls import reverse
+
+    class TestMyGetView(AuthorizeStaffTestCase, ModuleStoreTestCase, TestCase):
+        def make_request(self, course_id=None, data=None):
+            url = self.get_url(self.course.id)
+            response = self.client.get(url, data)
+            return response
+
+        def get_url(self, course_key):
+            url = reverse(
+                'cms.djangoapps.contentstore:v0:my_get_view',
+                kwargs={'course_id': self.course.id}
+            )
+            return url
+
+As you can see, you need to inherit from `AuthorizeStaffTestCase` and `ModuleStoreTestCase`, and then either
+`TestCase` or `APITestCase` depending on the type of view you are testing. For cookie-based
+authentication, `TestCase` is sufficient, for Oauth2 use `ApiTestCase`.
+
+The only two methods you need to implement are `make_request` and `get_url`. The `make_request` method
+should make the request to the view and return the response. The `get_url` method should return the URL
+for the view you are testing.
+
+Overwriting Tests
+-----------------
+If you need different behavior you can overwrite the tests from the parent class.
+For example, if students should have access to the view, simply implement the
+`test_student` method in your test class.
+
+Adding other tests
+------------------
+If you want to test other things in the view - let's say validation -
+it's easy to just add another `test_...` function to your test class
+and you can use the `make_request` method to make the request.

cms/djangoapps/contentstore/rest_api/v0/tests/test_course_optimizer.py

@@ -1,22 +1,74 @@
-from cms.djangoapps.contentstore.tests.test_utils import AuthorizeStaffTestCase
-from rest_framework import status
 from django.test import TestCase
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from django.urls import reverse
 
-class TestCourseOptimizer(AuthorizeStaffTestCase, ModuleStoreTestCase, TestCase):
+from cms.djangoapps.contentstore.tests.test_utils import AuthorizeStaffTestCase
+
+class TestGetLinkCheckStatus(AuthorizeStaffTestCase, ModuleStoreTestCase, TestCase):
     '''
-    Tests for CourseOptimizer
+    Authentication and Authorization Tests for CourseOptimizer.
+    For concrete tests that are run, check `AuthorizeStaffTestCase`.
     '''
-    def test_inherited(self):
-        # This method ensures that pytest recognizes this class as containing tests
-        pass
-
-    def make_request(self, course_id=None, data=None):
-        return self.client.get(self.get_url(course_id), data)
+    def make_request(self, course_id=None, data=None, **kwargs):
+        url = self.get_url(self.course.id)
+        response = self.client.get(url, data)
+        return response
 
     def get_url(self, course_key):
-        return reverse(
+        url = reverse(
             'cms.djangoapps.contentstore:v0:link_check_status',
-            kwargs={'course_id': 'course-v1:someOrg+someCourse+someRun'}
+            kwargs={'course_id': self.course.id}
         )
+        return url
+
+    def test_produces_4xx_when_invalid_course_id(self):
+        '''
+        Test course_id validation
+        '''
+        response = self.make_request(course_id='invalid_course_id')
+        self.assertIn(response.status_code, range(400, 500))
+
+    def test_produces_4xx_when_additional_kwargs(self):
+        '''
+        Test additional kwargs validation
+        '''
+        response = self.make_request(course_id=self.course.id, malicious_kwarg='malicious_kwarg')
+        self.assertIn(response.status_code, range(400, 500))
+
+class TestPostLinkCheck(AuthorizeStaffTestCase, ModuleStoreTestCase, TestCase):
+    '''
+    Authentication and Authorization Tests for CourseOptimizer.
+    For concrete tests that are run, check `AuthorizeStaffTestCase`.
+    '''
+    def make_request(self, course_id=None, data=None, **kwargs):
+        url = self.get_url(self.course.id)
+        response = self.client.post(url, data)
+        return response
+
+    def get_url(self, course_key):
+        url = reverse(
+            'cms.djangoapps.contentstore:v0:link_check',
+            kwargs={'course_id': self.course.id}
+        )
+        return url
+
+    def test_produces_4xx_when_invalid_course_id(self):
+        '''
+        Test course_id validation
+        '''
+        response = self.make_request(course_id='invalid_course_id')
+        self.assertIn(response.status_code, range(400, 500))
+
+    def test_produces_4xx_when_additional_kwargs(self):
+        '''
+        Test additional kwargs validation
+        '''
+        response = self.make_request(course_id=self.course.id, malicious_kwarg='malicious_kwarg')
+        self.assertIn(response.status_code, range(400, 500))
+
+    def test_produces_4xx_when_unexpected_data(self):
+        '''
+        Test validation when request contains unexpected data
+        '''
+        response = self.make_request(course_id=self.course.id, data={'unexpected_data': 'unexpected_data'})
+        self.assertIn(response.status_code, range(400, 500))

cms/djangoapps/contentstore/rest_api/v0/views/course_optimizer.py

@@ -150,6 +150,7 @@ def get(self, request: Request, course_id: str):
         """
         course_key = CourseKey.from_string(course_id)
         if not has_course_author_access(request.user, course_key):
+            print('missing course author access')
             self.permission_denied(request)
 
         task_status = _latest_task_status(request, course_id)
@@ -189,9 +190,7 @@ def get(self, request: Request, course_id: str):
             **({'LinkCheckOutput': broken_links_dto} if broken_links_dto else {}),
             **({'LinkCheckError': error} if error else {})
         }
-
-        serializer = LinkCheckSerializer(data=data)
-        serializer.is_valid(raise_exception=True)
+        serializer = LinkCheckSerializer(data)
 
         return Response(serializer.data)
 

cms/djangoapps/contentstore/tests/test_tasks.py

@@ -268,7 +268,6 @@ def test_check_broken_links_stores_broken_and_locked_urls(
         ### Check that _save_broken_links_file was called with the correct arguments
         mock_save_broken_links_file.assert_called_once_with(mock_user_task_artifact.return_value, mock.ANY)
 
-
     def test_user_does_not_exist_raises_exception(self):
         raise NotImplementedError
 

openedx/core/lib/api/view_utils.py

@@ -125,6 +125,7 @@ def _decorator(func_or_class):
             SessionAuthenticationAllowInactiveUser
         )
         func_or_class.permission_classes = ()
+        print('is_authenticated', is_authenticated)
         if is_authenticated:
             func_or_class.permission_classes += (IsAuthenticated,)
         if is_user: