Skip to content

Commit

Permalink
refactor(TeacherProjectService): Move lock node functions to NavItemC…
Browse files Browse the repository at this point in the history
…omponent (#1922)
  • Loading branch information
hirokiterashima authored Aug 29, 2024
1 parent 5fcf338 commit 6b1b0d5
Show file tree
Hide file tree
Showing 6 changed files with 116 additions and 97 deletions.
34 changes: 0 additions & 34 deletions src/app/services/teacherProjectService.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,6 @@ describe('TeacherProjectService', () => {
testGetNodeIdAfter();
testCreateNodeAfter();
shouldGetTheBranchLetter();
lockNode();
unlockNode();
getNextAvailableNodeId();
shouldReturnTheNextAvailableGroupId();
shouldReturnTheGroupIdsInTheProject();
Expand Down Expand Up @@ -196,38 +194,6 @@ function shouldGetTheBranchLetter() {
});
}

function lockNode() {
describe('lockNode()', () => {
it('should add teacherRemoval constraint to node', () => {
service.setProject(teacherProjectJSON);
const group1 = service.getNodeById('group1');
const periodIdToLock = 123;
expect(group1.constraints).toBeUndefined();
service.addTeacherRemovalConstraint(group1, periodIdToLock);
expect(group1.constraints.length).toEqual(1);
const contraintRemovalCriteria = group1.constraints[0].removalCriteria[0];
expect(contraintRemovalCriteria.name).toEqual('teacherRemoval');
expect(contraintRemovalCriteria.params.periodId).toEqual(periodIdToLock);
});
});
}

function unlockNode() {
describe('unlockNode()', () => {
it('should remove teacherRemoval constraint from node', () => {
service.setProject(teacherProjectJSON);
const node6 = service.getNodeById('node6');
expect(node6.constraints.length).toEqual(2);
service.removeTeacherRemovalConstraint(node6, 123);
expect(node6.constraints.length).toEqual(1);
service.removeTeacherRemovalConstraint(node6, 124);
expect(node6.constraints.length).toEqual(0);
service.removeTeacherRemovalConstraint(node6, 999);
expect(node6.constraints.length).toEqual(0);
});
});
}

function getNextAvailableNodeId() {
describe('getNextAvailableNodeId', () => {
it('should return the next available node id', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { ClassroomMonitorTestingModule } from '../../../classroom-monitor-testin
import { NavItemComponent } from './nav-item.component';
import { NodeService } from '../../../../services/nodeService';
import { Node } from '../../../../common/Node';
import { NO_ERRORS_SCHEMA } from '@angular/core';

class MockNotificationService {
getAlertNotifications() {
Expand Down Expand Up @@ -43,12 +44,14 @@ class MockTeacherProjectService {
return new Node();
}
getParentGroup() {}
saveProject() {}
}

let component: NavItemComponent;
let fixture: ComponentFixture<NavItemComponent>;
const nodeId = 'node1';
const periodId = 1;
let node1;

describe('NavItemComponent', () => {
beforeEach(async () => {
Expand All @@ -60,7 +63,8 @@ describe('NavItemComponent', () => {
{ provide: NotificationService, useClass: MockNotificationService },
{ provide: TeacherDataService, useClass: MockTeacherDataService },
{ provide: TeacherProjectService, useClass: MockTeacherProjectService }
]
],
schemas: [NO_ERRORS_SCHEMA]
}).compileComponents();
});

Expand All @@ -72,6 +76,7 @@ describe('NavItemComponent', () => {
});

itemClicked();
toggleLockNode();
});

function itemClicked() {
Expand All @@ -98,3 +103,55 @@ function itemClicked() {
});
});
}

function toggleLockNode() {
describe('toggleLockNode()', () => {
let projectService: TeacherProjectService;
let lockNodeButton;
let saveSpy;
beforeEach(() => {
node1 = new Node();
node1.id = nodeId;
component.isGroup = true;
component.type = 'card';
fixture.detectChanges();
projectService = TestBed.inject(TeacherProjectService);
saveSpy = spyOn(projectService, 'saveProject').and.returnValue(new Promise(() => {}));
lockNodeButton = fixture.debugElement.nativeElement.querySelector('button');
});
describe('when there is no teacherRemovalConstraint', () => {
it('should add constraint', () => {
const getNodeSpy = spyOn(projectService, 'getNode').and.returnValue(node1);
expect(node1.constraints.length).toEqual(0);
lockNodeButton.click();
expect(getNodeSpy).toHaveBeenCalled();
expect(saveSpy).toHaveBeenCalled();
expect(node1.constraints.length).toEqual(1);
expect(node1.constraints[0].action).toEqual('makeThisNodeNotVisitable');
});
});
describe('when there is teacherRemovalConstraint', () => {
it('should remove constraint', () => {
node1.addConstraint({
id: 'ksav10btkr',
action: 'makeThisNodeNotVisitable',
targetId: nodeId,
removalConditional: 'any',
removalCriteria: [
Object({ name: 'teacherRemoval', params: Object({ periodId: periodId }) })
]
});
const getNodeSpy = spyOn(projectService, 'getNode').and.returnValue(node1);
const dataService = TestBed.inject(TeacherDataService);
const currentPeriodSpy = spyOn(dataService, 'getCurrentPeriod').and.returnValue({
periodId: periodId
});
lockNodeButton.click();
expect(getNodeSpy).toHaveBeenCalled();
expect(saveSpy).toHaveBeenCalled();
expect(currentPeriodSpy).toHaveBeenCalled();
expect(node1.constraints.length).toEqual(0);
});
});
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import { TeacherDataService } from '../../../../services/teacherDataService';
import { TeacherProjectService } from '../../../../services/teacherProjectService';
import { TeacherWebSocketService } from '../../../../services/teacherWebSocketService';
import { NodeService } from '../../../../services/nodeService';
import { generateRandomKey } from '../../../../common/string/string';
import { Node } from '../../../../common/Node';

@Component({
selector: 'nav-item',
Expand Down Expand Up @@ -185,16 +187,12 @@ export class NavItemComponent implements OnInit {
}

isLocked(): boolean {
const constraints = this.projectService.getNodeById(this.nodeId).constraints;
if (constraints == null) {
return false;
} else {
return (
(this.isShowingAllPeriods() && this.isLockedForAll(constraints)) ||
(!this.isShowingAllPeriods() &&
this.isLockedForPeriod(constraints, this.dataService.getCurrentPeriod().periodId))
);
}
const constraints = this.projectService.getNode(this.nodeId).constraints;
return (
(this.isShowingAllPeriods() && this.isLockedForAll(constraints)) ||
(!this.isShowingAllPeriods() &&
this.isLockedForPeriod(constraints, this.dataService.getCurrentPeriod().periodId))
);
}

private isLockedForAll(constraints: any): boolean {
Expand All @@ -220,7 +218,7 @@ export class NavItemComponent implements OnInit {
}

protected toggleLockNode(): void {
const node = this.projectService.getNodeById(this.nodeId);
const node = this.projectService.getNode(this.nodeId);
const isLocked = this.isLocked();
if (isLocked) {
this.unlockNode(node);
Expand All @@ -241,38 +239,61 @@ export class NavItemComponent implements OnInit {
);
}

private unlockNode(node: any): void {
private unlockNode(node: Node): void {
if (this.isShowingAllPeriods()) {
this.unlockNodeForAllPeriods(node);
} else {
this.projectService.removeTeacherRemovalConstraint(
node,
this.dataService.getCurrentPeriod().periodId
);
this.removeTeacherRemovalConstraint(node, this.dataService.getCurrentPeriod().periodId);
}
}

private lockNode(node: any): void {
private lockNode(node: Node): void {
if (this.isShowingAllPeriods()) {
this.lockNodeForAllPeriods(node);
} else {
this.projectService.addTeacherRemovalConstraint(
node,
this.dataService.getCurrentPeriod().periodId
);
this.addTeacherRemovalConstraint(node, this.dataService.getCurrentPeriod().periodId);
}
}

private unlockNodeForAllPeriods(node: any): void {
private addTeacherRemovalConstraint(node: Node, periodId: number): void {
const lockConstraint = {
id: generateRandomKey(),
action: 'makeThisNodeNotVisitable',
targetId: node.id,
removalConditional: 'any',
removalCriteria: [
{
name: 'teacherRemoval',
params: {
periodId: periodId
}
}
]
};
node.addConstraint(lockConstraint);
}

private unlockNodeForAllPeriods(node: Node): void {
for (const period of this.dataService.getPeriods()) {
this.projectService.removeTeacherRemovalConstraint(node, period.periodId);
this.removeTeacherRemovalConstraint(node, period.periodId);
}
}

private lockNodeForAllPeriods(node: any): void {
private removeTeacherRemovalConstraint(node: Node, periodId: number) {
node.constraints = node.constraints.filter((constraint) => {
return !(
constraint.action === 'makeThisNodeNotVisitable' &&
constraint.targetId === node.id &&
constraint.removalCriteria[0].name === 'teacherRemoval' &&
constraint.removalCriteria[0].params.periodId === periodId
);
});
}

private lockNodeForAllPeriods(node: Node): void {
for (const period of this.dataService.getPeriods()) {
if (period.periodId !== -1 && !this.isLockedForPeriod(node.constraints, period.periodId)) {
this.projectService.addTeacherRemovalConstraint(node, period.periodId);
this.addTeacherRemovalConstraint(node, period.periodId);
}
}
}
Expand Down
4 changes: 4 additions & 0 deletions src/assets/wise5/common/Node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,10 @@ export class Node {
);
}

addConstraint(constraint: any): void {
this.constraints.push(constraint);
}

getConstraints(): any[] {
return this.constraints;
}
Expand Down
29 changes: 0 additions & 29 deletions src/assets/wise5/services/teacherProjectService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -517,35 +517,6 @@ export class TeacherProjectService extends ProjectService {
this.refreshProjectSource.next();
}

addTeacherRemovalConstraint(node: any, periodId: number) {
const lockConstraint = {
id: generateRandomKey(),
action: 'makeThisNodeNotVisitable',
targetId: node.id,
removalConditional: 'any',
removalCriteria: [
{
name: 'teacherRemoval',
params: {
periodId: periodId
}
}
]
};
this.addConstraintToNode(node, lockConstraint);
}

removeTeacherRemovalConstraint(node: any, periodId: number) {
node.constraints = node.constraints.filter((constraint) => {
return !(
constraint.action === 'makeThisNodeNotVisitable' &&
constraint.targetId === node.id &&
constraint.removalCriteria[0].name === 'teacherRemoval' &&
constraint.removalCriteria[0].params.periodId === periodId
);
});
}

/**
* Saves the project to Config.saveProjectURL and returns commit history promise.
* if Config.saveProjectURL or Config.projectId are undefined, does not save and returns null
Expand Down
16 changes: 8 additions & 8 deletions src/messages.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -1808,7 +1808,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/nodeProgress/nav-item/nav-item.component.ts</context>
<context context-type="linenumber">80</context>
<context context-type="linenumber">82</context>
</context-group>
</trans-unit>
<trans-unit id="1217086124949795770" datatype="html">
Expand All @@ -1830,7 +1830,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/nodeProgress/nav-item/nav-item.component.ts</context>
<context context-type="linenumber">83</context>
<context context-type="linenumber">85</context>
</context-group>
</trans-unit>
<trans-unit id="88936072a55699e997d6cc9e47305cdbb26ad42a" datatype="html">
Expand Down Expand Up @@ -14204,21 +14204,21 @@ Click &quot;Cancel&quot; to keep the invalid JSON open so you can fix it.</sourc
<source><x id="PH" equiv-text="this.nodeTitle"/> has been locked for <x id="PH_1" equiv-text="this.getPeriodLabel()"/>.</source>
<context-group purpose="location">
<context context-type="sourcefile">src/assets/wise5/classroomMonitor/classroomMonitorComponents/nodeProgress/nav-item/nav-item.component.ts</context>
<context context-type="linenumber">239</context>
<context context-type="linenumber">237</context>
</context-group>
</trans-unit>
<trans-unit id="7788246130452063015" datatype="html">
<source><x id="PH" equiv-text="this.nodeTitle"/> has been unlocked for <x id="PH_1" equiv-text="this.getPeriodLabel()"/>.</source>
<context-group purpose="location">
<context context-type="sourcefile">src/assets/wise5/classroomMonitor/classroomMonitorComponents/nodeProgress/nav-item/nav-item.component.ts</context>
<context context-type="linenumber">240</context>
<context context-type="linenumber">238</context>
</context-group>
</trans-unit>
<trans-unit id="1775311053456611668" datatype="html">
<source>All Periods</source>
<context-group purpose="location">
<context context-type="sourcefile">src/assets/wise5/classroomMonitor/classroomMonitorComponents/nodeProgress/nav-item/nav-item.component.ts</context>
<context context-type="linenumber">350</context>
<context context-type="linenumber">371</context>
</context-group>
<context-group purpose="location">
<context context-type="sourcefile">src/assets/wise5/classroomMonitor/classroomMonitorComponents/select-period/select-period.component.ts</context>
Expand All @@ -14233,7 +14233,7 @@ Click &quot;Cancel&quot; to keep the invalid JSON open so you can fix it.</sourc
<source>Period: <x id="PH" equiv-text="this.currentPeriod.periodName"/></source>
<context-group purpose="location">
<context context-type="sourcefile">src/assets/wise5/classroomMonitor/classroomMonitorComponents/nodeProgress/nav-item/nav-item.component.ts</context>
<context context-type="linenumber">351</context>
<context context-type="linenumber">372</context>
</context-group>
<context-group purpose="location">
<context context-type="sourcefile">src/assets/wise5/classroomMonitor/classroomMonitorComponents/select-period/select-period.component.ts</context>
Expand All @@ -14244,14 +14244,14 @@ Click &quot;Cancel&quot; to keep the invalid JSON open so you can fix it.</sourc
<source>Unlock for <x id="PH" equiv-text="this.getPeriodLabel()"/></source>
<context-group purpose="location">
<context context-type="sourcefile">src/assets/wise5/classroomMonitor/classroomMonitorComponents/nodeProgress/nav-item/nav-item.component.ts</context>
<context context-type="linenumber">356</context>
<context context-type="linenumber">377</context>
</context-group>
</trans-unit>
<trans-unit id="2330813372531032088" datatype="html">
<source>Lock for <x id="PH" equiv-text="this.getPeriodLabel()"/></source>
<context-group purpose="location">
<context context-type="sourcefile">src/assets/wise5/classroomMonitor/classroomMonitorComponents/nodeProgress/nav-item/nav-item.component.ts</context>
<context context-type="linenumber">357</context>
<context context-type="linenumber">378</context>
</context-group>
</trans-unit>
<trans-unit id="5411ee1bc7179f254f2db73cf448e995fca45f43" datatype="html">
Expand Down

0 comments on commit 6b1b0d5

Please sign in to comment.