Skip to content

Commit

Permalink
fix(Manage Students): Remove student does not update the UI (#1450)
Browse files Browse the repository at this point in the history
  • Loading branch information
geoffreykwan authored Oct 13, 2023
1 parent 2ff3f69 commit 06336ba
Show file tree
Hide file tree
Showing 10 changed files with 190 additions and 60 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,12 @@ <h4 class="mat-subtitle-2 warn" *ngIf="isUnassigned" i18n>Students without a tea
<h4 class="mat-subtitle-2" *ngIf="!isUnassigned" i18n>Team {{ team.workgroupId }}</h4>
<span fxFlex></span>
<span class="mat-small">
<a *ngIf="canChangePeriod" class="change-period" href="#" (click)="changePeriod($event)" i18n
<a
*ngIf="canGradeStudentWork && team.users.length > 0 && !isUnassigned"
class="change-period"
href="#"
(click)="changePeriod($event)"
i18n
>Change Period</a
>
<span *ngIf="this.team.users.length === 0" i18n>No students</span>
Expand All @@ -39,7 +44,11 @@ <h4 class="mat-subtitle-2" *ngIf="!isUnassigned" i18n>Team {{ team.workgroupId }
(cdkDragExited)="dragExit($event)"
[cdkDragPreviewContainer]="'parent'"
>
<manage-user class="user control-bg-bg mat-elevation-z1" [user]="user"></manage-user>
<manage-user
class="user control-bg-bg mat-elevation-z1"
[user]="user"
(removeUserEvent)="removeUser($event)"
></manage-user>
</li>
</ng-container>
</ul>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,75 +1,108 @@
import { ComponentFixture, TestBed } from '@angular/core/testing';
import { MatCardModule } from '@angular/material/card';
import { MatDialog } from '@angular/material/dialog';
import { MatDialog, MatDialogModule, MatDialogRef } from '@angular/material/dialog';
import { MatSnackBarModule } from '@angular/material/snack-bar';
import { By } from '@angular/platform-browser';
import { UpdateWorkgroupService } from '../../../../../../app/services/updateWorkgroupService';
import { ConfigService } from '../../../../services/configService';
import { ManageTeamComponent } from './manage-team.component';
import { NO_ERRORS_SCHEMA } from '@angular/core';
import { TestbedHarnessEnvironment } from '@angular/cdk/testing/testbed';
import { ManageTeamHarness } from './manage-team.harness';
import { ManageStudentsModule } from '../manage-students.module';
import { HttpClientTestingModule } from '@angular/common/http/testing';
import { of } from 'rxjs';
import { RemoveUserConfirmDialogComponent } from '../remove-user-confirm-dialog/remove-user-confirm-dialog.component';
import { HttpClient } from '@angular/common/http';
import { BrowserAnimationsModule } from '@angular/platform-browser/animations';

class ConfigServiceStub {
getPermissions() {}
retrieveConfig() {
return {};
}
}

class UpdateWorkgroupServiceStub {}

let component: ManageTeamComponent;
let configService: ConfigService;
let dialog: MatDialog;
let fixture: ComponentFixture<ManageTeamComponent>;
let component: ManageTeamComponent;
let getPermissionsSpy: jasmine.Spy;
let http: HttpClient;
let manageTeamHarness: ManageTeamHarness;
const studentName = 'a a';
const studentUsername = 'aa0101';

describe('ManageTeamComponent', () => {
beforeEach(() => {
beforeEach(async () => {
TestBed.configureTestingModule({
declarations: [ManageTeamComponent],
imports: [MatSnackBarModule, MatCardModule],
providers: [
{ provide: ConfigService, useClass: ConfigServiceStub },
{ provide: UpdateWorkgroupService, useClass: UpdateWorkgroupServiceStub },
{ provide: MatDialog, useValue: {} }
imports: [
BrowserAnimationsModule,
HttpClientTestingModule,
ManageStudentsModule,
MatCardModule,
MatDialogModule,
MatSnackBarModule
],
providers: [ConfigService, UpdateWorkgroupService],
schemas: [NO_ERRORS_SCHEMA]
});
configService = TestBed.inject(ConfigService);
dialog = TestBed.inject(MatDialog);
fixture = TestBed.createComponent(ManageTeamComponent);
http = TestBed.inject(HttpClient);
getPermissionsSpy = spyOn(configService, 'getPermissions');
spyOn(configService, 'getRunId').and.returnValue(1);
spyOnCanGradeStudentWork(true);
component = fixture.componentInstance;
component.team = { workgroupId: 3, users: [{ id: 1 }] };
component.team = {
workgroupId: 10,
users: [{ id: 1, name: studentName, username: studentUsername }]
};
manageTeamHarness = await TestbedHarnessEnvironment.harnessForFixture(
fixture,
ManageTeamHarness
);
});
changePeriodLinkVisible();
changePeriodLinkVisibility();
removeStudent();
});

function changePeriodLinkVisible() {
function spyOnCanGradeStudentWork(canGrade: boolean) {
getPermissionsSpy.and.returnValue({
canGradeStudentWork: canGrade,
canViewStudentNames: true,
canAuthorProject: true
});
}

function changePeriodLinkVisibility() {
describe('change period link', () => {
it('should appear when user has GradeStudentWork permission', () => {
spyOnCanGradeStudentWork(true);
fixture.detectChanges();
expect(getChangePeriodLink()).toBeTruthy();
describe('teacher has GradeStudentWork permission', () => {
it('makes change period link visible', async () => {
expect(await manageTeamHarness.isChangePeriodLinkVisible()).toBeTrue();
});
});
it('should not appear when user does not have GradeStudentWork permission', () => {
spyOnCanGradeStudentWork(false);
fixture.detectChanges();
expect(getChangePeriodLink()).toBeFalsy();
describe('teacher does not have GradeStudentWork permission', () => {
it('makes change period link not visible', async () => {
spyOnCanGradeStudentWork(false);
component.ngOnInit();
expect(await manageTeamHarness.isChangePeriodLinkVisible()).toBeFalse();
});
});
it('should not appear when there are no members', () => {
component.team.users = [];
spyOnCanGradeStudentWork(true);
fixture.detectChanges();
expect(getChangePeriodLink()).toBeFalsy();
describe('team has no members', () => {
it('makes change period link not visible', async () => {
component.team.users = [];
expect(await manageTeamHarness.isChangePeriodLinkVisible()).toBeFalse();
});
});
});
}

function spyOnCanGradeStudentWork(canGrade: boolean) {
spyOn(configService, 'getPermissions').and.returnValue({
canGradeStudentWork: canGrade,
canViewStudentNames: true,
canAuthorProject: true
function removeStudent() {
describe('removeStudent()', () => {
describe('click remove student button on a student', () => {
it('removes student from the team', async () => {
spyOn(dialog, 'open').and.returnValue({
afterClosed: () => of(true)
} as MatDialogRef<typeof RemoveUserConfirmDialogComponent>);
spyOn(http, 'delete').and.returnValue(of({}));
await manageTeamHarness.clickRemoveUser(`${studentName} (${studentUsername})`);
expect(await manageTeamHarness.getMemberCount()).toBe(0);
});
});
});
}

function getChangePeriodLink() {
return fixture.debugElement.query(By.css('.change-period'));
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import { getAvatarColorForWorkgroupId } from '../../../../common/workgroup/workg
})
export class ManageTeamComponent {
avatarColor: string;
canChangePeriod: boolean;
canGradeStudentWork: boolean;
isUnassigned: boolean;
@Input() team: any;

Expand All @@ -35,11 +35,8 @@ export class ManageTeamComponent {

ngOnInit() {
this.avatarColor = getAvatarColorForWorkgroupId(this.team.workgroupId);
this.canGradeStudentWork = this.configService.getPermissions().canGradeStudentWork;
this.isUnassigned = this.team.workgroupId == null;
this.canChangePeriod =
this.configService.getPermissions().canGradeStudentWork &&
this.team.users.length > 0 &&
!this.isUnassigned;
}

changePeriod(event: Event) {
Expand Down Expand Up @@ -113,4 +110,8 @@ export class ManageTeamComponent {
}
});
}

protected removeUser(user: any): void {
this.team.users.splice(this.team.users.indexOf(user), 1);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import { ComponentHarness } from '@angular/cdk/testing';
import { ManageUserHarness } from '../manage-user/manage-user.harness';

export class ManageTeamHarness extends ComponentHarness {
static hostSelector = 'manage-team';
protected getChangePeriodLink = this.locatorForOptional('.change-period');
protected getMembers = this.locatorForAll(ManageUserHarness);

async isChangePeriodLinkVisible(): Promise<boolean> {
return (await this.getChangePeriodLink()) != null;
}

async getMember(username: string): Promise<ManageUserHarness> {
for (const member of await this.getMembers()) {
if ((await member.getUsername()) === username) {
return member;
}
}
return null;
}

async clickRemoveUser(username: string): Promise<void> {
const member = await this.getMember(username);
await member.clickRemoveUserButton();
}

async getMemberCount(): Promise<number> {
return (await this.getMembers()).length;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@
matTooltip="Remove student"
i18n-matTooltip
matTooltipPosition="above"
aria-label="Remove student"
i18n-aria-label
(click)="removeUser($event)"
>
<mat-icon class="text-secondary">clear</mat-icon>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,15 @@ import { BrowserAnimationsModule } from '@angular/platform-browser/animations';
import { ConfigService } from '../../../../services/configService';
import { ManageUserComponent } from './manage-user.component';
import { NO_ERRORS_SCHEMA } from '@angular/core';
import { of } from 'rxjs';

class ConfigServiceStub {
getPermissions() {}
getRunId() {
return 123;
}
retrieveConfig() {
return {};
return of({});
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { HttpClient } from '@angular/common/http';
import { Component, Input, ViewEncapsulation } from '@angular/core';
import { Component, EventEmitter, Input, Output, ViewEncapsulation } from '@angular/core';
import { MatDialog } from '@angular/material/dialog';
import { MatSnackBar } from '@angular/material/snack-bar';
import { ConfigService } from '../../../../services/configService';
Expand All @@ -15,6 +15,7 @@ import { RemoveUserConfirmDialogComponent } from '../remove-user-confirm-dialog/
})
export class ManageUserComponent {
@Input() user: any;
@Output() removeUserEvent: EventEmitter<any> = new EventEmitter<any>();

constructor(
private dialog: MatDialog,
Expand Down Expand Up @@ -49,9 +50,22 @@ export class ManageUserComponent {
performRemoveUser() {
const runId = this.configService.getRunId();
const studentId = this.user.id;
this.http.delete(`/api/teacher/run/${runId}/student/${studentId}/remove`).subscribe(() => {
this.snackBar.open($localize`Removed ${this.user.name} (${this.user.username}) from unit.`);
this.configService.retrieveConfig(`/api/config/classroomMonitor/${runId}`);
this.http.delete(`/api/teacher/run/${runId}/student/${studentId}/remove`).subscribe({
next: () => {
this.removeUserEvent.emit(this.user);
this.configService.retrieveConfig(`/api/config/classroomMonitor/${runId}`).subscribe({
next: () => {
this.snackBar.open(
$localize`Removed ${this.user.name} (${this.user.username}) from unit.`
);
}
});
},
error: () => {
this.snackBar.open(
$localize`Error: Could not remove ${this.user.name} (${this.user.username}) from unit.`
);
}
});
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import { ComponentHarness } from '@angular/cdk/testing';
import { MatButtonHarness } from '@angular/material/button/testing';
import { ShowStudentInfoHarness } from '../show-student-info/show-student-info.harness';

export class ManageUserHarness extends ComponentHarness {
static hostSelector = 'manage-user';
protected getStudentInfo = this.locatorForOptional(ShowStudentInfoHarness);
protected getRemoveUserButton = this.locatorFor(
MatButtonHarness.with({ selector: '[aria-label="Remove student"]' })
);

async clickRemoveUserButton(): Promise<void> {
(await this.getRemoveUserButton()).click();
}

async getUsername(): Promise<string> {
return await (await this.getStudentInfo()).getUsernameText();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { ComponentHarness } from '@angular/cdk/testing';

export class ShowStudentInfoHarness extends ComponentHarness {
static hostSelector = 'show-student-info';
protected getUsername = this.locatorFor('.username');

async getUsernameText(): Promise<string> {
return (await this.getUsername()).text();
}
}
21 changes: 16 additions & 5 deletions src/messages.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -12877,7 +12877,7 @@ Click &quot;Cancel&quot; to keep the invalid JSON open so you can fix it.</sourc
</context-group>
<context-group purpose="location">
<context context-type="sourcefile">src/assets/wise5/classroomMonitor/classroomMonitorComponents/manageStudents/manage-team/manage-team.component.html</context>
<context context-type="linenumber">17</context>
<context context-type="linenumber">22</context>
</context-group>
</trans-unit>
<trans-unit id="f4c42ea42ec642ccfddbf5ba0b28f85f5911b980" datatype="html">
Expand Down Expand Up @@ -12973,21 +12973,21 @@ Click &quot;Cancel&quot; to keep the invalid JSON open so you can fix it.</sourc
<source>No students</source>
<context-group purpose="location">
<context context-type="sourcefile">src/assets/wise5/classroomMonitor/classroomMonitorComponents/manageStudents/manage-team/manage-team.component.html</context>
<context context-type="linenumber">19</context>
<context context-type="linenumber">24</context>
</context-group>
</trans-unit>
<trans-unit id="513458985182639179" datatype="html">
<source>Moved student to Team <x id="PH" equiv-text="workgroupId"/>.</source>
<context-group purpose="location">
<context context-type="sourcefile">src/assets/wise5/classroomMonitor/classroomMonitorComponents/manageStudents/manage-team/manage-team.component.ts</context>
<context context-type="linenumber">107</context>
<context context-type="linenumber">104</context>
</context-group>
</trans-unit>
<trans-unit id="784919496056996013" datatype="html">
<source>Error: Could not move student.</source>
<context-group purpose="location">
<context context-type="sourcefile">src/assets/wise5/classroomMonitor/classroomMonitorComponents/manageStudents/manage-team/manage-team.component.ts</context>
<context context-type="linenumber">112</context>
<context context-type="linenumber">109</context>
</context-group>
</trans-unit>
<trans-unit id="9b83b532c3f74e3d6e25a6ba749eef54324bea48" datatype="html">
Expand Down Expand Up @@ -13017,12 +13017,23 @@ Click &quot;Cancel&quot; to keep the invalid JSON open so you can fix it.</sourc
<context context-type="sourcefile">src/assets/wise5/classroomMonitor/classroomMonitorComponents/manageStudents/manage-user/manage-user.component.html</context>
<context context-type="linenumber">38</context>
</context-group>
<context-group purpose="location">
<context context-type="sourcefile">src/assets/wise5/classroomMonitor/classroomMonitorComponents/manageStudents/manage-user/manage-user.component.html</context>
<context context-type="linenumber">41</context>
</context-group>
</trans-unit>
<trans-unit id="3255676199172660043" datatype="html">
<source>Removed <x id="PH" equiv-text="this.user.name"/> (<x id="PH_1" equiv-text="this.user.username"/>) from unit.</source>
<context-group purpose="location">
<context context-type="sourcefile">src/assets/wise5/classroomMonitor/classroomMonitorComponents/manageStudents/manage-user/manage-user.component.ts</context>
<context context-type="linenumber">53</context>
<context context-type="linenumber">59</context>
</context-group>
</trans-unit>
<trans-unit id="213219330219823977" datatype="html">
<source>Error: Could not remove <x id="PH" equiv-text="this.user.name"/> (<x id="PH_1" equiv-text="this.user.username"/>) from unit.</source>
<context-group purpose="location">
<context context-type="sourcefile">src/assets/wise5/classroomMonitor/classroomMonitorComponents/manageStudents/manage-user/manage-user.component.ts</context>
<context context-type="linenumber">66</context>
</context-group>
</trans-unit>
<trans-unit id="8e728195d4eb16b80063b74eecea8f498762d1ef" datatype="html">
Expand Down

0 comments on commit 06336ba

Please sign in to comment.