feat(Feedback Rule): New myChoiceChosen token #1410

hirokiterashima · 2023-09-08T23:44:13Z

Changes

Add support for myChoiceChosen("choiceId") token that returns true when the student chose the specified choice as their last response for the reference component.

Test

Test with feat(PeerGroupWork): Return StudentWorkAnnotation for MC Reference Component WISE-API#235
Add myChoiceChosen("choiceId") in the FeedbackRule expression for Dynamic Prompt, Question Bank and make sure that the rule is matched if the student chose the specified choice for the reference component. If should not match otherwise.
Existing Peer Chat steps with Dynamic Prompt, Question Bank work as before

Closes #1409

#1409

geoffreykwan

The choseChoice rules do not work in Peer Chat Dynamic Prompt and Question Bank if the Reference Component is set to the Multiple Choice component. It only works if the Reference Component is set to an Open Response CRater component.
Ideally you should be able to set the Reference Component to the Multiple Choice component and then only need to specify the choice id in the expression like choseChoice("mdfmfybcuw").
You can't choose Multiple Choice components in the Reference Component. In the Component drop down, Multiple Choice components are greyed out and can't be selected.

DynamicPrompt and QuestionBank, which use ReferenceComponent to look up work for the component #1409

hirokiterashima · 2023-09-13T19:20:53Z

@geoffreykwan I addressed those issues. myChoiceChosen("choiceId") token now only works in QuestionBank and DynamicPrompt, and it looks at the ReferenceComponent's data to evaluate. PTAL

geoffreykwan

When I test the Peer Chat in preview the questions in the question bank show up properly but when I test with an actual run, the questions don't show up and this error is thrown in the browser console.

ERROR TypeError: Cannot read properties of undefined (reading 'length')
    at PeerChatQuestionBankComponent.filterQuestions (peer-chat-question-bank.component.ts:105:38)
    at PeerChatQuestionBankComponent.chooseQuestionBankRulesToDisplay (peer-chat-question-bank.component.ts:87:17)
    at Object.next (peer-chat-question-bank.component.ts:57:38)

It looks like in peer-chat-question-bank.component.ts line 71 the peerGroupStudentData is an empty array even though both of my students have submitted the Multiple Choice. In preview, it retrieves the student data locally but in a real run it retrieves the work from the API and it looks like the API is returning an empty array.

geoffreykwan · 2023-09-13T21:16:44Z

src/assets/wise5/components/common/cRater/CRaterResponse.ts

-      }
-    }
+    super(jsonObject);
+    this.populateFields(jsonObject);


Is this call to populateFields() necessary if the parent also calls it?

When we run the unit tests, because we initialize the ideas variable to an empty array ideas: CRaterIdea[] = []; in the class declaration, the webpack compiler (I think) adds this line below the line super(jsonObject) this.ideas = [];. If we don't call this.populateFields(jsonObject) again, the ideas fields is always an empty array, and causes the tests to fail. So in effect, this line is there just to make the tests pass, which is not ideal.

This problem goes away if we don't initialize the ideas variable to an empty array, but this may cause side-effects with the actual behavior of the program.

Seems like somewhat unexpected behavior but is good to know for the future.

geoffreykwan · 2023-09-13T21:17:37Z

src/assets/wise5/components/common/feedbackRule/TermEvaluator/test-utils.ts

@@ -5,7 +5,7 @@ import { AbstractIdeaCountTermEvaluator } from './AbstractIdeaCountTermEvaluator
 const idea1 = new CRaterIdea('1', true);
 const idea2 = new CRaterIdea('2', true);
 const idea3 = new CRaterIdea('3', true);
-const response_with_idea_1 = new CRaterResponse({ ideas: [idea1] });
+const response_with_idea_1 = new CRaterResponse({ ideas: [idea1], ba: 1 });


Is the ba: 1 needed here? I can't tell what it's for.

I was using this for testing something. Removed.

geoffreykwan · 2023-09-13T21:23:36Z

src/assets/wise5/components/common/feedbackRule/FeedbackRuleExpression.ts

@@ -28,7 +28,7 @@ export class FeedbackRuleExpression {
    return this.text
      .replace(/ /g, '')
      .split(
-        /(hasKIScore\(\d\)|accumulatedIdeaCountEquals\(\d\)|accumulatedIdeaCountLessThan\(\d\)|accumulatedIdeaCountMoreThan\(\d\)|ideaCountEquals\(\S+\)|ideaCountLessThan\(\S+\)|ideaCountMoreThan\(\S+\)|isSubmitNumber\(\d+\)|&&|\|\||!|\(|\))/g
+        /(hasKIScore\(\d\)|accumulatedIdeaCountEquals\(\d\)|accumulatedIdeaCountLessThan\(\d\)|accumulatedIdeaCountMoreThan\(\d\)|ideaCountEquals\(\S+\)|ideaCountLessThan\(\S+\)|ideaCountMoreThan\(\S+\)|myChoiceChosen\(\S+\)|isSubmitNumber\(\d+\)|&&|\|\||!|\(|\))/g


Can you split this up into multiple lines to make it more readable?

I spent 30 minutes on this but couldn't figure it out. Can you? Or, in the interest of moving forward, let's save this for another time?

You could try something like this but we can do it later

https://stackoverflow.com/questions/12317049/how-to-split-a-long-regular-expression-into-multiple-lines-in-javascript

geoffreykwan · 2023-09-13T22:07:31Z

...ssets/wise5/components/peerChat/peer-chat-question-bank/peer-chat-question-bank.component.ts

@@ -70,8 +70,8 @@ export class PeerChatQuestionBankComponent implements OnInit {
  ): QuestionBankRule[] {
    const cRaterResponses = peerGroupStudentData.map((peerMemberData: PeerGroupStudentData) => {


Should this variable be renamed to responses since it can now be Multiple Choice responses too?

geoffreykwan · 2023-09-13T22:13:52Z

...ssets/wise5/components/peerChat/peer-chat-question-bank/peer-chat-question-bank.component.ts

      if (
        this.content.questionBank.isPeerGroupingTagSpecified() &&
-        referenceComponent.content.type === 'OpenResponse'
+        ['OpenResponse', 'MultipleChoice'].includes(referenceComponent.content.type)


Should the order of the component types be alphabetical like in the other locations? Ideally all these locations would just reference a single array somewhere that contains ['MultipleChoice', 'OpenResponse'].

Hmm. Maybe in a ReferenceableComponents class or isReferenceableComponent() function?

Yea, somewhere to put the single source of truth to avoid duplication.

geoffreykwan · 2023-09-13T22:23:42Z

src/assets/wise5/directives/dynamic-prompt/DynamicPromptEvaluator.ts

+    const evaluator = this.component.dynamicPrompt.isPeerGroupingTagSpecified()
+      ? new FeedbackRuleEvaluatorMultipleStudents(
+          new FeedbackRuleComponent(
+            this.component.dynamicPrompt.getRules(),
+            referenceComponent.content.maxSubmitCount,
+            false
+          ),
+          this.component.constraintService
+        )
+      : new FeedbackRuleEvaluator(
+          new FeedbackRuleComponent(
+            this.component.dynamicPrompt.getRules(),
+            referenceComponent.content.maxSubmitCount,
+            false
+          ),
+          this.component.constraintService
+        );


Is this new FeedbackRuleComponent() the same in both cases? If so, can it be done once and set into a single variable.

new FeedbackRuleComponent( this.component.dynamicPrompt.getRules(), referenceComponent.content.maxSubmitCount, false )

Good catch. Changed.

hirokiterashima · 2023-09-14T17:45:11Z

I forgot to note that you need to test this PR with WISE-Community/WISE-API#235. My bad.

breity

Functionality is working well. 👍

I did find one minor grading issue, but it exists on production and applies to all Peer Chat Question Banks. So we should make a separate issue. This is the problem:
The grading view can show different Question Bank questions than what the students see. This happens when a workgroup adds something to the chat and then goes back to the reference component and changes their answer. When the workgroup revisits the Peer Chat component, the Question Bank will update based on the students' latest response in the reference component. But the grading view will show the old Question Bank questions because the student hasn't submitted any more work to the Peer Chat component.

geoffreykwan

Looks good.

hirokiterashima · 2023-09-15T22:15:13Z

🎉 This PR is included in version 5.111.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

hirokiterashima · 2023-09-15T22:16:18Z

Thanks @breity I wrote up the issue here: #1419

feat(Feedback Rule): New ChoseChoice token

3d119a1

#1409

hirokiterashima self-assigned this Sep 8, 2023

hirokiterashima requested review from geoffreykwan and breity September 8, 2023 23:50

hirokiterashima marked this pull request as ready for review September 8, 2023 23:50

geoffreykwan requested changes Sep 11, 2023

View reviewed changes

hirokiterashima changed the title ~~feat(Feedback Rule): New ChoseChoice token~~ feat(Feedback Rule): New myChoiceChosen token Sep 13, 2023

hirokiterashima added 3 commits September 12, 2023 17:49

Change token to MyChoiceChosen("choiceId"), which will only work with

442336e

DynamicPrompt and QuestionBank, which use ReferenceComponent to look up work for the component #1409

Clean up code. #1409

10b4dff

Extract duplicate getReferenceComponent() to ProjectService #1409

093d531

hirokiterashima requested a review from geoffreykwan September 13, 2023 19:21

geoffreykwan requested changes Sep 13, 2023

View reviewed changes

Clean up code. #1409

08e5d7b

hirokiterashima requested a review from geoffreykwan September 14, 2023 17:17

hirokiterashima mentioned this pull request Sep 14, 2023

feat(PeerGroupWork): Return StudentWorkAnnotation for MC Reference Component WISE-Community/WISE-API#235

Merged

breity approved these changes Sep 14, 2023

View reviewed changes

geoffreykwan approved these changes Sep 15, 2023

View reviewed changes

hirokiterashima merged commit c6c1d64 into develop Sep 15, 2023

hirokiterashima deleted the issue-1409-new-chose-choice-feedback-rule-token branch September 15, 2023 22:13

hirokiterashima added the released label Sep 15, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(Feedback Rule): New myChoiceChosen token #1410

feat(Feedback Rule): New myChoiceChosen token #1410

hirokiterashima commented Sep 8, 2023 •

edited

Loading

geoffreykwan left a comment

hirokiterashima commented Sep 13, 2023 •

edited

Loading

geoffreykwan left a comment •

edited

Loading

geoffreykwan Sep 13, 2023

hirokiterashima Sep 14, 2023

geoffreykwan Sep 15, 2023

geoffreykwan Sep 13, 2023

hirokiterashima Sep 14, 2023

geoffreykwan Sep 13, 2023

hirokiterashima Sep 14, 2023

geoffreykwan Sep 15, 2023

geoffreykwan Sep 13, 2023

hirokiterashima Sep 14, 2023

geoffreykwan Sep 13, 2023

hirokiterashima Sep 14, 2023

geoffreykwan Sep 15, 2023

geoffreykwan Sep 13, 2023

hirokiterashima Sep 14, 2023

hirokiterashima commented Sep 14, 2023

breity left a comment

geoffreykwan left a comment

hirokiterashima commented Sep 15, 2023

hirokiterashima commented Sep 15, 2023

		@@ -70,8 +70,8 @@ export class PeerChatQuestionBankComponent implements OnInit {
		): QuestionBankRule[] {
		const cRaterResponses = peerGroupStudentData.map((peerMemberData: PeerGroupStudentData) => {

feat(Feedback Rule): New myChoiceChosen token #1410

feat(Feedback Rule): New myChoiceChosen token #1410

Conversation

hirokiterashima commented Sep 8, 2023 • edited Loading

Changes

Test

geoffreykwan left a comment

Choose a reason for hiding this comment

hirokiterashima commented Sep 13, 2023 • edited Loading

geoffreykwan left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hirokiterashima commented Sep 14, 2023

breity left a comment

Choose a reason for hiding this comment

geoffreykwan left a comment

Choose a reason for hiding this comment

hirokiterashima commented Sep 15, 2023

hirokiterashima commented Sep 15, 2023

hirokiterashima commented Sep 8, 2023 •

edited

Loading

hirokiterashima commented Sep 13, 2023 •

edited

Loading

geoffreykwan left a comment •

edited

Loading