Skip to content

Conversation

@jvega190
Copy link
Member

@jvega190 jvega190 commented Feb 10, 2026

craftercms/craftercms#7095

Summary by CodeRabbit

  • Bug Fixes
    • Rename asset and content dialogs now auto-refresh when related items are changed elsewhere in the app.
    • Dependency lists in rename dialogs update in real-time to show current relationships without manual refresh.
    • Improved reliability of dependency updates to ensure displayed information stays current after external edits.

@coderabbitai
Copy link

coderabbitai bot commented Feb 10, 2026

Walkthrough

Two 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

Cohort / File(s) Summary
Event-driven dependency reactivity
ui/app/src/components/RenameAssetDialog/RenameAssetDialog.tsx, ui/app/src/components/RenameContentDialog/RenameContentDialog.tsx
Added host-to-host subscriptions that filter contentEvents and match targetPath against current dependantItems paths; on match the components re-run fetchDependant. Introduced useUpdateRefs to expose latest dependantItems to the event handler and added cleanup to unsubscribe previous and host-to-host listeners.

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
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description check ⚠️ Warning The description only contains a GitHub issue link without expanding on the problem, solution, or changes made. It lacks the required structure and meaningful content. Expand the description to explain the issue, the solution implemented, and how the code changes address the problem referenced in the ticket.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly references the ticket [7095] and accurately describes the core issue being addressed: rename dialog behavior with broken references.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

@jvega190 jvega190 marked this pull request as ready for review February 10, 2026 22:52
@jvega190 jvega190 requested a review from rart as a code owner February 10, 2026 22:52
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.

1 participant