[7095] Rename dialog offers the ability to edit a broken reference but will not allow the user to open the content form#4792
Open
jvega190 wants to merge 3 commits intocraftercms:developfrom
Open
Conversation
…part of current dependant list
WalkthroughTwo dialog components (RenameAssetDialog and RenameContentDialog) now subscribe to a host-to-host event bus, filter for content-related events matching dependency paths, and re-fetch dependant items on matches using a refs-based tracker to avoid stale closures. Changes
Sequence Diagram(s)sequenceDiagram
participant UserComponent as RenameDialogComponent
participant H2H as HostToHostBus
participant Fetch as fetchDependant
participant EventSource as ExternalContentChange
EventSource->>H2H: emit contentEvent(targetPath)
H2H->>UserComponent: deliver filtered contentEvent
UserComponent->>UserComponent: check event.targetPath vs refs.dependantItems
alt match found
UserComponent->>Fetch: invoke fetchDependant()
Fetch-->>UserComponent: updated dependantItems
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@ui/app/src/components/RenameContentDialog/RenameContentDialog.tsx`:
- Around line 70-89: The filter in the subscription uses
refs.current.dependantItems.some(...) which can be null before fetchDependant
resolves and will throw; update the predicate inside the getHostToHostBus()
subscription (the filter that compares e.type === contentEvent.type) to safely
handle null by using optional chaining or nullish coalescing, e.g. replace
refs.current.dependantItems.some(...) with
refs.current.dependantItems?.some(...) ?? false so the check returns false when
dependantItems is null (alternatively, initialize dependantItems to [] where
it's declared if that semantic change is acceptable).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
craftercms/craftercms#7095
Summary by CodeRabbit