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

Fix 404 error in inspect submitter view #1209

Merged
merged 1 commit into from
Aug 10, 2023

Conversation

lainets
Copy link
Contributor

@lainets lainets commented Jun 27, 2023

Description

What?

Include error submissions in CachedPoints, and prioritize INITIALIZED and WAITING over ERROR and REJECTED submissions when falling back to last submission in inspect views.

Why?

#1193

Fixes #1193

Testing

What type of test did you run?

  • Accessibility test using the WAVE extension.
  • Django unit tests.
  • Selenium tests.
  • Other test. (Add a description below)
  • Manual testing.

Tested that the best submission is prioritized, and INITIALIZED and WAITING are prioritized over ERROR and REJECTED when a best submission doesn't exist.

Did you test the changes in

  • Chrome
  • Firefox
  • This pull request cannot be tested in the browser.

Is it Done?

  • Reviewer has finished the code review
  • After the review, the developer has made changes accordingly
  • Customer/Teacher has accepted the implementation of the feature

@lainets lainets force-pushed the error_submissions branch 2 times, most recently from 34b4ffe to 963e42d Compare June 27, 2023 11:38
This will add error submissions to some APIs, to the submission
list dropdowns in the course instance page, and to
NextUnassessedSubmitterView (this means the view falls back on
error submissions if no proper submissions exist, which seems to
be in accordance with rest of the view code). Closes apluslms#1193.
Copy link
Contributor

@markkuriekkinen markkuriekkinen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! I could merge this now. Is that OK since you have been working on other cache changes too?

@lainets lainets merged commit 28f2735 into apluslms:master Aug 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
2 participants