disable editable fields if criterion released #7264
Conversation
Pull Request Test Coverage Report for Build 11431520026Details
💛 - Coveralls |
for more information, see https://pre-commit.ci
… criterion-deactivate
…01/Markus-Anubha into criterion-deactivate
| @@ -1,8 +1,15 @@ | |||
| <div class='float-right'> | |||
|
|
|||
| <% released = !criterion.results_unreleased? %> | |||
There was a problem hiding this comment.
This is correct but a bit inefficient, since it'll be repeated for each criterion (and rubric level). Instead, try to figure out how the existing flash message "Cannot update criteria for an assignment with released marks." is being displayed, presumably conditioned on some boolean. That same boolean can be passed into this partial as a released variable, and you can use that directly. Same with the rubric levels.
On a related note, please modify the "Cannot update criteria for an assignment with released marks." so that it appears as an "info" rather than "error", since technically there's no error, we're just letting instructors know they can't change anything.
for more information, see https://pre-commit.ci
… criterion-deactivate
| @@ -1,3 +1,5 @@ | |||
| <% marks_released = flash[:notice].present? %> | |||
There was a problem hiding this comment.
Rather than define this here, compute it once in the main (top-level) view that is initially rendered and that includes this partial, and then pass the boolean value into the partial as a local variable. Please do this for all partials.
app/views/criteria/index.html.erb
Outdated
| @@ -9,16 +9,18 @@ | |||
| @assignment.parent_assignment.short_identifier + ' ' + PeerReview.model_name.human : | |||
| @assignment.short_identifier) %> | |||
|
|
|||
| <% marks_released = flash[:notice].present? %> | |||
There was a problem hiding this comment.
As a note related to my above comment: It's good to define this variable here, since it's a top-level view.
However it's not ideal to rely on whether the flash is set or not, since there could be other messages we have. Instead, access @assignment directly to compute the boolean value.
|
|
||
| <% if marks_released %> | ||
| <span class="button disabled"><%= t('rubric_criteria.level.add') %></span> | ||
| <% else %> | ||
| <%= link_to t('rubric_criteria.level.add'), |
There was a problem hiding this comment.
Indent these lines (since they're now enclosed in an else block)
| remote: true %> | ||
| remote: true, | ||
| disabled: marks_released | ||
| %> |
There was a problem hiding this comment.
Keep this on the preceding line (follow the existing style)
| @@ -2,23 +2,24 @@ | |||
| # Erb template for display rubric criterion levels in the assignment rubric manager | |||
| %> | |||
| <div class='level'> | |||
|
|
|||
Proposed Changes
(Describe your changes here. Also describe the motivation for your changes: what problem do they solve, or how do they improve the application or codebase? If this pull request fixes an open issue, use a keyword to link this pull request to the issue.)
On an Assignment's Criterion page, if the criterion is released, disable the input/editable fields, and disable the sortable list
Screenshots of your changes (if applicable)
When criterion is unreleased, still looks the same:
Light Mode
Dark Mode
When criterion IS released:
Light Mode
Dark Mode
Associated documentation repository pull request (if applicable)
Type of Change
(Write an
Xor a brief description next to the type or types that best describe your changes.)Checklist
(Complete each of the following items for your pull request. Indicate that you have completed an item by changing the
[ ]into a[x]in the raw text, or by clicking on the checkbox in the rendered description on GitHub.)Before opening your pull request:
After opening your pull request:
Questions and Comments
(Include any questions or comments you have regarding your changes.)