Skip to content
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

student viewing results: the spinner keeps going until the user clicks on the page #12182

Closed
damithc opened this issue Mar 9, 2023 · 10 comments · Fixed by #12183
Closed

student viewing results: the spinner keeps going until the user clicks on the page #12182

damithc opened this issue Mar 9, 2023 · 10 comments · Fixed by #12183
Labels
p.Urgent Would like to handle in the very next release s.ToInvestigate The issue sounds complete but needs validation by a core team member

Comments

@damithc
Copy link
Contributor

damithc commented Mar 9, 2023

v8.23, production
Reported by a user, reproducible

When the student is viewing results using a link, the spinner for questions keep going indefinitely, until the user clicks on the page. After that, the questions show up.

image

Doesn't seem to affect everyone but students in this particular course is affected, so perhaps it depends on the question configuration. The first and second questions are weighted rubric question.
image

The rest are essay questions:
image

Let me know if you need more info.

@damithc damithc added p.Urgent Would like to handle in the very next release s.ToInvestigate The issue sounds complete but needs validation by a core team member labels Mar 9, 2023
@samuelfangjw
Copy link
Member

Possibly related to #12068

@damithc
Copy link
Contributor Author

damithc commented Mar 9, 2023

Possibly related to #12068

@samuelfangjw Yup, most likely. It seems to affect other type of questions in the page as well. Can we get someone to fix it soon?

@zhaojj2209
Copy link
Contributor

After much investigation, I think I have a general idea of what is happening:

When a student views the results of a feedback session, the frontend initially calls GetFeedbackQuestionsAction (/webapi/questions) to retrieve the list of questions for this feedback session, then loads the results lazily by calling GetSessionResultsAction (/webapi/result) for each question depending on whether the response panel is in the viewport.

However, GetFeedbackQuestionsAction hides the rubric weights for students viewing feedback results (intent=STUDENT_RESULT). This causes the weights to be an empty array even though hasWeights is set to true, therefore an array access error is thrown in the console when calculating the statistics (as it accesses the weights array if hasWeights is true), causing the frontend to "halt" and the spinner to load infinitely.

Clicking causes activity to resume, and after GetSessionResultsAction is called, no error is encountered when calculating rubric statistics because GetSessionResultsAction does not hide the rubric weights when intent=STUDENT_RESULT.

The fix here would therefore be to add an extra check and calculate statistics only if the weights array is not empty. This would allow the frontend to load the responses without any errors.

However, I believe there is a much, much more serious issue here: the weights are not being hidden from the students in GetSessionResultsAction. This means that students would be able to view weights related statistics (e.g. per recipient stats) even though they are not supposed to (see below screenshot of student view responses page).
Screenshot 2023-03-09 at 23 12 03

@damithc Can I confirm if students should not have any knowledge on whether the rubric question is weighted (so the statistics they view for weighted rubric questions should be the same as if the question was not weighted)? I will be adding the extra fix to hide rubric weights in GetSessionResultsAction as well.

@damithc
Copy link
Contributor Author

damithc commented Mar 9, 2023

@damithc Can I confirm if students should not have any knowledge on whether the rubric question is weighted (so the statistics they view for weighted rubric questions should be the same as if the question was not weighted)? I will be adding the extra fix to hide rubric weights in GetSessionResultsAction as well.

@zhaojj2209 thanks for investigating this. I feel students should know that the rubric is weighted and also know the weights, when they are submitting responses as well as when they are viewing results. In fact knowing the weights can help them make more informed decisions when evaluating. So, there is no need to hide the weights. What do you guys think? Or did I misunderstand the question?

@zhaojj2209
Copy link
Contributor

@damithc As shown in the above screenshot, the students are able to view the per recipient statistics of other students as well, which I believe is not intended as these statistics are meant for instructors. This is because the rubric stats table component is shared between the instructor view results and student view results pages, but it does not differentiate between student and instructor; whether or not the per recipient statistics are shown is solely dependent on whether the question has weights.

The faster fix right now would be to hide weights from the students so that the per recipient statistics will also be hidden from the students. Given that rubric weights are deliberately hidden from students in GetFeedbackQuestionsAction, I think the current intended behaviour should be to hide weights from students altogether, so this fix would also be in line with the current intended behaviour.

We can leave the showing of weights to students as a separate issue, as that will require more changes (specifically, unhiding the weights from students on the backend side, and refactoring the frontend rubric stats table to differentiate between student and instructor).

@damithc
Copy link
Contributor Author

damithc commented Mar 9, 2023

@damithc As shown in the above screenshot, the students are able to view the per recipient statistics of other students as well, which I believe is not intended as these statistics are meant for instructors.

Yes, that should not be shown to students. This is a serious data leak. Must be fixed ASAP.

e can leave the showing of weights to students as a separate issue, as that will require more changes (specifically, unhiding the weights from students on the backend side, and refactoring the frontend rubric stats table to differentiate between student and instructor).

Agreed.

@samuelfangjw
Copy link
Member

@damithc As shown in the above screenshot, the students are able to view the per recipient statistics of other students as well, which I believe is not intended as these statistics are meant for instructors.

Yes, that should not be shown to students. This is a serious data leak. Must be fixed ASAP.

This is not a data leak. Students cannot see results of other students. The results table is constructed from the results given and received by the student.

@samuelfangjw
Copy link
Member

This should still be removed though, doesn't really make sense to have per recipient stats in the context of student responses.

@zhaojj2209
Copy link
Contributor

This is not a data leak. Students cannot see results of other students. The results table is constructed from the results given and received by the student.

My bad for the misconception! So the per recipient stats tables would only be showing the responses that the user gave to other students.

Nevertheless, I agree that it doesn't make sense for the per recipient tables to be there.

@damithc
Copy link
Contributor Author

damithc commented Mar 9, 2023

Agreed, not really a data leak, but best to be removed from view.

samuelfangjw pushed a commit that referenced this issue Mar 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p.Urgent Would like to handle in the very next release s.ToInvestigate The issue sounds complete but needs validation by a core team member
Projects
None yet
3 participants