Run migrations to fix incorrect source fields of contentnodes #4720
Conversation
...ntcuration/kolibri_public/management/commands/rectify_incorrect_contentnode_source_fields.py
Outdated
Show resolved
Hide resolved
contentcuration/contentcuration/tests/test_rectify_source_field_migraiton_command.py
Outdated
Show resolved
Hide resolved
| source_channel_id=self.original_channel.id, | ||
| source_node_id=self.original_contentnode.node_id, | ||
| original_source_node_id=self.original_contentnode.node_id, | ||
| ) |
There was a problem hiding this comment.
One suggestion would be to use the same copy/import utilities that we use elsewhere, then override the things that shouldn't have changed, but it isn't a big deal. From my perspective, I like to do my best to ensure the tests are founded upon the app's behaviors as much as possible, because too many differences could cause the tests to pass when they shouldn't (under the typical behaviors of the app)
| print(node.id == base_channel.main_tree.id) | ||
| print("checking if the node is complete or not ", node.complete) | ||
| node.changed = False | ||
| # This should probably again change the changed=true but suprisingly it doesnot |
There was a problem hiding this comment.
In case this helps this feel less surprising: https://github.com/learningequality/studio/blob/unstable/contentcuration/contentcuration/models.py#L1831
We have an explicit exclude list of fields for which updates to them do not trigger change to be set as True.
There was a problem hiding this comment.
One question about how many times we're republishing :)
I am still not completely sure what to do about the identity of the user who does the publish, but an admin account seems easiest. We can query for that using [email protected] as the email address to look up the user id.
| if is_test: | ||
| publish_channel(user_id, base_channel.id) | ||
| else: | ||
| publish_channel("SOME ID", base_channel.id, base_channel.id) |
There was a problem hiding this comment.
My only uncertainty here is whether we should be running the publish as the user whose channel it is, or as some administrator.
The administrator means that we can always reliably run the publish as the same user, but does mean that it now appears that someone else has published the channel. The only other thing that comes to mind is that if we ran as the channel editor, they would receive an email indicating to them that their channel had been republished.
There was a problem hiding this comment.
The email draft that will be sent by imps team mentions that "WE" would be the one doing the change so the administrator seems more reliable option here?
There was a problem hiding this comment.
Altho this email is not sent to editors of channels which are not public, so they might be confused about whats happening. When as channel editor the sending email event can be avoided, as by default send_email=False is set for publish channel function?
...ntcuration/kolibri_public/management/commands/rectify_incorrect_contentnode_source_fields.py
Show resolved
Hide resolved
...ntcuration/kolibri_public/management/commands/rectify_incorrect_contentnode_source_fields.py
Outdated
Show resolved
Hide resolved
|
@akolson no issues observed while running the CWs. |
Summary
We filter out all the contentnodes whose source fields have been changed after being imported and reset them to their original values.
Manual verification steps performed
Reviewer guidance
How can a reviewer test these changes?
Recommended to run the query in hotfixes and verify the returned nodes. some of the fields to look at would be
last date of modification and verifying what source_node_fields have been changed.
Are there any risky areas that deserve extra testing?
If there is something that is missed migrations may be incorrect and may effect large number of nodes.
References
closes #4190
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)