Fix bug with next button in review interface

[kaisersakhi]

Proposed Changes

Fixes #931

• Modify GraphQL query in CoursesReview__Editor.res
• Renamed exclude_submission_id to current_submission_id
• Remove notifications when submission is graded and when coach assigns submssion, for review, to self.

@pupilfirst/developers

Merge Checklist

• Add specs that demonstrate bug / test a new feature.
• [ ] Check if route, query, or mutation authorization looks correct.
  • Add tests for authorization, if required.
• Ensure that UI text is kept in I18n files.
• [ ] Update developer and product docs, where applicable.
• Prep screenshot or demo video for changelog entry, and attach it to issue.
• [ ] Check if new tables or columns that have been added need to be handled in the following services:
  • Users::DeleteAccountService
  • Courses::CloneService
  • Courses::DeleteService
  • Courses::DemoContentService
  • Levels::CloneService
  • Schools::DeleteService
• [ ] Check if changes in packaged components have been published to npm.
• [ ] Add development seeds for new tables.
• [ ] If the updates involve Graph mutations ensure that the files are migrated to the new approach without a mutator.

Demo

fix-bug-next-button-review-interface.mov

[harigopal]

There are a couple of problems with this change that I think result from the way the Next button is implemented:

This implementation works, but can have dismal performance. When looking at a list of submissions that contains thousands of entries, all of those entries will needed to be loaded (the call to scope.index) to get the index of the current submission being looked at. Right now, that'll mean instantiating thousands of ActiveRecord objects just to get the index of one entry.

There might be ways to reduce memory consumption there, but fundamentally, the fact that we need to do things like this is just weird.

I think we should inspect the original requirement for the Next button. Quoting from the issue:

This button was meant to work primarily as a way to jump to the next submission when reviewing student submissions pending review... as a way for coaches to quickly jump to the next submission to look at.

However, this button is present in the UI, regardless of whether the Pending filter is applied, and because it's called Next, its behaviour is confusing.

@pupilfirst/designers Note how the issue text focuses on jumping to the next pending submission. I think we can side-step the entire query performance issue by making a Next button appear only after a reviewer has reviewed a submission. i.e., Right after a submission is reviewed, show a button in the UI (in the area where the review was done) to Show next submission awaiting review.

This button can then pick the first (any) submission that is pending review. It might make sense to actually pick the oldest submission awaiting review. That way, the original need for this button is satisfied, and we avoid performance complications that this approach raises.

[vinutv]

@kaisersakhi There is a UX issue with the current implementation of the "Next" button.
the "Next" button should be hidden from the user when it reaches the last pending submission. Currently, the button is still visible for the last pending submission. When users click on this button, nothing happens except for a notification saying: "There are no similar pending submissions."

To improve the UX,
Hide the "Next" button when the last pending submission is reached. To avoid unnecessary clicks when there are no more pending submissions.

Also, we can improve the button's copy to 'Show next submission awaiting review' as suggested by @harigopal.

[mahesh-krishnakumar]

LGTM