Fix part of issue #19435: Migrate the Collection Player page to Angul…

…ar Router (#19901) * Registered the COLLECTION_PLAYER page * Removed backend handler and updated the associated tests. * Removed URL to handler linkage in main.py * Added access validator * Deleted Redundant File collection-player-page.mainpage.html * Created Route Guard * Component Upgraded * Created Root Component Files * Updated Main Module * Updated Components in Module * Added module to App Routing * Updated Webpack Configuration * Added required spec files * Fixed a error * Fixed lint and frontend karma errors * Update authors since 7f6c6b6 (#19878) * Update core/templates/pages/about-page/about-page.constants.ts * Update AUTHORS * Update CONTRIBUTORS * Update contributors * Fixed lint and frontend karma errors * Fixed lint, frontend errors and backend errors. * resolved minor issues * Added access validator * Created Route Guard * resolved minor issues * Added access validator * Created Route Guard * Fixed lint, frontend errors and backend errors. * resolved minor issues * format code with prettier * format code with prettier * format code with prettier * format code with prettier --------- Co-authored-by: amaanlari <amaanlari@outlook.comm> Co-authored-by: Sean Lip <sean@seanlip.org>
oppia · Mar 21, 2024 · b2358cc · b2358cc
1 parent 3ea5a1c
commit b2358cc
Show file tree

Hide file tree

Showing 24 changed files with 562 additions and 334 deletions.
diff --git a/assets/constants.ts b/assets/constants.ts
@@ -6602,6 +6602,15 @@ export default {
         }
       ]
     },
+    "COLLECTION_PLAYER": {
+      "ROUTE": "collection/:collection_id",
+      "TITLE": "",
+      // Some routes contain url fragments, as syntax for url fragments are
+      // different for angular router and backend. They have to be registered
+      // manually in the backend. Please use angular router syntax here.
+      "MANUALLY_REGISTERED_WITH_BACKEND": true,
+      "META": []
+    },
     "EMAIL_DASHBOARD": {
       "ROUTE": "emaildashboard",
       "TITLE": "Email Dashboard - Oppia",

diff --git a/core/controllers/access_validators.py b/core/controllers/access_validators.py
@@ -78,6 +78,28 @@ def get(self) -> None:
             raise self.PageNotFoundException
 
 
+class CollectionViewerPageAccessValidationHandler(
+    base.BaseHandler[Dict[str, str], Dict[str, str]]
+):
+    """Validates access to collection page."""
+
+    GET_HANDLER_ERROR_RETURN_TYPE = feconf.HANDLER_TYPE_JSON
+
+    URL_PATH_ARGS_SCHEMAS = {
+       'collection_id': {
+           'schema': {
+               'type': 'basestring'
+           }
+       }
+    }
+    HANDLER_ARGS_SCHEMAS: Dict[str, Dict[str, str]] = {'GET': {}}
+
+    @acl_decorators.can_play_collection
+    def get(self, _: str) -> None:
+        """Handles GET requests."""
+        pass
+
+
 class ManageOwnAccountValidationHandler(
     base.BaseHandler[Dict[str, str], Dict[str, str]]
 ):

diff --git a/core/controllers/access_validators_test.py b/core/controllers/access_validators_test.py
@@ -24,6 +24,8 @@
 from core.domain import classroom_config_services
 from core.domain import learner_group_fetchers
 from core.domain import learner_group_services
+from core.domain import rights_manager
+from core.domain import user_services
 from core.platform import models
 from core.storage.blog import gae_models as blog_models
 from core.tests import test_utils
@@ -79,6 +81,98 @@ def test_validation_returns_false_if_classroom_doesnot_exists(self) -> None:
             expected_status_int=404)
 
 
+class CollectionViewerPageAccessValidationHandlerTests(
+        test_utils.GenericTestBase):
+    """Test for collection page access validation."""
+
+    COLLECTION_ID: Final = 'cid'
+    OTHER_EDITOR_EMAIL: Final = 'another@example.com'
+
+    def setUp(self) -> None:
+        """Before each individual test, create a dummy collection."""
+        super().setUp()
+        self.signup(self.EDITOR_EMAIL, self.EDITOR_USERNAME)
+        self.editor_id = self.get_user_id_from_email(self.EDITOR_EMAIL)
+        self.editor = user_services.get_user_actions_info(self.editor_id)
+        self.signup(self.NEW_USER_EMAIL, self.NEW_USER_USERNAME)
+        self.new_user_id = self.get_user_id_from_email(self.NEW_USER_EMAIL)
+        self.save_new_valid_collection(self.COLLECTION_ID, self.editor_id)
+
+    def test_unpublished_collections_are_invisible_to_logged_out_users(
+        self
+    ) -> None:
+        self.get_json(
+            '%s/can_access_collection_player_page/%s' %
+            (ACCESS_VALIDATION_HANDLER_PREFIX, self.COLLECTION_ID),
+            expected_status_int=404)
+
+    def test_unpublished_collections_are_invisible_to_unconnected_users(
+        self
+    ) -> None:
+        self.login(self.NEW_USER_EMAIL)
+        self.get_json(
+        '%s/can_access_collection_player_page/%s' %
+            (ACCESS_VALIDATION_HANDLER_PREFIX, self.COLLECTION_ID),
+        expected_status_int=404)
+        self.logout()
+
+    def test_unpublished_collections_are_invisible_to_other_editors(
+        self
+    ) -> None:
+        self.signup(self.OTHER_EDITOR_EMAIL, 'othereditorusername')
+        self.save_new_valid_collection('cid2', self.OTHER_EDITOR_EMAIL)
+        self.login(self.OTHER_EDITOR_EMAIL)
+        self.get_json(
+        '%s/can_access_collection_player_page/%s' %
+            (ACCESS_VALIDATION_HANDLER_PREFIX, self.COLLECTION_ID),
+        expected_status_int=404)
+        self.logout()
+
+    def test_unpublished_collections_are_visible_to_their_editors(
+        self
+    ) -> None:
+        self.login(self.EDITOR_EMAIL)
+        self.get_html_response(
+            '%s/can_access_collection_player_page/%s' %
+            (ACCESS_VALIDATION_HANDLER_PREFIX, self.COLLECTION_ID))
+        self.logout()
+
+    def test_unpublished_collections_are_visible_to_admins(self) -> None:
+        self.signup(self.MODERATOR_EMAIL, self.MODERATOR_USERNAME)
+        self.set_moderators([self.MODERATOR_USERNAME])
+        self.login(self.MODERATOR_EMAIL)
+        self.get_html_response(
+            '%s/can_access_collection_player_page/%s' %
+            (ACCESS_VALIDATION_HANDLER_PREFIX, self.COLLECTION_ID))
+        self.logout()
+
+    def test_published_collections_are_visible_to_logged_out_users(
+        self
+    ) -> None:
+        rights_manager.publish_collection(self.editor, self.COLLECTION_ID)
+
+        self.get_html_response(
+            '%s/can_access_collection_player_page/%s' %
+            (ACCESS_VALIDATION_HANDLER_PREFIX, self.COLLECTION_ID))
+
+    def test_published_collections_are_visible_to_logged_in_users(
+        self
+    ) -> None:
+        rights_manager.publish_collection(self.editor, self.COLLECTION_ID)
+        self.login(self.NEW_USER_EMAIL)
+        self.get_html_response(
+            '%s/can_access_collection_player_page/%s' %
+            (ACCESS_VALIDATION_HANDLER_PREFIX, self.COLLECTION_ID))
+
+    def test_invalid_collection_error(self) -> None:
+        self.login(self.EDITOR_EMAIL)
+        self.get_json(
+            '%s/can_access_collection_player_page/%s' %
+            (ACCESS_VALIDATION_HANDLER_PREFIX, 'none'),
+            expected_status_int=404)
+        self.logout()
+
+
 class ReleaseCoordinatorAccessValidationHandlerTests(
         test_utils.GenericTestBase):
     """Test for release coordinator access validation."""

diff --git a/core/controllers/collection_viewer.py b/core/controllers/collection_viewer.py
@@ -26,25 +26,6 @@
 from typing import Dict
 
 
-class CollectionPage(base.BaseHandler[Dict[str, str], Dict[str, str]]):
-    """Page describing a single collection."""
-
-    URL_PATH_ARGS_SCHEMAS = {
-        'collection_id': {
-            'schema': {
-                'type': 'basestring'
-            }
-        }
-    }
-    HANDLER_ARGS_SCHEMAS: Dict[str, Dict[str, str]] = {'GET': {}}
-
-    @acl_decorators.can_play_collection
-    def get(self, _: str) -> None:
-        """Handles GET requests."""
-
-        self.render_template('collection-player-page.mainpage.html')
-
-
 class CollectionDataHandler(base.BaseHandler[Dict[str, str], Dict[str, str]]):
     """Provides the data for a single collection."""
 

diff --git a/core/controllers/collection_viewer_test.py b/core/controllers/collection_viewer_test.py
@@ -18,101 +18,8 @@
 
 from core import feconf
 from core.domain import collection_services
-from core.domain import rights_manager
-from core.domain import user_services
 from core.tests import test_utils
 
-from typing import Final
-
-
-class CollectionViewerPermissionsTests(test_utils.GenericTestBase):
-    """Test permissions for learners to view collections."""
-
-    COLLECTION_ID: Final = 'cid'
-    OTHER_EDITOR_EMAIL: Final = 'another@example.com'
-
-    def setUp(self) -> None:
-        """Before each individual test, create a dummy collection."""
-        super().setUp()
-
-        self.signup(self.EDITOR_EMAIL, self.EDITOR_USERNAME)
-        self.editor_id = self.get_user_id_from_email(self.EDITOR_EMAIL)
-        self.editor = user_services.get_user_actions_info(self.editor_id)
-
-        self.signup(self.NEW_USER_EMAIL, self.NEW_USER_USERNAME)
-        self.new_user_id = self.get_user_id_from_email(self.NEW_USER_EMAIL)
-
-        self.save_new_valid_collection(self.COLLECTION_ID, self.editor_id)
-
-    def test_unpublished_collections_are_invisible_to_logged_out_users(
-        self
-    ) -> None:
-        self.get_html_response(
-            '%s/%s' % (feconf.COLLECTION_URL_PREFIX, self.COLLECTION_ID),
-            expected_status_int=404)
-
-    def test_unpublished_collections_are_invisible_to_unconnected_users(
-        self
-    ) -> None:
-        self.login(self.NEW_USER_EMAIL)
-        self.get_html_response(
-            '%s/%s' % (feconf.COLLECTION_URL_PREFIX, self.COLLECTION_ID),
-            expected_status_int=404)
-        self.logout()
-
-    def test_unpublished_collections_are_invisible_to_other_editors(
-        self
-    ) -> None:
-        self.signup(self.OTHER_EDITOR_EMAIL, 'othereditorusername')
-
-        self.save_new_valid_collection('cid2', self.OTHER_EDITOR_EMAIL)
-
-        self.login(self.OTHER_EDITOR_EMAIL)
-        self.get_html_response(
-            '%s/%s' % (feconf.COLLECTION_URL_PREFIX, self.COLLECTION_ID),
-            expected_status_int=404)
-        self.logout()
-
-    def test_unpublished_collections_are_visible_to_their_editors(
-        self
-    ) -> None:
-        self.login(self.EDITOR_EMAIL)
-        self.get_html_response(
-            '%s/%s' % (feconf.COLLECTION_URL_PREFIX, self.COLLECTION_ID))
-        self.logout()
-
-    def test_unpublished_collections_are_visible_to_admins(self) -> None:
-        self.signup(self.MODERATOR_EMAIL, self.MODERATOR_USERNAME)
-        self.set_moderators([self.MODERATOR_USERNAME])
-        self.login(self.MODERATOR_EMAIL)
-        self.get_html_response(
-            '%s/%s' % (feconf.COLLECTION_URL_PREFIX, self.COLLECTION_ID))
-        self.logout()
-
-    def test_published_collections_are_visible_to_logged_out_users(
-        self
-    ) -> None:
-        rights_manager.publish_collection(self.editor, self.COLLECTION_ID)
-
-        self.get_html_response(
-            '%s/%s' % (feconf.COLLECTION_URL_PREFIX, self.COLLECTION_ID))
-
-    def test_published_collections_are_visible_to_logged_in_users(
-        self
-    ) -> None:
-        rights_manager.publish_collection(self.editor, self.COLLECTION_ID)
-
-        self.login(self.NEW_USER_EMAIL)
-        self.get_html_response(
-            '%s/%s' % (feconf.COLLECTION_URL_PREFIX, self.COLLECTION_ID))
-
-    def test_invalid_collection_error(self) -> None:
-        self.login(self.EDITOR_EMAIL)
-        self.get_html_response(
-            '%s/%s' % (feconf.COLLECTION_URL_PREFIX, 'none'),
-            expected_status_int=404)
-        self.logout()
-
 
 class CollectionViewerControllerEndToEndTests(test_utils.GenericTestBase):
     """Test the collection viewer controller using a sample collection."""

diff --git a/core/templates/pages/collection-player-page/collection-footer/collection-footer.component.ts b/core/templates/pages/collection-player-page/collection-footer/collection-footer.component.ts
@@ -18,7 +18,6 @@
  */
 
 import {Component, OnInit} from '@angular/core';
-import {downgradeComponent} from '@angular/upgrade/static';
 
 import {UrlInterpolationService} from 'domain/utilities/url-interpolation.service';
 import {UrlService} from 'services/contextual/url.service';
@@ -45,10 +44,3 @@ export class CollectionFooterComponent implements OnInit {
     return this.urlInterpolationService.getStaticImageUrl(imagePath);
   }
 }
-
-angular
-  .module('oppia')
-  .directive(
-    'collectionFooter',
-    downgradeComponent({component: CollectionFooterComponent})
-  );
diff --git a/...lates/pages/collection-player-page/collection-local-nav/collection-local-nav.component.ts b/...lates/pages/collection-player-page/collection-local-nav/collection-local-nav.component.ts
@@ -17,7 +17,6 @@
  */
 
 import {Component, OnInit, OnDestroy} from '@angular/core';
-import {downgradeComponent} from '@angular/upgrade/static';
 
 import {Subscription} from 'rxjs';
 
@@ -58,9 +57,3 @@ export class CollectionLocalNavComponent implements OnInit, OnDestroy {
     this.directiveSubscriptions.unsubscribe();
   }
 }
-angular
-  .module('oppia')
-  .directive(
-    'collectionLocalNav',
-    downgradeComponent({component: CollectionLocalNavComponent})
-  );
diff --git a/core/templates/pages/collection-player-page/collection-navbar/collection-navbar.component.ts b/core/templates/pages/collection-player-page/collection-navbar/collection-navbar.component.ts
@@ -17,7 +17,6 @@
  */
 
 import {Component, OnDestroy, OnInit} from '@angular/core';
-import {downgradeComponent} from '@angular/upgrade/static';
 
 import {Subscription} from 'rxjs';
 
@@ -59,9 +58,3 @@ export class CollectionNavbarComponent implements OnInit, OnDestroy {
     return this.directiveSubscriptions.unsubscribe();
   }
 }
-angular
-  .module('oppia')
-  .directive(
-    'collectionNavbar',
-    downgradeComponent({component: CollectionNavbarComponent})
-  );
diff --git a/...lates/pages/collection-player-page/collection-node-list/collection-node-list.component.ts b/...lates/pages/collection-player-page/collection-node-list/collection-node-list.component.ts
@@ -17,7 +17,6 @@
  * playing the exploration in each node.
  */
 import {Component, Input} from '@angular/core';
-import {downgradeComponent} from '@angular/upgrade/static';
 
 import {CollectionNode} from 'domain/collection/collection-node.model';
 
@@ -34,10 +33,3 @@ export class CollectionNodeListComponent {
   @Input() collectionNodes!: CollectionNode[];
   constructor() {}
 }
-
-angular
-  .module('oppia')
-  .directive(
-    'collectionNodeList',
-    downgradeComponent({component: CollectionNodeListComponent})
-  );