Skip to content

Commit

Permalink
feat(FeedbackRule): isLowestWorkgroupIdInPeerGroup token (#1420)
Browse files Browse the repository at this point in the history
Also add BooleanTermEvaluator to evaluate "true" and "false" tokens that are added on the stack after evaluating other tokens
  • Loading branch information
hirokiterashima authored Sep 20, 2023
1 parent e9bba09 commit c226806
Show file tree
Hide file tree
Showing 21 changed files with 224 additions and 50 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ describe('FeedbackRuleEvaluator', () => {
beforeEach(() => {
evaluator = new FeedbackRuleEvaluator(
new FeedbackRuleComponent(DEFAULT_FEEDBACK_RULES, 5, true),
null,
null
);
});
Expand Down Expand Up @@ -78,6 +79,7 @@ function matchRule_hasKIScore() {
beforeEach(() => {
evaluator = new FeedbackRuleEvaluator(
new FeedbackRuleComponent(HAS_KI_SCORE_FEEDBACK_RULES, 5, true),
null,
null
);
});
Expand Down Expand Up @@ -123,6 +125,7 @@ function matchRule_ideaCount() {
];
evaluator = new FeedbackRuleEvaluator(
new FeedbackRuleComponent(feedbackRules, 5, true),
null,
null
);
});
Expand All @@ -147,7 +150,7 @@ function matchNoRule_ReturnDefault() {
function matchNoRule_NoDefaultFeedbackAuthored_ReturnApplicationDefault() {
it(`should return application default rule when no rule is matched and no default is
authored`, () => {
evaluator = new FeedbackRuleEvaluator(new FeedbackRuleComponent([], 5, true), null);
evaluator = new FeedbackRuleEvaluator(new FeedbackRuleComponent([], 5, true), null, null);
expectFeedback(['idea10', 'idea11'], [KI_SCORE_1], 1, evaluator.defaultFeedback);
});
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { Component } from '../../../common/Component';
import { ConfigService } from '../../../services/configService';
import { ConstraintService } from '../../../services/constraintService';
import { FeedbackRuleComponent } from '../../feedbackRule/FeedbackRuleComponent';
import { PeerGroup } from '../../peerChat/PeerGroup';
import { CRaterResponse } from '../cRater/CRaterResponse';
import { FeedbackRule } from './FeedbackRule';
import { FeedbackRuleExpression } from './FeedbackRuleExpression';
Expand All @@ -12,12 +14,14 @@ export class FeedbackRuleEvaluator<T extends Response[]> {
defaultFeedback = $localize`Thanks for submitting your response.`;
protected factory;
protected referenceComponent: Component;
protected peerGroup: PeerGroup;

constructor(
protected component: FeedbackRuleComponent,
protected configService: ConfigService,
protected constraintService: ConstraintService
) {
this.factory = new TermEvaluatorFactory(constraintService);
this.factory = new TermEvaluatorFactory(configService, constraintService);
}

getFeedbackRule(responses: T): FeedbackRule {
Expand Down Expand Up @@ -142,6 +146,10 @@ export class FeedbackRuleEvaluator<T extends Response[]> {
);
}

setPeerGroup(peerGroup: PeerGroup): void {
this.peerGroup = peerGroup;
}

setReferenceComponent(referenceComponent: Component): void {
this.referenceComponent = referenceComponent;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ describe('FeedbackRuleEvaluatorMultipleStudents', () => {
beforeEach(() => {
evaluator = new FeedbackRuleEvaluatorMultipleStudents(
new FeedbackRuleComponent(DEFAULT_FEEDBACK_RULES, 5, true),
null,
null
);
});
Expand All @@ -40,6 +41,7 @@ function matchRules_HasKIScore() {
beforeEach(() => {
evaluator = new FeedbackRuleEvaluatorMultipleStudents(
new FeedbackRuleComponent(HAS_KI_SCORE_FEEDBACK_RULES, 5, true),
null,
null
);
});
Expand Down Expand Up @@ -73,6 +75,7 @@ function matchNoRule_NoDefaultFeedbackAuthored_ReturnApplicationDefault() {
authored`, () => {
evaluator = new FeedbackRuleEvaluatorMultipleStudents(
new FeedbackRuleComponent([], 5, true),
null,
null
);
expectRules([createCRaterResponse(['idea10', 'idea11'], [KI_SCORE_1], 1)], ['default']);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ export class FeedbackRuleEvaluatorMultipleStudents extends FeedbackRuleEvaluator

protected evaluateTerm(term: string, responses: Response[]): boolean {
const evaluator: TermEvaluator = this.factory.getTermEvaluator(term);
evaluator.setPeerGroup(this.peerGroup);
evaluator.setReferenceComponent(this.referenceComponent);
return responses.some((response: Response) => {
return evaluator.evaluate(response);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { Response } from '../Response';
import { BooleanTermEvaluator } from './BooleanTermEvaluator';

describe('BooleanTermEvaluator', () => {
describe('evaluate()', () => {
testEvaluate('true', true);
testEvaluate('false', false);
});
});

function testEvaluate(term: string, expectedResult: boolean) {
describe(`term is "${term}"`, () => {
let evaluator: BooleanTermEvaluator;
beforeEach(() => {
evaluator = new BooleanTermEvaluator(term);
});
it(`returns ${expectedResult}`, () => {
expect(evaluator.evaluate(new Response())).toBe(expectedResult);
});
});
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import { Response } from '../Response';
import { TermEvaluator } from './TermEvaluator';

export class BooleanTermEvaluator extends TermEvaluator {
constructor(protected term: string) {
super(term);
}

evaluate(response: Response): boolean {
/**
* The term 'true' or 'false' is usually not authored, but is created and added to the termStack
* by the application as it processes operator expressions like "&&", "||", and "!"
* (see FeedbackRuleEvaluator.evaluateOperator()).
* For example, if the student had ideas 1, 2 and 3, evaluating the postfix expression
* ["1", "2", "&&", "3", "&&"] goes like this:
* ["1", "2", "&&", "3", "&&"] => ["true", "3", "&&"] => ["true"] => true.
*/
return this.term === 'true';
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,24 @@ import { CRaterResponse } from '../../cRater/CRaterResponse';
import { IdeaTermEvaluator } from './IdeaTermEvaluator';

describe('IdeaTermEvaluator', () => {
describe('evaluate()', () => {
const response_with_no_ideas = new CRaterResponse();
const response_with_2_ideas = new CRaterResponse();
response_with_2_ideas.ideas = [new CRaterIdea('1', true), new CRaterIdea('2', true)];
it("should always return true if term is 'true'", () => {
const evaluator_true = new IdeaTermEvaluator('true');
expect(evaluator_true.evaluate(response_with_no_ideas)).toBeTrue();
expect(evaluator_true.evaluate(response_with_2_ideas)).toBeTrue();
const evaluator = new IdeaTermEvaluator('1');
describe('evaluate(response)', () => {
let response: CRaterResponse;
beforeEach(() => {
response = new CRaterResponse();
});
it('should return true iff idea term is found in the response', () => {
const evaluator_idea1 = new IdeaTermEvaluator('1');
expect(evaluator_idea1.evaluate(response_with_no_ideas)).toBeFalse();
expect(evaluator_idea1.evaluate(response_with_2_ideas)).toBeTrue();
describe('response does not contain the idea', () => {
it('returns false', () => {
expect(evaluator.evaluate(response)).toBeFalse();
});
});
describe('response contains the idea', () => {
beforeEach(() => {
response.ideas = [new CRaterIdea('1', true), new CRaterIdea('2', true)];
});
it('returns true', () => {
expect(evaluator.evaluate(response)).toBeTrue();
});
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,6 @@ export class IdeaTermEvaluator extends TermEvaluator {
}

evaluate(response: CRaterResponse): boolean {
/**
* The term 'true' or 'false' is usually not authored, but is created and added to the termStack
* by the application as it processes operator expressions like "&&", "||", and "!"
* (see FeedbackRuleEvaluator.evaluateOperator()).
* For example, if the student had ideas 1, 2 and 3, evaluating the postfix expression
* ["1", "2", "&&", "3", "&&"] goes like this:
* ["1", "2", "&&", "3", "&&"] => ["true", "3", "&&"] => ["true"] => true.
*/
return this.term === 'true' || response.getDetectedIdeaNames().includes(this.term);
return response.getDetectedIdeaNames().includes(this.term);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import { PeerGrouping } from '../../../../../../app/domain/peerGrouping';
import { PeerGroup } from '../../../peerChat/PeerGroup';
import { PeerGroupMember } from '../../../peerChat/PeerGroupMember';
import { CRaterResponse } from '../../cRater/CRaterResponse';
import { IsLowestWorkgroupIdInPeerGroupTermEvaluator } from './IsLowestWorkgroupIdInPeerGroupTermEvaluator';

class ConfigService {
getWorkgroupId() {
return 1;
}
}

describe('IsLowestWorkgroupIdInPeerGroupTermEvaluator', () => {
let evaluator, mockConfigService;
beforeEach(() => {
evaluator = new IsLowestWorkgroupIdInPeerGroupTermEvaluator('isLowestWorkgroupIdInPeerGroup');
mockConfigService = new ConfigService();
evaluator.setConfigService(mockConfigService);
evaluator.setPeerGroup(
new PeerGroup(1, [{ id: 1 }, { id: 2 }] as PeerGroupMember[], new PeerGrouping())
);
});
describe('evaluate()', () => {
[
{
description: 'your workgroup id is the lowest',
myWorkgroupId: 1,
expected: true
},
{
description: 'your workgroup id is not the lowest',
myWorkgroupId: 2,
expected: false
}
].forEach(({ description, myWorkgroupId, expected }) => {
describe(description, () => {
beforeEach(() => {
spyOn(mockConfigService, 'getWorkgroupId').and.returnValue(myWorkgroupId);
});
it(`returns ${expected}`, () => {
expect(evaluator.evaluate(new CRaterResponse())).toEqual(expected);
});
});
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { Response } from '../Response';
import { TermEvaluator } from './TermEvaluator';

export class IsLowestWorkgroupIdInPeerGroupTermEvaluator extends TermEvaluator {
evaluate(response: Response | Response[]): boolean {
return this.peerGroup.getWorkgroupIds().sort().at(0) === this.configService.getWorkgroupId();
}
}
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
import { Component } from '../../../../common/Component';
import { ConfigService } from '../../../../services/configService';
import { ConstraintService } from '../../../../services/constraintService';
import { PeerGroup } from '../../../peerChat/PeerGroup';
import { Response } from '../Response';

export abstract class TermEvaluator {
protected referenceComponent: Component;
protected configService: ConfigService;
protected constraintService: ConstraintService;
protected peerGroup: PeerGroup;
protected referenceComponent: Component;

constructor(protected term: string) {}
abstract evaluate(response: Response | Response[]): boolean;
Expand All @@ -13,6 +17,14 @@ export abstract class TermEvaluator {
return /accumulatedIdeaCount(MoreThan|Equals|LessThan)\([\d+]\)/.test(term);
}

static isBooleanTerm(term: string): boolean {
return ['true', 'false'].includes(term);
}

static isLowestWorkgroupIdInPeerGroupTerm(term: string): boolean {
return term === 'isLowestWorkgroupIdInPeerGroup';
}

static isMyChoiceChosenTerm(term: string): boolean {
return /myChoiceChosen\("\w+"\)/.test(term);
}
Expand Down Expand Up @@ -40,10 +52,18 @@ export abstract class TermEvaluator {
);
}

setConfigService(service: ConfigService): void {
this.configService = service;
}

setConstraintService(service: ConstraintService): void {
this.constraintService = service;
}

setPeerGroup(peerGroup: PeerGroup): void {
this.peerGroup = peerGroup;
}

setReferenceComponent(referenceComponent: Component): void {
this.referenceComponent = referenceComponent;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,25 +4,36 @@ import { IdeaCountTermEvaluator } from './IdeaCountTermEvaluator';
import { IdeaTermEvaluator } from './IdeaTermEvaluator';
import { IsSubmitNumberEvaluator } from './IsSubmitNumberEvaluator';
import { TermEvaluatorFactory } from './TermEvaluatorFactory';
import { AccumulatedIdeaCountTermEvaluator } from './AccumulatedIdeaCountTermEvaluator';
import { IsLowestWorkgroupIdInPeerGroupTermEvaluator } from './IsLowestWorkgroupIdInPeerGroupTermEvaluator';
import { BooleanTermEvaluator } from './BooleanTermEvaluator';

describe('TermEvaluatorFactory', () => {
const factory = new TermEvaluatorFactory(null);
describe('getTermEvaluator()', () => {
it('should return correct evaluator', () => {
[
{
term: 'myChoiceChosen("choice1")',
instanceType: MyChoiceChosenTermEvaluator
},
{ term: 'hasKIScore(3)', instanceType: HasKIScoreTermEvaluator },
{ term: 'ideaCountMoreThan(1)', instanceType: IdeaCountTermEvaluator },
{ term: 'ideaCountEquals(3)', instanceType: IdeaCountTermEvaluator },
{ term: 'ideaCountLessThan(2)', instanceType: IdeaCountTermEvaluator },
{ term: 'isSubmitNumber(2)', instanceType: IsSubmitNumberEvaluator },
{ term: 'isSubmitNumber(23)', instanceType: IsSubmitNumberEvaluator },
{ term: '2', instanceType: IdeaTermEvaluator }
].forEach(({ term, instanceType }) => {
expect(factory.getTermEvaluator(term) instanceof instanceType).toBeTrue();
const factory = new TermEvaluatorFactory(null, null);
describe('getTermEvaluator(term)', () => {
[
{ term: 'true', evaluator: BooleanTermEvaluator },
{ term: 'false', evaluator: BooleanTermEvaluator },
{ term: 'hasKIScore(3)', evaluator: HasKIScoreTermEvaluator },
{ term: 'ideaCountMoreThan(1)', evaluator: IdeaCountTermEvaluator },
{ term: 'ideaCountEquals(3)', evaluator: IdeaCountTermEvaluator },
{ term: 'ideaCountLessThan(2)', evaluator: IdeaCountTermEvaluator },
{ term: 'isSubmitNumber(2)', evaluator: IsSubmitNumberEvaluator },
{ term: 'isSubmitNumber(23)', evaluator: IsSubmitNumberEvaluator },
{ term: 'accumulatedIdeaCountMoreThan(1)', evaluator: AccumulatedIdeaCountTermEvaluator },
{ term: 'accumulatedIdeaCountEquals(3)', evaluator: AccumulatedIdeaCountTermEvaluator },
{ term: 'accumulatedIdeaCountLessThan(2)', evaluator: AccumulatedIdeaCountTermEvaluator },
{ term: 'myChoiceChosen("1")', evaluator: MyChoiceChosenTermEvaluator },
{
term: 'isLowestWorkgroupIdInPeerGroup',
evaluator: IsLowestWorkgroupIdInPeerGroupTermEvaluator
},
{ term: '2', evaluator: IdeaTermEvaluator }
].forEach(({ term, evaluator }) => {
describe(`term is ${term}`, () => {
it(`returns ${evaluator.name}`, () => {
expect(factory.getTermEvaluator(term) instanceof evaluator).toBeTrue();
});
});
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,18 @@ import { IdeaCountWithResponseIndexTermEvaluator } from './IdeaCountWithResponse
import { IdeaTermEvaluator } from './IdeaTermEvaluator';
import { IsSubmitNumberEvaluator } from './IsSubmitNumberEvaluator';
import { TermEvaluator } from './TermEvaluator';
import { ConfigService } from '../../../../services/configService';
import { IsLowestWorkgroupIdInPeerGroupTermEvaluator } from './IsLowestWorkgroupIdInPeerGroupTermEvaluator';
import { BooleanTermEvaluator } from './BooleanTermEvaluator';

export class TermEvaluatorFactory {
constructor(private constraintService: ConstraintService) {}
constructor(private configService: ConfigService, private constraintService: ConstraintService) {}

getTermEvaluator(term: string): TermEvaluator {
let evaluator: TermEvaluator;
if (TermEvaluator.isHasKIScoreTerm(term)) {
if (TermEvaluator.isBooleanTerm(term)) {
evaluator = new BooleanTermEvaluator(term);
} else if (TermEvaluator.isHasKIScoreTerm(term)) {
evaluator = new HasKIScoreTermEvaluator(term);
} else if (TermEvaluator.isIdeaCountTerm(term)) {
evaluator = new IdeaCountTermEvaluator(term);
Expand All @@ -25,9 +30,12 @@ export class TermEvaluatorFactory {
evaluator = new AccumulatedIdeaCountTermEvaluator(term);
} else if (TermEvaluator.isMyChoiceChosenTerm(term)) {
evaluator = new MyChoiceChosenTermEvaluator(term);
} else if (TermEvaluator.isLowestWorkgroupIdInPeerGroupTerm(term)) {
evaluator = new IsLowestWorkgroupIdInPeerGroupTermEvaluator(term);
} else {
evaluator = new IdeaTermEvaluator(term);
}
evaluator.setConfigService(this.configService);
evaluator.setConstraintService(this.constraintService);
return evaluator;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ export class DialogGuidanceStudentComponent extends ComponentStudent {
this.getMaxSubmitCount(),
this.component.isMultipleFeedbackTextsForSameRuleAllowed()
),
this.configService,
this.constraintService
);
if (this.component.isComputerAvatarEnabled()) {
Expand Down
Loading

0 comments on commit c226806

Please sign in to comment.