From 01ef79277b473d566274952dc6f2c49ebb4e89ff Mon Sep 17 00:00:00 2001 From: Afzal Khan Date: Mon, 8 Jul 2024 18:29:09 -0400 Subject: [PATCH] [GSoC'24] Fix part of #19849: Classroom page bug fixes #3 (#20632) * Fix part of #19849: Classroom page bug fixes * add all classroom sorting option * use double negation --- core/controllers/classroom.py | 1 + core/controllers/classroom_test.py | 1 + .../classroom/classroom-backend-api.service.spec.ts | 2 ++ .../domain/classroom/classroom-backend-api.service.ts | 2 ++ .../domain/classroom/classroom-data.model.spec.ts | 2 ++ core/templates/domain/classroom/classroom-data.model.ts | 9 +++++++++ .../pages/classroom-page/classroom-page.component.css | 4 ++-- .../pages/classroom-page/classroom-page.component.html | 4 ++-- .../classroom-page/classroom-page.component.spec.ts | 4 +++- .../pages/classroom-page/classroom-page.component.ts | 4 ++-- ...tor-dashboard-admin-stats-backend-api.service.spec.ts | 2 ++ .../diagnostic-test-player.component.spec.ts | 1 + .../topics-and-skills-dashboard-page.component.ts | 7 +++++-- .../topics-and-skills-dashboard-page.constants.ajs.ts | 6 ++++++ .../topics-and-skills-dashboard-page.constants.ts | 1 + .../topics-and-skills-dashboard-page.service.spec.ts | 4 ++++ .../topics-and-skills-dashboard-page.service.ts | 8 ++++++++ 17 files changed, 53 insertions(+), 9 deletions(-) diff --git a/core/controllers/classroom.py b/core/controllers/classroom.py index 6d3b4c0d7a9d..2c41b67c7d72 100644 --- a/core/controllers/classroom.py +++ b/core/controllers/classroom.py @@ -128,6 +128,7 @@ def get(self, classroom_url_fragment: str) -> None: 'topic_list_intro': classroom.topic_list_intro, 'course_details': classroom.course_details, 'name': classroom.name, + 'url_fragment': classroom.url_fragment, 'teaser_text': classroom.teaser_text, 'is_published': classroom.is_published, 'thumbnail_data': classroom.thumbnail_data.to_dict(), diff --git a/core/controllers/classroom_test.py b/core/controllers/classroom_test.py index de49103b66c8..6273aa4e1ce1 100644 --- a/core/controllers/classroom_test.py +++ b/core/controllers/classroom_test.py @@ -217,6 +217,7 @@ def test_get(self) -> None: expected_dict = { 'classroom_id': 'test_id', 'name': 'math', + 'url_fragment': 'math', 'topic_summary_dicts': [ public_topic_1_summary_dict, private_topic_summary_dict ], diff --git a/core/templates/domain/classroom/classroom-backend-api.service.spec.ts b/core/templates/domain/classroom/classroom-backend-api.service.spec.ts index 3c1689f6cf4e..e14fbfb16dc7 100644 --- a/core/templates/domain/classroom/classroom-backend-api.service.spec.ts +++ b/core/templates/domain/classroom/classroom-backend-api.service.spec.ts @@ -82,6 +82,7 @@ describe('Classroom backend API service', function () { let responseDictionaries = { classroom_id: 'mathid', name: 'Math', + url_fragment: 'math', topic_summary_dicts: [firstTopicSummaryDict, secondTopicSummaryDict], course_details: 'Course Details', topic_list_intro: 'Topics Covered', @@ -131,6 +132,7 @@ describe('Classroom backend API service', function () { sampleClassroomDataObject = ClassroomData.createFromBackendData( responseDictionaries.classroom_id, responseDictionaries.name, + responseDictionaries.url_fragment, responseDictionaries.topic_summary_dicts, responseDictionaries.course_details, responseDictionaries.topic_list_intro, diff --git a/core/templates/domain/classroom/classroom-backend-api.service.ts b/core/templates/domain/classroom/classroom-backend-api.service.ts index 4612735eadee..d62b6b4e5f96 100644 --- a/core/templates/domain/classroom/classroom-backend-api.service.ts +++ b/core/templates/domain/classroom/classroom-backend-api.service.ts @@ -29,6 +29,7 @@ import {ImageData} from 'pages/classroom-admin-page/existing-classroom.model'; export interface ClassroomDataBackendDict { classroom_id: string; name: string; + url_fragment: string; topic_summary_dicts: CreatorTopicSummaryBackendDict[]; course_details: string; teaser_text: string; @@ -164,6 +165,7 @@ export class ClassroomBackendApiService { this.classroomData = ClassroomData.createFromBackendData( response.classroom_id, response.name, + response.url_fragment, response.topic_summary_dicts, response.course_details, response.topic_list_intro, diff --git a/core/templates/domain/classroom/classroom-data.model.spec.ts b/core/templates/domain/classroom/classroom-data.model.spec.ts index f7233e4de2b9..30902bf8ba38 100644 --- a/core/templates/domain/classroom/classroom-data.model.spec.ts +++ b/core/templates/domain/classroom/classroom-data.model.spec.ts @@ -59,6 +59,7 @@ describe('Classroom data model', () => { let classroomData = ClassroomData.createFromBackendData( 'mathid', 'Math', + 'math', topicSummaryDicts, 'Course Details', 'Topics Covered', @@ -68,6 +69,7 @@ describe('Classroom data model', () => { {filename: 'banner.png', size_in_bytes: 100, bg_color: 'transparent'} ); expect(classroomData.getName()).toEqual('Math'); + expect(classroomData.getUrlFragment()).toEqual('math'); expect(classroomData.getCourseDetails()).toEqual('Course Details'); expect(classroomData.getTopicListIntro()).toEqual('Topics Covered'); expect(classroomData.getTopicSummaries()[0]).toEqual( diff --git a/core/templates/domain/classroom/classroom-data.model.ts b/core/templates/domain/classroom/classroom-data.model.ts index 2dd24c8609aa..1189d7145b2a 100644 --- a/core/templates/domain/classroom/classroom-data.model.ts +++ b/core/templates/domain/classroom/classroom-data.model.ts @@ -25,6 +25,7 @@ import {ImageData} from 'pages/classroom-admin-page/existing-classroom.model'; export class ClassroomData { _classroom_id: string; _name: string; + _urlFragment: string; _topicSummaries: CreatorTopicSummary[]; _courseDetails: string; _topicListIntro: string; @@ -37,6 +38,7 @@ export class ClassroomData { constructor( classroomId: string, name: string, + urlFragment: string, topicSummaries: CreatorTopicSummary[], courseDetails: string, topicListIntro: string, @@ -48,6 +50,7 @@ export class ClassroomData { ) { this._classroom_id = classroomId; this._name = name; + this._urlFragment = urlFragment; this._topicSummaries = topicSummaries; this._courseDetails = courseDetails; this._topicListIntro = topicListIntro; @@ -61,6 +64,7 @@ export class ClassroomData { static createFromBackendData( classroomId: string, name: string, + urlFragment: string, topicSummaryDicts: CreatorTopicSummaryBackendDict[], courseDetails: string, topicListIntro: string, @@ -76,6 +80,7 @@ export class ClassroomData { return new ClassroomData( classroomId, name, + urlFragment, topicSummaries, courseDetails, topicListIntro, @@ -91,6 +96,10 @@ export class ClassroomData { return this._name; } + getUrlFragment(): string { + return this._urlFragment; + } + getTopicSummaries(): CreatorTopicSummary[] { return this._topicSummaries.slice(); } diff --git a/core/templates/pages/classroom-page/classroom-page.component.css b/core/templates/pages/classroom-page/classroom-page.component.css index 7e403db1ec0e..420364a3ecfb 100644 --- a/core/templates/pages/classroom-page/classroom-page.component.css +++ b/core/templates/pages/classroom-page/classroom-page.component.css @@ -73,7 +73,7 @@ font-size: 16px; font-style: normal; font-weight: 700; - height: 43px; + height: fit-content; letter-spacing: -0.02em; line-height: 20px; width: 266px; @@ -86,7 +86,7 @@ font-style: normal; font-weight: 400; letter-spacing: -0.02em; - line-height: 20px; + line-height: 16px; } .diagnostic-test-container .diagnostic-test-box a { diff --git a/core/templates/pages/classroom-page/classroom-page.component.html b/core/templates/pages/classroom-page/classroom-page.component.html index d0a62ee97bc4..25f09ed9ed6b 100644 --- a/core/templates/pages/classroom-page/classroom-page.component.html +++ b/core/templates/pages/classroom-page/classroom-page.component.html @@ -1,4 +1,4 @@ -
+
{{'I18N_CLASSROOM_PAGE_TOPICS_COVERED' | t
-

{{ 'I18N_CLASSROOM_PAGE_NEW_TO_MATH_HEADING' | translate }}?

+

{{ 'I18N_CLASSROOM_PAGE_NEW_TO_MATH_HEADING' | translate }}

{{ beginWithFirstTopicButtonText }}

{{ 'I18N_START_FIRST_CLASSROOM_TOPIC' | translate }} diff --git a/core/templates/pages/classroom-page/classroom-page.component.spec.ts b/core/templates/pages/classroom-page/classroom-page.component.spec.ts index 5322d10d7a73..743918d947d1 100644 --- a/core/templates/pages/classroom-page/classroom-page.component.spec.ts +++ b/core/templates/pages/classroom-page/classroom-page.component.spec.ts @@ -190,6 +190,7 @@ describe('Classroom Page Component', () => { let classroomData = ClassroomData.createFromBackendData( 'mathid', 'Math', + 'math', topicSummaryDicts, 'Course details', 'Topics covered', @@ -271,7 +272,7 @@ describe('Classroom Page Component', () => { classroomBackendApiService.fetchClassroomDataAsync ).toHaveBeenCalled(); expect(alertsService.addWarning).toHaveBeenCalledWith( - 'Failed to get dashboard data' + 'Failed to get classroom data' ); })); @@ -370,6 +371,7 @@ describe('Classroom Page Component', () => { let classroomData = ClassroomData.createFromBackendData( 'mathid', 'Math', + 'math', [], 'Course details', 'Topics covered', diff --git a/core/templates/pages/classroom-page/classroom-page.component.ts b/core/templates/pages/classroom-page/classroom-page.component.ts index 345cda0c2d47..a89518f4db3c 100644 --- a/core/templates/pages/classroom-page/classroom-page.component.ts +++ b/core/templates/pages/classroom-page/classroom-page.component.ts @@ -160,7 +160,7 @@ export class ClassroomPageComponent implements OnDestroy { ) { let firstTopic = classroomData.getTopicSummaries()[0].name; this.firstTopicUrl = - '/learn/math/' + + `/learn/${classroomData.getUrlFragment()}/` + classroomData.getTopicSummaries()[0].urlFragment; this.beginWithFirstTopicButtonText = @@ -191,7 +191,7 @@ export class ClassroomPageComponent implements OnDestroy { errorResponse.status ) !== -1 ) { - this.alertsService.addWarning('Failed to get dashboard data'); + this.alertsService.addWarning('Failed to get classroom data'); } } ); diff --git a/core/templates/pages/contributor-dashboard-admin-page/services/contributor-dashboard-admin-stats-backend-api.service.spec.ts b/core/templates/pages/contributor-dashboard-admin-page/services/contributor-dashboard-admin-stats-backend-api.service.spec.ts index 918d5a03ea46..91e812f01338 100644 --- a/core/templates/pages/contributor-dashboard-admin-page/services/contributor-dashboard-admin-stats-backend-api.service.spec.ts +++ b/core/templates/pages/contributor-dashboard-admin-page/services/contributor-dashboard-admin-stats-backend-api.service.spec.ts @@ -89,6 +89,7 @@ describe('Contribution Admin dashboard stats service', () => { let responseDictionaries = { classroom_id: 'mathid', name: 'Math', + url_fragment: 'math', topic_summary_dicts: [firstTopicSummaryDict, secondTopicSummaryDict], course_details: 'Course Details', topic_list_intro: 'Topics Covered', @@ -199,6 +200,7 @@ describe('Contribution Admin dashboard stats service', () => { sampleClassroomDataObject = ClassroomData.createFromBackendData( responseDictionaries.classroom_id, responseDictionaries.name, + responseDictionaries.url_fragment, responseDictionaries.topic_summary_dicts, responseDictionaries.course_details, responseDictionaries.topic_list_intro, diff --git a/core/templates/pages/diagnostic-test-player-page/diagnostic-test-player.component.spec.ts b/core/templates/pages/diagnostic-test-player-page/diagnostic-test-player.component.spec.ts index a77e55efd1c5..00a941c45557 100644 --- a/core/templates/pages/diagnostic-test-player-page/diagnostic-test-player.component.spec.ts +++ b/core/templates/pages/diagnostic-test-player-page/diagnostic-test-player.component.spec.ts @@ -237,6 +237,7 @@ describe('Diagnostic test player component', () => { let classroomData = new ClassroomData( 'id', 'test', + 'test', array, 'dummy', 'dummy', diff --git a/core/templates/pages/topics-and-skills-dashboard-page/topics-and-skills-dashboard-page.component.ts b/core/templates/pages/topics-and-skills-dashboard-page/topics-and-skills-dashboard-page.component.ts index 9ddeb5cf0f35..023524fa3b05 100644 --- a/core/templates/pages/topics-and-skills-dashboard-page/topics-and-skills-dashboard-page.component.ts +++ b/core/templates/pages/topics-and-skills-dashboard-page/topics-and-skills-dashboard-page.component.ts @@ -93,7 +93,9 @@ export class TopicsAndSkillsDashboardPageComponent { skillPageNumber: number = 0; lastSkillPage: number = 0; itemsPerPageChoice: number[] = [10, 15, 20]; - classrooms: string[] = []; + classrooms: string[] = [ + TopicsAndSkillsDashboardPageConstants.TOPIC_FILTER_ONLY_CLASSROOMS, + ]; sortOptions: string[] = []; statusOptions: (ETopicPublishedOptions | ETopicStatusOptions)[] = []; displayedTopicSummaries: CreatorTopicSummary[] = []; @@ -419,7 +421,8 @@ export class TopicsAndSkillsDashboardPageComponent { this.initSkillDashboard(); this.focusManagerService.setFocus('createSkillBtn'); } - this.classrooms = response.allClassroomNames; + + this.classrooms.push(...response.allClassroomNames); }); } } diff --git a/core/templates/pages/topics-and-skills-dashboard-page/topics-and-skills-dashboard-page.constants.ajs.ts b/core/templates/pages/topics-and-skills-dashboard-page/topics-and-skills-dashboard-page.constants.ajs.ts index 294728e51950..74aded464f80 100644 --- a/core/templates/pages/topics-and-skills-dashboard-page/topics-and-skills-dashboard-page.constants.ajs.ts +++ b/core/templates/pages/topics-and-skills-dashboard-page/topics-and-skills-dashboard-page.constants.ajs.ts @@ -45,6 +45,12 @@ angular 'TOPIC_FILTER_CLASSROOM_ALL', TopicsAndSkillsDashboardPageConstants.TOPIC_FILTER_CLASSROOM_ALL ); +angular + .module('oppia') + .constant( + 'TOPIC_FILTER_ONLY_CLASSROOMS', + TopicsAndSkillsDashboardPageConstants.TOPIC_FILTER_ONLY_CLASSROOMS + ); angular .module('oppia') .constant( diff --git a/core/templates/pages/topics-and-skills-dashboard-page/topics-and-skills-dashboard-page.constants.ts b/core/templates/pages/topics-and-skills-dashboard-page/topics-and-skills-dashboard-page.constants.ts index cb04ff75e702..885aa768e252 100644 --- a/core/templates/pages/topics-and-skills-dashboard-page/topics-and-skills-dashboard-page.constants.ts +++ b/core/templates/pages/topics-and-skills-dashboard-page/topics-and-skills-dashboard-page.constants.ts @@ -57,5 +57,6 @@ export const TopicsAndSkillsDashboardPageConstants = { TOPIC_PUBLISHED_OPTIONS: ETopicPublishedOptions, TOPIC_STATUS_OPTIONS: ETopicStatusOptions, TOPIC_FILTER_CLASSROOM_ALL: 'All', + TOPIC_FILTER_ONLY_CLASSROOMS: 'All Classrooms', SKILL_STATUS_OPTIONS: AppConstants.SKILL_STATUS_OPTIONS, } as const; diff --git a/core/templates/pages/topics-and-skills-dashboard-page/topics-and-skills-dashboard-page.service.spec.ts b/core/templates/pages/topics-and-skills-dashboard-page/topics-and-skills-dashboard-page.service.spec.ts index 54e20e3f2fd5..a78cade91318 100644 --- a/core/templates/pages/topics-and-skills-dashboard-page/topics-and-skills-dashboard-page.service.spec.ts +++ b/core/templates/pages/topics-and-skills-dashboard-page/topics-and-skills-dashboard-page.service.spec.ts @@ -197,6 +197,10 @@ describe('Topic and Skill dashboard page service', () => { filteredArray = tsds.getFilteredTopics(topicsArray, filterOptions); expect(filteredArray).toEqual([topic1, topic2, topic3]); + filterOptions.classroom = 'All Classrooms'; + filteredArray = tsds.getFilteredTopics(topicsArray, filterOptions); + expect(filteredArray).toEqual(topicsArray); + filterOptions.sort = ETopicSortOptions.IncreasingCreatedOn; filterOptions.classroom = 'Math'; filteredArray = tsds.getFilteredTopics(topicsArray, filterOptions); diff --git a/core/templates/pages/topics-and-skills-dashboard-page/topics-and-skills-dashboard-page.service.ts b/core/templates/pages/topics-and-skills-dashboard-page/topics-and-skills-dashboard-page.service.ts index 9baf0119ed5b..4e14e7066cbc 100644 --- a/core/templates/pages/topics-and-skills-dashboard-page/topics-and-skills-dashboard-page.service.ts +++ b/core/templates/pages/topics-and-skills-dashboard-page/topics-and-skills-dashboard-page.service.ts @@ -77,6 +77,14 @@ export class TopicsAndSkillsDashboardPageService { if (filterObject.classroom === 'Unassigned' && !topic.classroom) { return true; } + + if ( + filterObject.classroom === + TopicsAndSkillsDashboardPageConstants.TOPIC_FILTER_ONLY_CLASSROOMS + ) { + return !!topic.classroom; + } + return ( topic.classroom && filterObject.classroom.toLowerCase() === topic.classroom.toLowerCase()