Ensure contentnode.mark_complete() happens when File updated
#4697
Conversation
|
|
||
| if results.contentnode: | ||
| results.contentnode.refresh_from_db() | ||
| if not len(results.contentnode.mark_complete()): |
There was a problem hiding this comment.
Should we be handling this any differently? Basically mark_complete returns List<errors> when it is incomplete, is there anything I should do here w/ the information if there are issues?
| generate_update_event( | ||
| results.contentnode.id, | ||
| CONTENTNODE, | ||
| {"complete": True}, | ||
| channel_id=results.contentnode.get_channel_id(), | ||
| ), |
There was a problem hiding this comment.
Just basically copied this from the content node serializer _ensure_complete method and tweaked it to fit in here. Is there anything I've missed here that should be done/handled differently?
| ) | ||
| list_serializer_class = BulkListSerializer | ||
|
|
||
|
|
||
| def retrieve_storage_url(item): | ||
| """ Get the file_on_disk url """ | ||
| """Get the file_on_disk url""" |
There was a problem hiding this comment.
Hm... seems my formatter got aggressive here?
There was a problem hiding this comment.
I think that this was applied by pre-commit - I just accepted whatever pre-commit did thinking that it rewrote things related to my particular changes and didn't realize it was linting the whole file.
|
|
||
| self.assertEqual(complete_except_no_file.complete, False) | ||
|
|
||
| models.Change.create_change = MagicMock() |
There was a problem hiding this comment.
You should do this as a mock.patch call:
with mock.patch("contentcuration.viewsets.file.Change") as ChangeMock:
self.sync_changes(
[generate_update_event(file.id, FILE, {"contentnode": complete_except_no_file.id}, channel_id=self.channel.id)],
)
self.assertTrue(ChangeMock.create_change.called)
FileViewset mixes in UpdateModelMixin in place of BulkUpdateMixin to resolve primary issue, but this results in needing to call on_update directly to avoid duplicate content_ids
nucleogenesis
force-pushed
the
fix--contentnode-incompleted-by-file-update
branch
from
2e65bc7 to
84520f0
Compare
Summary
Description of the change(s) you made
mark_complete()on the File's content nodeUpdateModelMixinin theFileViewSetensuring that the File instance is saved, resulting in contentnode having its relation to the File updatedScreenshots (if applicable)
Does this introduce any tech-debt items?
Reviewer guidance
How can a reviewer test these changes?
At a high level, the issue was that a "Content Node" is not complete without a file - but the order of operations goes that we first create the contentnode, then upload the file and associate it with the contentnode. So, until we perform that second bit of the operation the contentnode is "incomplete" without the file.
Here, we now check that the file being added makes the node complete and if it does, we update the client accordingly so that the user doesn't see the incomplete flag unnecessarily.
References
Closes #4644
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)