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

[#4570] Instructor: support previewing results as a student #11859

Closed

Conversation

fans2619
Copy link
Contributor

@fans2619 fans2619 commented Jun 10, 2022

Fixes #4570

Outline of Solution

Frontend changes: Add a preview session result panel at the bottom of the instructor session result page. The panel is similar to preview session panel. Clicking "Preview as Student"/"Preview as Instructor" opens the session result page and pass a new query parameter "previewas". The parameter is then passed to backend for further processing.

Backend changes: In the process of getting session results, if it's in "preview mode", "remove" questions and comments (i.e., do not add those) that are NOT visible to instructors in general. However, take note of the corresponding [questions] and [responses (for comments)]. For [questions], pass the question attributes to frontend (to display brief and details) and set a boolean indicator hasResponseButNotVisibleForPreview. hasResponseButNotVisibleForPreview means "the question has responses that are viewable for respondent but the question responses are set to be not viewable to instructors". For [responses], just set a boolean indicator hasCommentNotVisibleForPreview (since the response is visible and should be returned to frontend). hasCommentNotVisibleForPreview means "the response has comments that are viewable for respondent but not viewable to instructors".
In the process of getting session results, if it's in "preview mode", "remove" questions and comments (i.e., do not add those) that are NOT visible to instructors in general. However, take note of the corresponding questions. For [questions that have responses which are visible for respondent but the question responses aren't set to be visible to instructors], pass the question attributes to frontend (to display brief and details) and set a boolean indicator hasResponseButNotVisibleForPreview. For [questions that have comments (on any responses) which are visible to respondent but not visible to instructors], set a boolean indicator hasCommentNotVisibleForPreview.

  • Make frontend and backend changes
  • Improve UI for non-visible questions?
  • Fix bug with non-visible comments
  • Show one "comment hidden" message for entire question
  • Update tests and add more tests

Some examples of intended behavior:

  • Response is not visible to the respondent. -> question hidden
  • Response should be visible to respondent, but at least one of response/giver's name/recipient's name is set to invisible to instructors in general. -> question shown with no response and alert
  • Response is not visible to instructors in general, but visible to the instructor previewing the results, i.e., the instructor may be the targeted recipient. -> question shown with no response and alert
  • Response, giver's name, recipient's name are visible to instructors in general, a specific comment is not visible to instructors in general. -> question and response shown, specific comment hidden, alert shown for the box containing the comment question

The following of this comment are outdated. See the latest comments.

Setting: For this question, giver's name is not visible to instructors in general
What respondent sees:
image

What previewer sees:
image


Setting: A comment is not visible to instructors in general
What respondent sees:
image

What previewer sees:
image

@damithc I tried to improve the UI, do you think it's better now? It's just the comment alert is shown for every group of responses.

image

fans2619 added 5 commits June 10, 2022 01:19
Currently preview shows exactly what the person being previewed sees. The next step is to remove/modify results that shouldn't be visible to the person previewing the results.
@fsgmhoward fsgmhoward added the s.Ongoing The PR is being worked on by the author(s) label Jun 12, 2022
@fans2619
Copy link
Contributor Author

@wkurniawan07 take a quick review?

@fans2619 fans2619 requested a review from wkurniawan07 June 14, 2022 11:43
@fans2619
Copy link
Contributor Author

image

@damithc
Copy link
Contributor

damithc commented Jun 15, 2022

@fans2619 is the message about hidden comments shown only when there are actual comments? Or shown even if there are no actual comments?

Perhaps the message (for hidden responses) can be revised as
Omitted from the preview because some (or all) parts of the responses are not visible to instructors, as per the visibility settings of the question.

@fans2619
Copy link
Contributor Author

fans2619 commented Jun 15, 2022

@fans2619 is the message about hidden comments shown only when there are actual comments? Or shown even if there are no actual comments?

The message about hidden comments is shown when there is at least one comment hidden for at least one response below. So it's not shown when there's simply no comment.

Perhaps the message (for hidden responses) can be revised as Omitted from the preview because some (or all) parts of the responses are not visible to instructors, as per the visibility settings of the question.

Sure.

@fans2619
Copy link
Contributor Author

@damithc For the hidden comments, an alternative way is to show only one alert message at question level. Let me know if that's preferred.

@damithc
Copy link
Contributor

damithc commented Jun 15, 2022

@damithc For the hidden comments, an alternative way is to show only one alert message at question level. Let me know if that's preferred.

Yes, that might be better (less noisy), if achievable without much extra cost.

@fans2619
Copy link
Contributor Author

I guess it should be achievable and also simpler than the current implementation. I'll change to that later.

@fans2619
Copy link
Contributor Author

fans2619 commented Jun 16, 2022

@damithc Could you please verify the position of hidden comment message is fine before I start implement? And you can suggest an actual message for it if you want.

image


image

@damithc
Copy link
Contributor

damithc commented Jun 16, 2022

@damithc Could you please verify the position of hidden comment message is fine before I start implement? And you can suggest an actual message for it if you want.

Yes, the location seems fine.

Message:

Some comments (on responses) received for this question are omitted from this preview because they are not visible to instructors, as per their visibility settings.

The other message can be revised as follows, to make the two more consistent with each other.

Responses received for this question are omitted from this preview because some (or all) parts of the responses are not visible to instructors, as per the visibility settings of the question.

What do you think?

If possible, we can use an icon like this at the start of the message to differentiate from other messages. https://fontawesome.com/icons/eye-slash?s=solid

@fans2619
Copy link
Contributor Author

Thanks for the suggestions! 😀

@fans2619
Copy link
Contributor Author

Updated:

image


image

@fans2619
Copy link
Contributor Author

fans2619 commented Jun 16, 2022

@damithc Another question is whether it's preferred for the info box to be above or below the page title "Feedback Session Results". Other two pages are putting it below the page title, although I prefer putting it above so that everything below is what the actual respondent sees. You're also welcome to suggest changes to the text in the box.

image


image

image

@damithc
Copy link
Contributor

damithc commented Jun 16, 2022

@damithc Another question is whether it's preferred for the info box to be above or below the page title "Feedback Session Results". Other two pages are putting it below the page title, although I prefer putting it above so that everything below is what the actual respondent sees. You're also welcome to suggest changes to the text in the box.

Yes, above-the-heading is probably better. However, follow what other similar pages does and create a separate issue to move the message box in all similar pages. This issue can be left to new contributors.

I'm OK with the message content. Do we call them alert boxes or message boxes (which is easier for users to understand)?

@fans2619
Copy link
Contributor Author

fans2619 commented Jun 16, 2022

I'm OK with the message content. Do we call them alert boxes or message boxes (which is easier for users to understand)?

I'm fine with "message box" :-)

image

@fans2619 fans2619 requested review from wkurniawan07 and removed request for wkurniawan07 December 8, 2022 10:56
@fans2619 fans2619 added s.ToReview The PR is waiting for review(s) and removed s.Ongoing The PR is being worked on by the author(s) labels Dec 8, 2022
@fans2619 fans2619 marked this pull request as ready for review December 8, 2022 13:10
@nusoss-bot
Copy link

Guys, This PR seems to be stalling (no activities for the past 8 days). 🐌 😢
Hope someone can get it to move forward again soon...

@wkurniawan07 wkurniawan07 modified the milestones: V8.20.0, V8.21.0 Dec 25, 2022
@nusoss-bot
Copy link

Guys, This PR seems to be stalling (no activities for the past 11 days). 🐌 😢
Hope someone can get it to move forward again soon...

@nusoss-bot
Copy link

Guys, This PR seems to be stalling (no activities for the past 14 days). 🐌 😢
Hope someone can get it to move forward again soon...

@fans2619 fans2619 added s.Ongoing The PR is being worked on by the author(s) and removed s.ToReview The PR is waiting for review(s) labels Jan 5, 2023
@nusoss-bot
Copy link

Guys, This PR seems to be stalling (no activities for the past 7 days). 🐌 😢
Hope someone can get it to move forward again soon...

@fsgmhoward
Copy link
Member

@fans2619 Reminder for this PR!

@fans2619
Copy link
Contributor Author

@fans2619 Reminder for this PR!

Hi thanks for the reminder. However, I'd like to continue working on it when it's confirmed that someone will review it immediately after I resolve the conflicts. It doesn't feel good that I resolved conflicts and there was no review, and after a while conflicts appeared again.

@wkurniawan07 wkurniawan07 removed this from the V8.21.0 milestone Jan 21, 2023
@nusoss-bot
Copy link

Guys, This PR seems to be stalling (no activities for the past 7 days). 🐌 😢
Hope someone can get it to move forward again soon...

@nusoss-bot
Copy link

Folks, This PR seems to be stalling (no activities for the past 7 days). 🐌 😢
Hope someone can get it to move forward again soon...

2 similar comments
@nusoss-bot
Copy link

Folks, This PR seems to be stalling (no activities for the past 7 days). 🐌 😢
Hope someone can get it to move forward again soon...

@nusoss-bot
Copy link

Folks, This PR seems to be stalling (no activities for the past 7 days). 🐌 😢
Hope someone can get it to move forward again soon...

@samuelfangjw
Copy link
Member

Closing due to inactivity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
s.Ongoing The PR is being worked on by the author(s)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Instructor: support previewing results as a student
7 participants