Survey modality #9000

nucleogenesis · 2022-01-14T22:33:43Z

Summary

Introduces Survey modality support by adding some display logic based on whether the content is a Survey or not.

No "correct" / "incorrect" iconography on the answering or reporting
Simplified CurrentTryOverview - only use that for surveys because TriesOverview summarizes multiple attempts which we don't need here.

It's not super complex logic there in CurrentTryOverview, but I wanted to be sure it was covered and started writing tests for the bits I was going to change and I figured that since I was there I should cover all of the props and display logic.

survey.mp4

References

Figma

Reviewer guidance

I tried to make the video cover everything so you don't have to do all this but I think it'd be good if someone spun this up.

Setup

Be sure to yarn install and pip install requirements.txt so you have the latest LE Utils & Kolibri constants.
Make sure you have the latest Kolibri QA Channel
Go to Exercises -> Channel based quizzes
Pick a piece of content from there that you want to update and update it's options: { modality: SURVEY } value. I do this in SQLite Browser, but you can probably do it in the Kolibri Shell getting the content node by id, updating it's extra_fields JSON accordingly.

Actually testing

Review the Figma
Check mobile
Take multiple tries
Test while not logged in

Testing checklist

Contributor has fully tested the PR manually
If there are any front-end changes, before/after screenshots are included
Critical user journeys are covered by Gherkin stories
Critical and brittle code paths are covered by unit tests

PR process

PR has the correct target branch and milestone
PR has 'needs review' or 'work-in-progress' label
If PR is ready for review, a reviewer has been added. (Don't use 'Assignees')
If this is an important user-facing change, PR or related issue has a 'changelog' label
If this includes an internal dependency change, a link to the diff is provided

Reviewer checklist

Automated test coverage is satisfactory
PR is fully functional
PR has been tested for accessibility regressions
External dependency files were updated if necessary (yarn and pip)
Documentation is updated
Contributor is in AUTHORS.md

fixes to current try overview spec remove unused in attemptloglist spec

and some lint

rtibbles

Spotted 1 potential regression in code review. Have not manually tested yet.

rtibbles · 2022-01-19T17:45:10Z

kolibri/core/assets/src/views/ExamReport/CurrentTryOverview.vue

    },
    computed: {
      ...mapGetters(['currentUserId']),
+      // QUESTION: Why? Wouldn't anything else fail validation? Why not just call tryValidator here?


Validators don't run in production, only in development. This value can still be undefined, so we need to handle that case. Could swap it out for tryValidator instead, which might be more robust.

If the component defines a prop as required and validates it then I think it should be able to operate under the assumption that it won't be used incorrectly. The parent component should (and I think in all/most cases do - can confirm in a bit) guard rendering CurrentTryOverview w/ v-if

I don't disagree - but it's not how Vue handles it, so better to be defensive, I think.

rtibbles · 2022-01-19T17:47:10Z

kolibri/core/assets/src/views/ExamReport/CurrentTryOverview.vue

@@ -171,7 +175,9 @@
        return this.currentTryDefined && !isUndefined(this.currentTry.correct);
      },
      score() {
-        return this.currentTryDefined ? this.currentTry.correct / this.totalQuestions : 0;


Have lost the division here in the small refactor.

Will update the test scenario to be more robust thank you!

rtibbles · 2022-01-19T17:49:56Z

kolibri/core/assets/src/views/ExamReport/index.vue

@@ -403,6 +421,7 @@
          return;
        }
        this.loading = true;
+        console.log(this.getParams());


Spare console.log

Changes addressed!

pcenov · 2022-01-20T15:49:41Z

Hi @radinamatic and @nucleogenesis, I tried testing this PR with all of the Buildkite builds plus directly from the nucleogenesis:survey-modality branch but I am not able to see any of the changes.
I tested with all the available survey exercises and also updated the options: { modality: SURVEY } value of an exercise using the SQLite Browser and still I'm not seeing any of the new functionality. Wondering am I missing something? :)

marcellamaki · 2022-01-20T16:01:37Z

@pcenov and @nucleogenesis -- I'm running into a similar problem after having updated an exercise to the SURVEY modality while trying to do some manual QA after a code read through this morning (which looks good!). One thing I did notice as I was troubleshooting is that Modalities.SURVEY coming from le-utils seems to be undefined when console logging it. I can't quite sort that out yet, since I've double checked and I have the new le-utils version and I checked the PR for the le-utils version and it all makes sense reading through. I will do a little more investigating and see if I can find anything else!

rtibbles · 2022-01-20T16:28:16Z

@nucleogenesis looks like you updated the version of le-utils but not its corollary JS package kolibri-constants. Need to update both! (because of the jank in kolibri-core, will need to update the version in both places it appears in two separate package.jsons)

kolibri/plugins/learn/assets/src/views/CompletionModal/index.vue

kolibri/plugins/learn/assets/src/views/QuizRenderer/QuizReport.vue

kolibri/plugins/learn/assets/src/views/QuizRenderer/index.vue

kolibri/plugins/learn/assets/src/views/CompletionModal/index.vue

radinamatic · 2022-01-21T06:37:02Z

Tested running from source (@pcenov please re-test with DEB and EXE assets), and seeing various issues in Firefox on Linux.

when looking at reports, (surveys) should not display any correct/incorrect information, any scores, or completion criteria.

Not seeing any completion criteria, but scores are visible in the Attempts dropdown when surveys are submitted multiple times.
When survey resources are browsed in their original channel (gutuf-potuv token), all questions display correctly. However, when I try to browse those same resources that are imported in the Kolibri QA channel, the first question (Teacher ID?) never loads properly, but the rest of them do. At one time I caught a glimpse of the Firefox can’t establish a connection to the server at ws://127.0.0.1:3016/sockjs-node/889/0vwz1pqo/websocket error that I never saw before 😕

1	2

There is also a bit of weirdness in displaying the question area when changing browsing width, and hint appearance seems to be affecting the width of the paragraph (shifting may be affecting readability), but those could be issues with the channel/ricecooker, and not this PR.

1	2

pcenov · 2022-01-21T15:28:51Z

In addition to the issues observed by @radinamatic I've reported separately the following issues: #9020, #9021, #9022, #9023
I didn't find any regressions for the learner facing practice quiz scenarios.

rtibbles · 2022-01-21T16:11:12Z

I think we need to fix #9020 and #9023 - before merging the PR. The other two are intended behaviour at the moment as far as I can tell.

…t button was hidden

nucleogenesis · 2022-01-21T23:20:05Z

@pcenov @radinamatic

The following fixes have been pushed here:

Questions ordering in report is now correct
Infinite loader on submission without answers - now the user sees the dropdown to look through past submissions only
(Unreported) the "submit" button was not visible so I added some padding on the bottom for small screens. It was getting hidden under the arrow icons at the bottom of the screen
Squished renderer of the content
% removed from list of attempts

I could not replicate the issue where the first question stopped working when accessed through the QA channel because I cannot upgrade my channel. I'll see if I can get it working later on.

radinamatic · 2022-01-24T11:08:48Z

- Questions ordering in report is now correct ✔️
- Infinite loader on submission without answers - now the user sees the dropdown to look through past submissions only ✔️
- (Unreported) the "submit" button was not visible so I added some padding on the bottom for small screens. It was getting hidden under the arrow icons at the bottom of the screen ✔️
- % removed from list of attempts ✔️
Squished renderer of the content 🔴

Mobile view looks OK, but the issue is still present while resizing on wider screens, or on tablet sizes in landscape mode:

Firefox Chrome

squished.mp4
Hints

Are still appearing, are those going away? And I did notice that the ID fields are not counting answers that contain letters as answered. Did we agree with partners that the student and teachers IDs can contain only numbers @rtibbles?

hint.mp4
Last thing: seems possible to open the survey even if the user is not signed in (c/p URL), oversight or meant to be...? 🤔

kolibri/plugins/perseus_viewer/assets/src/views/PerseusRendererIndex.vue

rtibbles · 2022-01-24T19:14:51Z

Are still appearing, are those going away? And I did notice that the ID fields are not counting answers that contain letters as answered. Did we agree with partners that the student and teachers IDs can contain only numbers @rtibbles?

Tooltips are not hints. There's nothing that should be stopping the tooltips from appearing, so this is expected behaviour.
It is a numeric input box, so it can only contain numbers - we have communicated this.

Last thing: seems possible to open the survey even if the user is not signed in (c/p URL), oversight or meant to be...? thinking

Users can take surveys anonymously - I'd say this is intended behaviour for now (even if potentially undesirable in some contexts).

radinamatic · 2022-01-24T22:30:05Z

Installed the APK asset on an Android tablet very similar to the one partner will be using, and content is rendering fine (no squishing), as are apparently all the other survey features 💯

The only doubt remains why the resources in this channel are not displaying correctly when imported in other channels...? 🤔
To recap:

When survey resources are browsed in their original channel (gutuf-potuv token), all questions display correctly. However, when I try to browse those same resources that are imported in the Kolibri QA channel, most of the time I get just the loader spinning

To exclude this being a QA channel quirk, I created a brand new channel (jotuh-mojop token) and imported only the 4 survey resources from the source channel, but no change. I even thought this might be because the Allow as channel quiz option (which is present in the source channel) is not automatically set up when the resources are imported in another channel, and went to check and apply them manually on my new channel, but in Kolibri the only result was to have the counter appear (as it should with channel quizzes), and the resources still do not display correctly.

The frustrating part is that hardly anything appears as error in the console... 😒

Source channel	New channel

If we are sure that the partner will only be using the gutuf-potuv token to view these resources on their tablets, this might not be a blocker, but if they are creating derivative channels, trouble... 😕

rtibbles · 2022-01-24T22:50:44Z

The error looks like an issue with loading the next question inside Perseus - so it is probably being passed a question identifier that does not exist inside the perseus file.

radinamatic · 2022-01-24T23:16:50Z

Ok, since we decided this is not a blocker, let's merge this in! 👍🏽

radinamatic

Manual QA cleared up, good to go! 🎉

nucleogenesis added 5 commits January 13, 2022 16:25

CurrentTryOverview survey logic & tests

91a2bc6

fixes to current try overview spec remove unused in attemptloglist spec

Add survey props, display logic, throughout

7e8e480

Survey title on exam report shows reflect icon

6d38fad

Submission modal, reports $trs added, logic for display

e8e2154

account for survey in coach reports

c36309b

nucleogenesis added TODO: needs review Waiting for review changelog Important user-facing changes labels Jan 14, 2022

nucleogenesis requested review from rtibbles and marcellamaki January 14, 2022 22:33

nucleogenesis force-pushed the survey-modality branch from 28c25e8 to a39eaba Compare January 18, 2022 17:35

bump le-utils/kolibri-constants deps to 0.1.38

3a3084b

and some lint

nucleogenesis force-pushed the survey-modality branch from a39eaba to 3a3084b Compare January 18, 2022 21:47

rtibbles previously requested changes Jan 19, 2022

View reviewed changes

nucleogenesis added 2 commits January 19, 2022 11:35

fixed it.each tests for CurrentTryOverview score() and fixed score()

93b00c3

clean up unnecessary comment, console.log

83b5994

rtibbles reviewed Jan 20, 2022

View reviewed changes

kolibri/plugins/learn/assets/src/views/CompletionModal/index.vue Outdated Show resolved Hide resolved

rtibbles reviewed Jan 20, 2022

View reviewed changes

kolibri/plugins/learn/assets/src/views/QuizRenderer/QuizReport.vue Outdated Show resolved Hide resolved

rtibbles reviewed Jan 20, 2022

View reviewed changes

kolibri/plugins/learn/assets/src/views/QuizRenderer/index.vue Outdated Show resolved Hide resolved

rtibbles reviewed Jan 20, 2022

View reviewed changes

kolibri/plugins/learn/assets/src/views/CompletionModal/index.vue Outdated Show resolved Hide resolved

nucleogenesis added 2 commits January 20, 2022 09:55

also bump kolibri-constants in core-for-export

126bd4f

strings cleanup (thanks @rtibbles!)

f330583

better empty survey ux, padding added on mobile quizrenderer as submi…

79dbd04

…t button was hidden

nucleogenesis force-pushed the survey-modality branch from 330edf5 to b7e9b4e Compare January 23, 2022 03:22

rtibbles reviewed Jan 24, 2022

View reviewed changes

kolibri/plugins/perseus_viewer/assets/src/views/PerseusRendererIndex.vue Outdated Show resolved Hide resolved

perseus to only show hints when allowHints or interactive is true

b2d2e57

nucleogenesis force-pushed the survey-modality branch from b7e9b4e to b2d2e57 Compare January 24, 2022 19:11

radinamatic approved these changes Jan 24, 2022

View reviewed changes

nucleogenesis merged commit cc176e4 into learningequality:develop Jan 25, 2022

nucleogenesis mentioned this pull request Jan 27, 2022

Survey modality - Cannot see the report for a survey submitted without any answered questions #9023

Closed

nucleogenesis mentioned this pull request Feb 16, 2022

Survey modality - Answer history is displayed in reverse order #9020

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Survey modality #9000

Survey modality #9000

nucleogenesis commented Jan 14, 2022 •

edited

Loading

rtibbles left a comment

rtibbles Jan 19, 2022

nucleogenesis Jan 19, 2022

rtibbles Jan 19, 2022

rtibbles Jan 19, 2022

nucleogenesis Jan 19, 2022

rtibbles Jan 19, 2022

pcenov commented Jan 20, 2022

marcellamaki commented Jan 20, 2022

rtibbles commented Jan 20, 2022

radinamatic commented Jan 21, 2022 •

edited

Loading

pcenov commented Jan 21, 2022

rtibbles commented Jan 21, 2022

nucleogenesis commented Jan 21, 2022

radinamatic commented Jan 24, 2022

rtibbles commented Jan 24, 2022

radinamatic commented Jan 24, 2022

rtibbles commented Jan 24, 2022

radinamatic commented Jan 24, 2022

radinamatic left a comment

Survey modality #9000

Survey modality #9000

Conversation

nucleogenesis commented Jan 14, 2022 • edited Loading

Summary

References

Reviewer guidance

Setup

Actually testing

Testing checklist

PR process

Reviewer checklist

rtibbles left a comment

Choose a reason for hiding this comment

rtibbles Jan 19, 2022

Choose a reason for hiding this comment

nucleogenesis Jan 19, 2022

Choose a reason for hiding this comment

rtibbles Jan 19, 2022

Choose a reason for hiding this comment

rtibbles Jan 19, 2022

Choose a reason for hiding this comment

nucleogenesis Jan 19, 2022

Choose a reason for hiding this comment

rtibbles Jan 19, 2022

Choose a reason for hiding this comment

pcenov commented Jan 20, 2022

marcellamaki commented Jan 20, 2022

rtibbles commented Jan 20, 2022

radinamatic commented Jan 21, 2022 • edited Loading

pcenov commented Jan 21, 2022

rtibbles commented Jan 21, 2022

nucleogenesis commented Jan 21, 2022

radinamatic commented Jan 24, 2022

rtibbles commented Jan 24, 2022

radinamatic commented Jan 24, 2022

rtibbles commented Jan 24, 2022

radinamatic commented Jan 24, 2022

radinamatic left a comment

Choose a reason for hiding this comment

nucleogenesis commented Jan 14, 2022 •

edited

Loading

radinamatic commented Jan 21, 2022 •

edited

Loading