-
Notifications
You must be signed in to change notification settings - Fork 254
Fixes folders or resources with missing title remain 'incomplete' after editing just the title #5539
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
base: hotfixes
Are you sure you want to change the base?
Conversation
contentcuration/contentcuration/frontend/channelEdit/components/ContentNodeValidator.vue
Show resolved
Hide resolved
akolson
left a comment
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.
Hi @AllanOXDi! I left a general comment about the implementation. Thanks
akolson
left a comment
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.
Change looks correct to me! Manual QA also checks out. Thanks @AllanOXDi
|
@AllanOXDi can we re-target this to hotfixes? Thanks |
590f5ea to
0047eca
Compare
|
@AllanOXDi, !t looks like the branch is from unstable so we might have to squash all the commits from unstable. I think cherypicking the commit instead should do the trick? or rebasing? |
|
working on that |
0047eca to
2378808
Compare
|
Fixed! |
akolson
left a comment
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.
Left a blocking comment. All else looks correct to me!
akolson
left a comment
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.
All should be good now. Thanks @AllanOXDi
General comment on the tests updates: I wonder if making the tests more liberal using the objectContaining function could increase the risk of masking bugs as it only asserts that certain keys exist, not that the whole payload is exactly what is expected (which I think is the intention of the previous test). Not a blocker but something to think about
|
Totally fair point ! |
|
I agree! However, the intent of the tests is lost--we've added the expect(updateContentNode).toHaveBeenCalledWith({
id: nodeId,
title: newTitle,
description: newDescription // or '',
checkComplete: true,
}); |
Summary
This PR fixes folders or resources with missing title remain 'incomplete' after editing just the title
Closes #5347
References
#5347
Before
2025-09-01_17-24-52.mp4
After
Screen.Recording.2025-11-06.at.18.45.55.mov
Reviewer guidance