Skip to content

Conversation

@AllanOXDi
Copy link
Member

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

  1. Sign in to Studio
  2. Go to a channel and add a folder or a resource without a title
  3. Edit just the title of the folder or resource and observe that it's still marked as incomplete

@AllanOXDi AllanOXDi added this to the Studio: Q4 Patches milestone Nov 6, 2025
Copy link
Member

@akolson akolson left a 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

Copy link
Member

@akolson akolson left a 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

@akolson
Copy link
Member

akolson commented Nov 20, 2025

@AllanOXDi can we re-target this to hotfixes? Thanks

@AllanOXDi AllanOXDi changed the base branch from unstable to hotfixes November 20, 2025 12:49
@AllanOXDi AllanOXDi changed the base branch from hotfixes to unstable November 20, 2025 12:49
@AllanOXDi AllanOXDi force-pushed the fixes-resources-remaining-incomplete-after-editing branch from 590f5ea to 0047eca Compare November 20, 2025 13:04
@AllanOXDi AllanOXDi changed the base branch from unstable to hotfixes November 20, 2025 13:09
@AllanOXDi AllanOXDi changed the base branch from hotfixes to unstable November 20, 2025 13:59
@AllanOXDi AllanOXDi changed the base branch from unstable to hotfixes November 20, 2025 14:00
@akolson
Copy link
Member

akolson commented Nov 20, 2025

@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?

@AllanOXDi
Copy link
Member Author

working on that

@AllanOXDi AllanOXDi force-pushed the fixes-resources-remaining-incomplete-after-editing branch from 0047eca to 2378808 Compare November 20, 2025 16:59
@AllanOXDi
Copy link
Member Author

Fixed!

Copy link
Member

@akolson akolson left a 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!

Copy link
Member

@akolson akolson left a 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

@AllanOXDi
Copy link
Member Author

Totally fair point ! objectContaining is more permissive.
In this case though, that’s intentional: the test’s job is just to ensure the important fields (id, title, description ) are sent correctly, not to lock down the entire payload shape. Using objectContaining keeps the test focused on behavior rather than structure, and avoids breaking anytime we add non-essential fields. So it’s still a safe assertion for the contract we care about here?

@AllanOXDi AllanOXDi requested a review from akolson November 20, 2025 17:55
@akolson
Copy link
Member

akolson commented Nov 21, 2025

I agree! However, the intent of the tests is lost--we've added the checkComplete to the payload in the updateContentNode action, so using objectContaining would bypass the added field. I think the test failures could have been resolved by just updating the tests to have the checkComplete: true,. I suspect the two tests are failing because they default checkComplete to undefined thus the difference in the expected payload.

expect(updateContentNode).toHaveBeenCalledWith({
  id: nodeId,
  title: newTitle,
  description: newDescription // or '',
  checkComplete: true,
});

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Folders or resources with missing title remain 'incomplete' after editing just the title

2 participants