Skip to content

Commit

Permalink
Fix part of oppia#19435: Learner group editor page migration (oppia#1…
Browse files Browse the repository at this point in the history 
…9761)

* Fix part of oppia#19435: Migrate learner group editor page

* complete migration

* remove browser module

* complete migration

* fix linter

* fix linter

* added frontend test

* add test

* fix linter

* fix test

* fix linter

* fix linter

* fix root

* fix linter

* update comment

* removed use of context service and fixed typo

* fix test

* fix test

* fix test

* fix test

* fix test

* remove max-len flag

* use paramMap instead of windowRef

* avoid using hard coded string

* add script for passive event
  • Loading branch information
@AFZL210
AFZL210 authored Feb 22, 2024
1 parent 2d1af0f commit e979bb1
Show file tree
Hide file tree
Showing 22 changed files with 472 additions and 245 deletions.
18 changes: 18 additions & 0 deletions assets/constants.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7214,6 +7214,24 @@ export default {
}
]
},
"LEARNER_GROUP_EDITOR": {
"ROUTE": "edit-learner-group/:learner_group_id",
"TITLE": "Edit Learner Group | Oppia",
"META": [
{
"PROPERTY_TYPE": "itemprop",
"PROPERTY_VALUE": "description",
// eslint-disable-next-line max-len
"CONTENT": "With Oppia, you can access free lessons on math, physics, statistics, chemistry, music, history and more from anywhere in the world. Oppia is a nonprofit with the mission of providing high-quality education to those who lack access to it."
},
{
"PROPERTY_TYPE": "itemprop",
"PROPERTY_VALUE": "og:description",
// eslint-disable-next-line max-len
"CONTENT": "With Oppia, you can access free lessons on math, physics, statistics, chemistry, music, history and more from anywhere in the world. Oppia is a nonprofit with the mission of providing high-quality education to those who lack access to it."
}
]
},
"LEARNER_GROUP_VIEWER": {
"ROUTE": "learner-group/:learner_group_id",
"TITLE": "I18N_LEARNER_GROUP_PAGE_TITLE",
Expand Down
47 changes: 47 additions & 0 deletions core/controllers/access_validators.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,53 @@ def get(self, learner_group_id: str) -> None:
raise self.PageNotFoundException


class EditLearnerGroupPageAccessValidationHandler(
base.BaseHandler[Dict[str, str], Dict[str, str]]
):
"""Validates access to edit learner group page."""

GET_HANDLER_ERROR_RETURN_TYPE = feconf.HANDLER_TYPE_JSON

URL_PATH_ARGS_SCHEMAS = {
'learner_group_id': {
'schema': {
'type': 'basestring',
'validators': [{
'id': 'is_regex_matched',
'regex_pattern': constants.LEARNER_GROUP_ID_REGEX
}]
}
}
}
HANDLER_ARGS_SCHEMAS: Dict[str, Dict[str, str]] = {
'GET': {}
}

@acl_decorators.can_access_learner_groups
def get(self, learner_group_id: str) -> None:
"""Validates access to edit learner group page.
Args:
learner_group_id: str. The learner group ID.
Raises:
PageNotFoundException. The learner groups are not enabled.
PageNotFoundException. The user is not a member of the learner
group.
"""
assert self.user_id is not None
if not learner_group_services.is_learner_group_feature_enabled(
self.user_id
):
raise self.PageNotFoundException

is_valid_request = learner_group_services.is_user_facilitator(
self.user_id, learner_group_id)

if not is_valid_request:
raise self.PageNotFoundException


class BlogHomePageAccessValidationHandler(
base.BaseHandler[Dict[str, str], Dict[str, str]]
):
Expand Down
67 changes: 67 additions & 0 deletions core/controllers/access_validators_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,73 @@ def test_validation_returns_true_for_valid_learner(self) -> None:
ACCESS_VALIDATION_HANDLER_PREFIX, self.LEARNER_GROUP_ID))


class EditLearnerGroupPageAccessValidationHandlerTests(
test_utils.GenericTestBase
):

def setUp(self) -> None:
super().setUp()
self.signup(self.NEW_USER_EMAIL, self.NEW_USER_USERNAME)
self.signup(
self.CURRICULUM_ADMIN_EMAIL, self.CURRICULUM_ADMIN_USERNAME)

self.facilitator_id = self.get_user_id_from_email(
self.CURRICULUM_ADMIN_EMAIL)

self.LEARNER_GROUP_ID = (
learner_group_fetchers.get_new_learner_group_id()
)
learner_group_services.create_learner_group(
self.LEARNER_GROUP_ID, 'Learner Group Title', 'Description',
[self.facilitator_id], [],
['subtopic_id_1'], ['story_id_1'])

def test_validation_returns_false_with_learner_groups_feature_disabled(
self
) -> None:
self.login(self.CURRICULUM_ADMIN_EMAIL)

swap_is_feature_flag_enabled = self.swap_to_always_return(
feature_flag_services,
'is_feature_flag_enabled',
False
)
with swap_is_feature_flag_enabled:
self.get_json(
'%s/can_access_edit_learner_group_page/%s' % (
ACCESS_VALIDATION_HANDLER_PREFIX, self.LEARNER_GROUP_ID),
expected_status_int=404)

def test_validation_returns_false_with_user_not_being_a_facilitator(
self
) -> None:
self.login(self.NEW_USER_EMAIL)

swap_is_feature_flag_enabled = self.swap_to_always_return(
feature_flag_services,
'is_feature_flag_enabled',
True
)
with swap_is_feature_flag_enabled:
self.get_json(
'%s/can_access_edit_learner_group_page/%s' % (
ACCESS_VALIDATION_HANDLER_PREFIX, self.LEARNER_GROUP_ID),
expected_status_int=404)

def test_validation_returns_true_for_valid_facilitator(self) -> None:
self.login(self.CURRICULUM_ADMIN_EMAIL)

swap_is_feature_flag_enabled = self.swap_to_always_return(
feature_flag_services,
'is_feature_flag_enabled',
True
)
with swap_is_feature_flag_enabled:
self.get_html_response(
'%s/can_access_edit_learner_group_page/%s' % (
ACCESS_VALIDATION_HANDLER_PREFIX, self.LEARNER_GROUP_ID))


class BlogHomePageAccessValidationHandlerTests(test_utils.GenericTestBase):
"""Checks the access to the blog home page and its rendering."""

Expand Down
38 changes: 0 additions & 38 deletions core/controllers/learner_group.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -761,44 +761,6 @@ def get(self) -> None:
})


class EditLearnerGroupPage(
base.BaseHandler[Dict[str, str], Dict[str, str]]
):
"""Page for editing a learner group."""

URL_PATH_ARGS_SCHEMAS = {
'group_id': {
'schema': {
'type': 'basestring',
'validators': [{
'id': 'is_regex_matched',
'regex_pattern': constants.LEARNER_GROUP_ID_REGEX
}]
}
}
}
HANDLER_ARGS_SCHEMAS: Dict[str, Dict[str, str]] = {
'GET': {}
}

@acl_decorators.can_access_learner_groups
def get(self, group_id: str) -> None:
"""Handles GET requests."""
assert self.user_id is not None
if not learner_group_services.is_learner_group_feature_enabled(
self.user_id
):
raise self.PageNotFoundException

is_valid_request = learner_group_services.is_user_facilitator(
self.user_id, group_id)

if not is_valid_request:
raise self.PageNotFoundException

self.render_template('edit-learner-group-page.mainpage.html')


class LearnerGroupLearnersInfoHandler(
base.BaseHandler[Dict[str, str], Dict[str, str]]
):
Expand Down
64 changes: 0 additions & 64 deletions core/controllers/learner_group_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1007,70 +1007,6 @@ def test_searching_a_valid_user_to_invite(self) -> None:
self.logout()


class EditLearnerGroupPageTests(test_utils.GenericTestBase):
"""Checks the access and rendering of the edit learner page."""

LEARNER_ID: Final = 'learner_user_1'

def setUp(self) -> None:
super().setUp()
self.signup(self.OWNER_EMAIL, self.OWNER_USERNAME)
self.FACILITATOR_ID = self.get_user_id_from_email(self.OWNER_EMAIL)
self.signup(self.NEW_USER_EMAIL, self.NEW_USER_USERNAME)
self.login(self.OWNER_EMAIL)
self.learner_group_id = (
learner_group_fetchers.get_new_learner_group_id()
)
self.learner_group = learner_group_services.create_learner_group(
self.learner_group_id, 'Learner Group Name', 'Description',
[self.FACILITATOR_ID], [self.LEARNER_ID], ['subtopic_id_1'],
['story_id_1'])

def test_page_with_disabled_learner_groups_leads_to_404(self) -> None:
swap_is_feature_flag_enabled = self.swap_to_always_return(
feature_flag_services,
'is_feature_flag_enabled',
False
)
with swap_is_feature_flag_enabled:
self.get_html_response(
'/edit-learner-group/%s' % self.learner_group_id,
expected_status_int=404)
self.logout()

def test_page_with_enabled_learner_groups_loads_correctly_for_facilitator(
self
) -> None:
swap_is_feature_flag_enabled = self.swap_to_always_return(
feature_flag_services,
'is_feature_flag_enabled',
True
)
with swap_is_feature_flag_enabled:
response = self.get_html_response(
'/edit-learner-group/%s' % self.learner_group_id)
response.mustcontain(
'<oppia-edit-learner-group-page>'
'</oppia-edit-learner-group-page>')
self.logout()

def test_page_with_enabled_learner_groups_leads_to_404_for_non_facilitators(
self
) -> None:
swap_is_feature_flag_enabled = self.swap_to_always_return(
feature_flag_services,
'is_feature_flag_enabled',
True
)
self.logout()
self.login(self.NEW_USER_EMAIL)
with swap_is_feature_flag_enabled:
self.get_html_response(
'/edit-learner-group/%s' % self.learner_group_id,
expected_status_int=404)
self.logout()


class LearnerGroupLearnerInvitationHandlerTests(test_utils.GenericTestBase):
"""Checks learner successfully accepting or declining a learner group
invitation.
Expand Down
108 changes: 108 additions & 0 deletions ...templates/pages/learner-group-pages/edit-group/edit-learner-group-page-auth.guard.spec.ts
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
// Copyright 2023 The Oppia Authors. All Rights Reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS-IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

/**
* @fileoverview Tests for EditLearnerGroupPageAuthGuard
*/
import { Location } from '@angular/common';
import { TestBed, fakeAsync, tick } from '@angular/core/testing';
import { ActivatedRouteSnapshot, RouterStateSnapshot, Router } from '@angular/router';
import { RouterTestingModule } from '@angular/router/testing';

import { AppConstants } from 'app.constants';
import { EditLearnerGroupPageAuthGuard } from './edit-learner-group-page-auth.guard';
import { AccessValidationBackendApiService } from 'pages/oppia-root/routing/access-validation-backend-api.service';


class MockAccessValidationBackendApiService {
validateAccessToLearnerGroupEditorPage(learnerGroupId: string) {
return Promise.resolve();
}
}

class MockRouter {
navigate(commands: string[]): Promise<boolean> {
return Promise.resolve(true);
}
}

describe('EditLearnerGroupPageAuthGuard', () => {
let guard: EditLearnerGroupPageAuthGuard;
let accessValidationBackendApiService: AccessValidationBackendApiService;
let router: Router;

beforeEach(() => {
TestBed.configureTestingModule({
imports: [RouterTestingModule],
providers: [
EditLearnerGroupPageAuthGuard,
{ provide: AccessValidationBackendApiService,
useClass: MockAccessValidationBackendApiService },
{ provide: Router, useClass: MockRouter },
Location,
],
});

guard = TestBed.inject(EditLearnerGroupPageAuthGuard);
accessValidationBackendApiService = TestBed.inject(
AccessValidationBackendApiService
);
router = TestBed.inject(Router);
});

it('should allow access if validation succeeds', fakeAsync(() => {
const validateAccessSpy = spyOn(
accessValidationBackendApiService,
'validateAccessToLearnerGroupEditorPage')
.and.returnValue(Promise.resolve());
const navigateSpy = spyOn(router, 'navigate')
.and.returnValue(Promise.resolve(true));

let canActivateResult: boolean | null = null;

guard.canActivate(new ActivatedRouteSnapshot(), {} as RouterStateSnapshot)
.then((result) => {
canActivateResult = result;
});

tick();

expect(canActivateResult).toBeTrue();
expect(validateAccessSpy).toHaveBeenCalled();
expect(navigateSpy).not.toHaveBeenCalled();
}));

it('should redirect to 401 page if validation fails', fakeAsync(() => {
spyOn(
accessValidationBackendApiService,
'validateAccessToLearnerGroupEditorPage')
.and.returnValue(Promise.reject());
const navigateSpy = spyOn(router, 'navigate')
.and.returnValue(Promise.resolve(true));

let canActivateResult: boolean | null = null;

guard.canActivate(new ActivatedRouteSnapshot(), {} as RouterStateSnapshot)
.then((result) => {
canActivateResult = result;
});

tick();

expect(canActivateResult).toBeFalse();
expect(navigateSpy).toHaveBeenCalledWith(
[`${AppConstants.PAGES_REGISTERED_WITH_FRONTEND.ERROR.ROUTE}/401`]
);
}));
});
Loading

0 comments on commit e979bb1

Please sign in to comment.