-
Notifications
You must be signed in to change notification settings - Fork 176
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
Fixes content_id
bug whenever underlying content is changed 🎉
#3803
Conversation
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've laid out a case I think this fails to properly capture - some tests to cover these cases will help to document and validate the assumptions being made here.
- update content_id only of copied nodes - update content_id of nodes on channel syncing - fix sync of assessment items bug wherein assessment items were getting synced based on tags attribute
@rtibbles I updated the For everyone else here is the summary of our consensus on how @rtibbles @bjester The test cases should cover 100% of the changes I made and they test all possible scenarios I had in mind. I am very confident that this should be ready to merge! 🎉 |
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.
This all makes sense to me, and my first read of the tests suggests to me that they are covering everything they should. I'd like to take another look to be sure, or a second pair of eyes would work too!
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.
Awesome tests! 💯
Summary
This PR fixes a long standing bug on studio wherein copied contentnodes
content_id
were not changed upon modification of their underlying content.Particulary, exercises and file contentnodes were affected by this. Both the cases have been resolved by this PR.
Manual verification steps performed
content_id
of copied node is same as of the original node.content_id
should be changed!content_id
updated.Reviewer guidance
References
Closes #1055
Closes #3112
Unblocks #3725
Contributor's Checklist
PR process:
CHANGELOG
label been added to this PR. Note: items with this label will be added to the CHANGELOG at a later timedocs
label has been added if this introduces a change that needs to be updated in the user docs?requirements.txt
files also included in this PRStudio-specifc:
notranslate
class been added to elements that shouldn't be translated by Google Chrome's automatic translation feature (e.g. icons, user-generated text)pages
,components
, andlayouts
directories as described in the docsTesting:
Reviewer's Checklist
This section is for reviewers to fill out.
yarn
andpip
)