Skip to content

Commit

Permalink
[GSoC'24] Fix part of oppia#19849: Classroom page bug fixes oppia#3 (o…
Browse files Browse the repository at this point in the history
…ppia#20632)

* Fix part of oppia#19849: Classroom page bug fixes

* add all classroom sorting option

* use double negation
  • Loading branch information
AFZL210 authored Jul 8, 2024
1 parent 5b6a254 commit 01ef792
Show file tree
Hide file tree
Showing 17 changed files with 53 additions and 9 deletions.
1 change: 1 addition & 0 deletions core/controllers/classroom.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
1 change: 1 addition & 0 deletions core/controllers/classroom_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions core/templates/domain/classroom/classroom-data.model.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ describe('Classroom data model', () => {
let classroomData = ClassroomData.createFromBackendData(
'mathid',
'Math',
'math',
topicSummaryDicts,
'Course Details',
'Topics Covered',
Expand All @@ -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(
Expand Down
9 changes: 9 additions & 0 deletions core/templates/domain/classroom/classroom-data.model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -37,6 +38,7 @@ export class ClassroomData {
constructor(
classroomId: string,
name: string,
urlFragment: string,
topicSummaries: CreatorTopicSummary[],
courseDetails: string,
topicListIntro: string,
Expand All @@ -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;
Expand All @@ -61,6 +64,7 @@ export class ClassroomData {
static createFromBackendData(
classroomId: string,
name: string,
urlFragment: string,
topicSummaryDicts: CreatorTopicSummaryBackendDict[],
courseDetails: string,
topicListIntro: string,
Expand All @@ -76,6 +80,7 @@ export class ClassroomData {
return new ClassroomData(
classroomId,
name,
urlFragment,
topicSummaries,
courseDetails,
topicListIntro,
Expand All @@ -91,6 +96,10 @@ export class ClassroomData {
return this._name;
}

getUrlFragment(): string {
return this._urlFragment;
}

getTopicSummaries(): CreatorTopicSummary[] {
return this._topicSummaries.slice();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<div class="oppia-classroom-page-container">
<div class="oppia-classroom-page-container" *ngIf="classroomData">
<div class="classroom-top breadcrumbs-container" aria-label="Classroom banner" [style.background-image]="'url(' + classroomBanner + ')'">
<div *ngIf="showPrivateClassroomBanner"
class="classroom-admin-banner"
Expand Down Expand Up @@ -48,7 +48,7 @@ <h1 class="classroom-content-heading">{{'I18N_CLASSROOM_PAGE_TOPICS_COVERED' | t
<div *ngIf="isDiagnosticTestFeatureFlagEnabled()" class="content-section diagnostic-test-container">
<div class="diagnostic-test-box">
<div class="diagnostic-test-info">
<h4>{{ 'I18N_CLASSROOM_PAGE_NEW_TO_MATH_HEADING' | translate }}?</h4>
<h4>{{ 'I18N_CLASSROOM_PAGE_NEW_TO_MATH_HEADING' | translate }}</h4>
<p>{{ beginWithFirstTopicButtonText }}</p>
</div>
<a [href]="firstTopicUrl">{{ 'I18N_START_FIRST_CLASSROOM_TOPIC' | translate }}</a>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,7 @@ describe('Classroom Page Component', () => {
let classroomData = ClassroomData.createFromBackendData(
'mathid',
'Math',
'math',
topicSummaryDicts,
'Course details',
'Topics covered',
Expand Down Expand Up @@ -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'
);
}));

Expand Down Expand Up @@ -370,6 +371,7 @@ describe('Classroom Page Component', () => {
let classroomData = ClassroomData.createFromBackendData(
'mathid',
'Math',
'math',
[],
'Course details',
'Topics covered',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down Expand Up @@ -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');
}
}
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,7 @@ describe('Diagnostic test player component', () => {
let classroomData = new ClassroomData(
'id',
'test',
'test',
array,
'dummy',
'dummy',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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[] = [];
Expand Down Expand Up @@ -419,7 +421,8 @@ export class TopicsAndSkillsDashboardPageComponent {
this.initSkillDashboard();
this.focusManagerService.setFocus('createSkillBtn');
}
this.classrooms = response.allClassroomNames;

this.classrooms.push(...response.allClassroomNames);
});
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down

0 comments on commit 01ef792

Please sign in to comment.