-
Notifications
You must be signed in to change notification settings - Fork 84
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix/potential answer mcset #233
Fix/potential answer mcset #233
Conversation
updated to have potetial and correct response in the statement and if the submission was right, based on exact match.
forgot to update handler
updated test for potential answer addition
@ryansmith94 This is the answer update, that has response and correct response with potential questions. |
updated test, and fix array issues and tab and space issue.
…neoignis/moodle-logstore_xapi into fix/potential_answer_mcset
@ryansmith94 I tried updating the test and can't seem to get the strings to match. What am I doing wrong? seems the expected is not matching the actual and I am not sure why that is. |
The event that is the expected string is for the attempt submitted and not the actual question statement. did I break something? lol |
} | ||
], | ||
"question": [ | ||
{ | ||
"id": 1, | ||
"qtype": "multichoice", | ||
"qtype": "multichoiceset", | ||
"questiontext": "test_question" | ||
} | ||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing a comma here.
"id": 1, | ||
"questionusageid": 1, | ||
"questionid": 1, | ||
"responsesummary": "answer 2; answer 1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing comma.
"qtype": "multichoiceset", | ||
"questiontext": "test_question" | ||
} | ||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing comma
"correctResponsePatter" => [$questionattempt->rightanswer], | ||
], | ||
'extensions' => [ | ||
'answers' => $answerarray |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Firstly, in the test this is in the result extensions not the object extensions. Secondly, the key needs to be a URL. Thirdly, this probably won't be merged since it'll be too much data for some of the installers with larger volumes of users.
updated to correct an issue, still testing physically.
…neoignis/moodle-logstore_xapi into fix/potential_answer_mcset
@ryansmith94 for the url part.. is there something like http://learninglocker.net/xapi/cmi/choice/response but answers? instead of response? I'm having trouble locating the documentation that spells out which url should be used to identify the extension. |
updating with working version need to update test.
…neoignis/moodle-logstore_xapi into fix/potential_answer_mcset
Because the statement return is set to false by default the test will return false because it is not being sent.
@ryansmith94 I updated the settings section to have a check box that a user can check in order to send the MC choices and right answer to the LRS. If they leave the setting off, the statements will send as normal. If they turn the setting on, then any multiple choice questions will have the choices, and correct response sent with the statement to the LRS. I have tested the output and it is working correctly. |
Trying to get the test to pass. May just copy and paste old test and make the minor corrections needed.
misspell in the data area, that might be why that wasn't working.
missing variable in database which was causing test failure.
I forgot to add the array value to the $config array with the config. That has been fixed.
Didn't notice there was a subarray and that was being used in the transformer. That was dumb of me.
couldn't get $config to work like I wanted it too so going this route.
I didn't know the test case inputs data seperate from the class call. Couldn't figure out why the code was working but not while in CI.
fixing lint issues, and updated variable name
I finally have passing test! That was fun. |
Closing to be continued in #235. |
Description
PR Type