Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
feat: ORA staff grader endpoints upgrade #10
base: FG/ORA_staff_grader_initialize_upgrade
Are you sure you want to change the base?
feat: ORA staff grader endpoints upgrade #10
Changes from 1 commit
d0d9b02
9e5b92b
6c9079d
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this
given
andreceived
as the assessment type present elsewhere? can we elaborate on the meaning of each?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe the default should be list all assessments for the submission uuid, so received.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is a single handler better than separating concerns?
list_assessments_received
orlist_assessments_given
are more explicit thanlist_assessments...
, but I don't have other strong argumentsThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the MFE, it's just a table with a filter button; the data output has the same format. At the backend level, it's just a design decision that, compared to having two handlers, neither adds nor subtracts anything; it's pretty much the same.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not the same. When using a single handler with a variable
assessment_type
, which defaults to given, you are making a decision there, you're saying the defaults assessments returned are given by students on the submission. When using two handlers, the client can actively decide which one to call without guessing the system's default. Don't you think?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah! that's true... but it does not exists a request without assessment_type (now called
assessment_filter
), it will always begiven
orreceived
...In fact, it is a required param in the
AssessmentFeedbackView
of edx-platform and declared in lms/djangoapps/ora_staff_grader/constants.py so, list_assessments() would be never called without assessment_filter...jeje, but now tha you are saying this... I think that in form, this will look even better 😃