Fix incorrectly marked "incomplete" exercises on exercise copy #4761
Conversation
…ot we need to check for completion - required within the EditModal for validations to work properly, but not in other quick edit options. previously was setting to false incorrectly due to assessmentItems validation failing when we hadn't loaded the data
There was a problem hiding this comment.
A couple of places where I don't think we do actually need to run the completeness check in the action.
As this is not changing existing behaviour, this is not blocking though.
The only blocker would be not setting complete at all in the case where we're not checking completeness.
| assessmentItems: context.rootGetters['assessmentItem/getAssessmentItems'](id), | ||
| files: context.rootGetters['file/getContentNodeFiles'](id), | ||
| }); | ||
| let complete = true; |
There was a problem hiding this comment.
If we're not checking complete, we should just not set this attribute at all, either true or false.
| @@ -258,6 +258,7 @@ | |||
| retryFailedCopy: withChangeTracker(function(changeTracker) { | |||
| this.updateContentNode({ | |||
| id: this.nodeId, | |||
| checkComplete: true, | |||
There was a problem hiding this comment.
I think we probably don't need to check complete here either.
| @@ -399,7 +399,11 @@ | |||
|
|
|||
| if (completeCheck !== node.complete) { | |||
| validationPromises.push( | |||
| vm.updateContentNode({ id: nodeId, complete: completeCheck }) | |||
There was a problem hiding this comment.
I guess technically, we don't need to rerun the complete check here, because we already ran it above?
Update tests.
There was a problem hiding this comment.
Both code changes and manual QA check out. However, I discovered a minor bug that I will open in a separate issue. Thanks @marcellamaki @rtibbles
|
LGTM as well, have reported a minor console error issue here: #4763 |
Summary
Description of the change(s) you made
adds a flag to the updateContentNode action to determine whether or not we need to check for completion - required within the EditModal for validations to work properly, but not in other quick edit options. Previously, this check was setting
completeto false incorrectly due to assessmentItems validation failing in situations where we hadn't loaded that assessment data (i.e. not EditModal).Manual verification steps performed
Regression testing:
Does this introduce any tech-debt items?
Well, there probably needs to be some general cleanup of a lot of how these validations are working.... and this feels pretty brittle.
Reviewer guidance
How can a reviewer test these changes?
Are there any risky areas that deserve extra testing?
References
Fixes #4747
Comments
Contributor's Checklist
PR process:
CHANGELOGlabel been added to this PR. Note: items with this label will be added to the CHANGELOG at a later timedocslabel has been added if this introduces a change that needs to be updated in the user docs?requirements.txtfiles also included in this PRStudio-specifc:
notranslateclass been added to elements that shouldn't be translated by Google Chrome's automatic translation feature (e.g. icons, user-generated text)pages,components, andlayoutsdirectories as described in the docsTesting:
Reviewer's Checklist
This section is for reviewers to fill out.
yarnandpip)