[GSoC'24] Fix part of oppia#19849: Classrooms project bug fixes (oppi…

…a#20581)

* Fix part of oppia#19849: Classrooms project bug fixes

* add check when saving a published classroom

* fix a11y and classroom admin page mobile

* fix linter

* minor fixes

* simplify code

* fix topic issue

core/templates/pages/classroom-admin-page/classroom-admin-page.component.html

@@ -89,7 +89,7 @@ <h3>Classroom details</h3>
                       [(ngModel)]="tempClassroomData._teaserText"
                       (blur)="updateClassroomField()"
                       (ngModelChange)="classroomAdminDataService.validateClassroom(tempClassroomData, classroomData)"
-                      rows="1"
+                      rows="3"
                       class="classroom-field form-control e2e-test-update-classroom-teaser-text">
             </textarea>
           </div>
@@ -105,7 +105,7 @@ <h3>Classroom details</h3>
                       [(ngModel)]="tempClassroomData._topicListIntro"
                       (blur)="updateClassroomField()"
                       (ngModelChange)="classroomAdminDataService.validateClassroom(tempClassroomData, classroomData)"
-                      rows="1"
+                      rows="3"
                       class="classroom-field form-control e2e-test-update-classroom-topic-list-intro">
             </textarea>
           </div>
@@ -121,7 +121,7 @@ <h3>Classroom details</h3>
                       [(ngModel)]="tempClassroomData._courseDetails"
                       (blur)="updateClassroomField()"
                       (ngModelChange)="classroomAdminDataService.validateClassroom(tempClassroomData, classroomData)"
-                      rows="2"
+                      rows="3"
                       class="classroom-field form-control e2e-test-update-classroom-course-details">
             </textarea>
           </div>
@@ -146,7 +146,14 @@ <h3>Classroom details</h3>
 
           <div>
             <hr>
-            <p><strong>Topics and their prerequisites</strong></p>
+            <div class="topic-and-prerequisites-text-container">
+              <p><strong>Topics and their prerequisites</strong></p>
+              <button class="btn btn-secondary view-graph-button"
+                      (click)="viewGraph()"
+                      *ngIf="tempClassroomData.getTopicsCount() > 0">
+                View Graph
+              </button>
+            </div>
             <p class="topic-dependency-description">
               <em>
                 The prerequisite topics below are used to generate the diagnostic test.
@@ -175,7 +182,6 @@ <h3>Classroom details</h3>
                     <span class="topics-name">
                       <strong>{{ topicName }}</strong>
                     </span>
-
                     <div class="prerequisite-topics-container">
                       <div *ngIf="!classroomEditorMode">
                         <span *ngIf="getPrerequisiteLength(topicName) === 0">
@@ -275,11 +281,6 @@ <h3>Classroom details</h3>
           </div>
 
           <div *ngIf="classroomEditorMode" class="action-buttons">
-            <button class="btn btn-secondary view-graph-button"
-                    (click)="viewGraph()"
-                    *ngIf="tempClassroomData.getTopicsCount() > 0">
-              View Graph
-            </button>
             <button class="btn btn-secondary cancel-classroom-changes e2e-cancel-classroom-changes"
                     (click)="closeClassroomConfigEditor()"
                     [disabled]="classroomDataSaveInProgress">
@@ -296,13 +297,13 @@ <h3>Classroom details</h3>
                            [diameter]="22">
               </mat-spinner>
             </button>
-            <span [matTooltip]="validationErrors.join('\n')"
+            <span [matTooltip]="allValidationErrors.join('\n')"
                   matTooltipClass="oppia-mat-tooltip-list"
                   [matTooltipDisabled]="false"
                   aria-label="List that shows issues preventing to publish a classroom"
                   >
               <button *ngIf="!tempClassroomData._isPublished"
-                      [disabled]="!(validationErrors.length === 0)"
+                      [disabled]="!(allValidationErrors.length === 0)"
                       class="btn btn-success publish-classroom-btn e2e-test-publish-classroom-btn"
                       (click)="publishClassroom()"
                   >
@@ -311,25 +312,25 @@ <h3>Classroom details</h3>
                              class="oppia-update-classroom-data-spinning-button"
                              [diameter]="22">
                 </mat-spinner>
-                <mat-icon *ngIf="validationErrors.length > 0" class="save-draft-button-warning">
+                <mat-icon *ngIf="allValidationErrors.length > 0" class="save-draft-button-warning">
                   warning
                 </mat-icon>
               </button>
             </span>
-            <span [matTooltip]="(saveClassroomValidationErrors()).join('\n')"
+            <span [matTooltip]="(saveClassroomValidationErrors).join('\n')"
                   matTooltipClass="oppia-mat-tooltip-list"
                   [matTooltipDisabled]="false"
                   aria-label="List that shows issues preventing to save a classroom"
                   >
               <button class="btn btn-success save-classroom-changes e2e-test-save-classroom-config-button"
-                      [disabled]="!(classroomDataIsChanged && tempClassroomData.isClassroomDataValid())"
+                      [disabled]="!(classroomDataIsChanged && tempClassroomData.isClassroomDataValid() && saveClassroomValidationErrors.length === 0)"
                       (click)="saveClassroomData(classroom.key)">
                 <span *ngIf="!classroomDataSaveInProgress">Save</span>
                 <mat-spinner *ngIf="classroomDataSaveInProgress"
                              class="oppia-update-classroom-data-spinning-button"
                              [diameter]="22">
                 </mat-spinner>
-                <mat-icon *ngIf="saveClassroomValidationErrors().length > 0" class="save-draft-button-warning">
+                <mat-icon *ngIf="saveClassroomValidationErrors.length > 0" class="save-draft-button-warning">
                         warning
                 </mat-icon>
               </button>
@@ -453,10 +454,6 @@ <h3>Classroom details</h3>
   .save-classroom-changes {
     margin-top: 5px;
   }
-  .view-graph-button {
-    margin-right: 5px;
-    margin-top: 5px;
-  }
   .cancel-classroom-changes, .publish-classroom-btn {
     margin-right: 5px;
     margin-top: 5px;
@@ -554,9 +551,25 @@ <h3>Classroom details</h3>
     margin-left: 3px;
     z-index: 10;
   }
+
+  .topic-and-prerequisites-text-container {
+    align-items: start;
+    display: flex;
+    flex-direction: row;
+    justify-content: space-between;
+    width: 100%;
+  }
   @media screen and (max-width: 768px) {
     .oppia-editor-card-with-avatar {
       margin-top: 20px;
     }
+
+    .oppia-classroom-input-container span {
+      padding-left: 0;
+    }
+
+    .topic-dependency-container {
+      overflow: auto;
+    }
   }
 </style>

core/templates/pages/classroom-admin-page/classroom-admin-page.component.spec.ts

@@ -1307,7 +1307,7 @@ describe('Classroom Admin Page component ', () => {
       response.classroomDict
     );
 
-    expect(component.validationErrors.length).toEqual(0);
+    expect(component.allValidationErrors.length).toEqual(0);
     expect(component.classroomDataPublishInProgress).toBeFalse();
     spyOn(component, 'updateClassroomData').and.returnValue(Promise.resolve());
     tick();
@@ -1333,7 +1333,7 @@ describe('Classroom Admin Page component ', () => {
     component.classroomDataIsChanged = false;
     component.updateClassroomField();
 
-    expect(component.validationErrors.length).toEqual(3);
+    expect(component.allValidationErrors.length).toEqual(3);
   });
 
   it('should not be able to save classroom due to validation errors', () => {
@@ -1342,6 +1342,29 @@ describe('Classroom Admin Page component ', () => {
         ...dummyClassroomDict,
         name: '',
         urlFragment: '',
+        isPublished: false,
+      },
+    };
+    component.tempClassroomData = ExistingClassroomData.createClassroomFromDict(
+      response.classroomDict
+    );
+    component.classroomData = ExistingClassroomData.createClassroomFromDict(
+      response.classroomDict
+    );
+    component.updateClassroomField();
+
+    expect(component.saveClassroomValidationErrors.length).toEqual(2);
+  });
+
+  it('should not save a published classroom if user deletes some data', () => {
+    spyOn(component, 'updateClassroomData');
+    const response = {
+      classroomDict: {
+        ...dummyClassroomDict,
+        name: '',
+        urlFragment: '',
+        isPublished: true,
+        teaserText: '',
       },
     };
     component.tempClassroomData = ExistingClassroomData.createClassroomFromDict(
@@ -1351,8 +1374,10 @@ describe('Classroom Admin Page component ', () => {
       response.classroomDict
     );
     component.updateClassroomField();
+    component.saveClassroomData();
 
-    expect(component.saveClassroomValidationErrors().length).toEqual(2);
+    expect(component.canSaveClassroom()).toBeFalse();
+    expect(component.updateClassroomData).not.toHaveBeenCalled();
   });
 
   it('should be able to unpublish a published classroom', fakeAsync(() => {

core/templates/pages/classroom-admin-page/classroom-admin-page.component.ts

@@ -103,7 +103,8 @@ export class ClassroomAdminPageComponent implements OnInit {
 
   topicsToClassroomRelation: TopicClassroomRelationDict[] = [];
   filteredTopicsToClassroomRelation: TopicClassroomRelationDict[] = [];
-  validationErrors: string[] = [];
+  allValidationErrors: string[] = [];
+  saveClassroomValidationErrors: string[] = [];
 
   thumbnailParameters: ImageUploaderParameters = {
     disabled: false,
@@ -128,6 +129,7 @@ export class ClassroomAdminPageComponent implements OnInit {
 
   getEligibleTopicPrerequisites(currentTopicName: string): void {
     this.eligibleTopicNamesForPrerequisites = [];
+    this.tempEligibleTopicNamesForPrerequisites = [];
     this.prerequisiteInput = '';
     let topicNames = Object.keys(this.topicNameToPrerequisiteTopicNames);
 
@@ -185,6 +187,8 @@ export class ClassroomAdminPageComponent implements OnInit {
         );
 
         this.classroomDataIsChanged = false;
+        this.eligibleTopicNamesForPrerequisites = [];
+        this.tempEligibleTopicNamesForPrerequisites = [];
 
         this.existingClassroomNames = Object.values(
           this.classroomIdToClassroomName
@@ -204,8 +208,10 @@ export class ClassroomAdminPageComponent implements OnInit {
           this.tempClassroomData,
           this.classroomData
         );
-        this.validationErrors =
+        this.allValidationErrors =
           this.classroomAdminDataService.getAllClassroomValidationErrors();
+        this.saveClassroomValidationErrors =
+          this.getSaveClassroomValidationErrors();
         this.setTopicDependencyByTopicName(
           this.tempClassroomData.getTopicIdToPrerequisiteTopicId()
         );
@@ -329,8 +335,10 @@ export class ClassroomAdminPageComponent implements OnInit {
       this.tempClassroomData,
       this.classroomData
     );
-    this.validationErrors =
+    this.allValidationErrors =
       this.classroomAdminDataService.getAllClassroomValidationErrors();
+    this.saveClassroomValidationErrors =
+      this.getSaveClassroomValidationErrors();
     const topicDependencyIsChanged =
       JSON.stringify(
         this.tempClassroomData.getTopicIdToPrerequisiteTopicId()
@@ -377,6 +385,7 @@ export class ClassroomAdminPageComponent implements OnInit {
     this.updateClassroomData(this.tempClassroomData.getClassroomId()).then(
       () => {
         this.classroomDataUnpublishInProgress = false;
+        this.openClassroomInViewerMode();
       }
     );
   }
@@ -392,9 +401,12 @@ export class ClassroomAdminPageComponent implements OnInit {
   }
 
   saveClassroomData(classroomId: string): void {
+    if (!this.canSaveClassroom()) {
+      return;
+    }
     this.classroomDataSaveInProgress = true;
-    this.openClassroomInViewerMode();
     this.updateClassroomData(classroomId).then(() => {
+      this.openClassroomInViewerMode();
       this.classroomDataIsChanged = false;
     });
   }
@@ -761,7 +773,14 @@ export class ClassroomAdminPageComponent implements OnInit {
     );
   }
 
-  saveClassroomValidationErrors(): string[] {
+  canSaveClassroom(): boolean {
+    return this.getSaveClassroomValidationErrors().length === 0;
+  }
+
+  getSaveClassroomValidationErrors(): string[] {
+    if (this.tempClassroomData.getIsPublished()) {
+      return this.allValidationErrors;
+    }
     return this.classroomAdminDataService.getSaveClassroomValidationErrors();
   }
 

core/templates/pages/classroom-admin-page/existing-classroom.model.ts

@@ -348,7 +348,7 @@ export class ExistingClassroomData
 
   private getClassroomBannerValidationErrors(): string {
     let errorMsg = '';
-    if (this._thumbnail_data.filename === '') {
+    if (this._banner_data.filename === '') {
       errorMsg = 'The classroom banner should not be empty.';
     }
     return errorMsg;

core/templates/pages/classroom-admin-page/modals/create-new-classroom-modal.component.html

@@ -7,11 +7,10 @@ <h3>
   <div class="row new-classroom-input-data">
     <span class="col-lg-3 col-md-3 col-sm-3">Name</span>
     <div class="new-classroom-name-input">
-      <textarea [(ngModel)]="tempClassroom._name"
-                rows="1"
-                class="e2e-test-new-classroom-name"
-                (ngModelChange)="classroomAdminDataService.validateClassroom(tempClassroom, classroom)">
-      </textarea>
+      <input [(ngModel)]="tempClassroom._name"
+             rows="1"
+             class="e2e-test-new-classroom-name form-control required"
+             (ngModelChange)="classroomAdminDataService.validateClassroom(tempClassroom, classroom)">
       <p *ngIf="classroomAdminDataService.nameValidationError.length > 0" class="oppia-warning-text">
         {{ classroomAdminDataService.nameValidationError }}
       </p>
@@ -21,11 +20,10 @@ <h3>
   <div class="row new-classroom-input-data">
     <span class="col-lg-3 col-md-3 col-sm-3">URL Fragment</span>
     <div class="new-classroom-name-input">
-      <textarea [(ngModel)]="tempClassroom._urlFragment"
-                rows="1"
-                class="e2e-test-new-classroom-url-fragment"
-                (ngModelChange)="classroomAdminDataService.validateClassroom(tempClassroom, classroom)">
-      </textarea>
+      <input [(ngModel)]="tempClassroom._urlFragment"
+             rows="1"
+             class="e2e-test-new-classroom-url-fragment form-control required"
+             (ngModelChange)="classroomAdminDataService.validateClassroom(tempClassroom, classroom)">
       <p *ngIf="classroomAdminDataService.urlValidationError.length > 0" class="oppia-warning-text">
         {{ classroomAdminDataService.urlValidationError }}
       </p>

core/templates/pages/topic-editor-page/editor-tab/topic-editor-tab.directive.html

@@ -167,7 +167,7 @@ <h3 class="topic-card-header">Details</h3>
               </span>
               <em *ngIf="!isAssignedToAClassroom()">No classroom assigned</em>
               <div *ngIf="isAssignedToAClassroom()" class="oppia-input-box-subtitle">
-                <em>To change the assigned classroom, please contact one of the curriculum admins:</em><br>
+                <em>To change the assigned classroom, please contact one of the curriculum admins:</em>
                 <em *ngFor="let username of curriculumAdminUsernames; let last = last">
                   {{ username }}<ng-container *ngIf="!last">, </ng-container>
                 </em>

core/templates/pages/topics-and-skills-dashboard-page/topics-and-skills-dashboard-page.component.html

@@ -200,12 +200,12 @@
                 </div>
               </div>
             </div>
-            <div class="topic-skill-filter-row">
+            <div class="topic-skill-filter-row" *ngIf="activeTab === TAB_NAME_TOPICS">
               <div class="topic-skill-filter-key">
                 <span>Classroom</span>
               </div>
               <div class="topic-skill-filter-value">
-                <div *ngIf="activeTab === TAB_NAME_TOPICS">
+                <div>
                   <oppia-filtered-choices-field class="e2e-test-select-classroom-dropdown"
                                                 [choices]="classrooms"
                                                 placeholder="Classrooms"
@@ -214,14 +214,6 @@
                                                 (selectionChange)="applyFilters()">
                   </oppia-filtered-choices-field>
                 </div>
-                <div *ngIf="activeTab === TAB_NAME_SKILLS">
-                  <oppia-filtered-choices-field [choices]="classrooms"
-                                                placeholder="Classrooms"
-                                                searchLabel="Search"
-                                                [(selection)]="filterObject.classroom"
-                                                (selectionChange)="applyFilters()">
-                  </oppia-filtered-choices-field>
-                </div>
               </div>
             </div>
             <div class="topic-skill-filter-row">