Skip to content

Conversation

@aleksandernsilva
Copy link
Contributor

@aleksandernsilva aleksandernsilva commented Sep 24, 2025

Proposed changes (including videos or screenshots)

This PR fixes an issue where disconnected phone numbers remained available as outbound senders due to stale cached data. This is resolved by clearing the outbound providers query cache when the wizard unmounts, ensuring the sender list is fresh each time.

Issue(s)

CTZ-338
CTZ-346

Steps to test or reproduce

  • Workspace with outbound-messaging add-on
  • Configure WhatsApp 360 app
  • Connect a phone number
  • Create new > Outbound message
  • Select a contact, then a channel
  • Number should be available as sender
  • Close outbound message modal
  • Disconnect the number from whatsapp
  • Create new > Outbound message
  • Select a contact, then a channel
  • Error message should be displayed informing that there are no phone number available for the selected channel

Further comments

Summary by CodeRabbit

  • Bug Fixes

    • Prevents stale phone numbers from appearing when composing outbound messages.
    • Ensures provider details refresh correctly, reducing outdated information.
  • Performance and Reliability

    • Short-term caching improves responsiveness while keeping outbound provider data fresh.
    • Automatically clears related cached data when leaving the outbound messaging flow to avoid stale state.
  • Chores

    • Added patch release entry documenting the fix.

@aleksandernsilva aleksandernsilva added this to the 7.11.0 milestone Sep 24, 2025
@dionisio-bot
Copy link
Contributor

dionisio-bot bot commented Sep 24, 2025

Looks like this PR is not ready to merge, because of the following issues:

  • This PR is missing the 'stat: QA assured' label

Please fix the issues and try again

If you have any trouble, please check the PR guidelines

@changeset-bot
Copy link

changeset-bot bot commented Sep 24, 2025

🦋 Changeset detected

Latest commit: b6c38a5

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 40 packages
Name Type
@rocket.chat/meteor Patch
@rocket.chat/core-typings Patch
@rocket.chat/rest-typings Patch
@rocket.chat/uikit-playground Patch
@rocket.chat/api-client Patch
@rocket.chat/apps Patch
@rocket.chat/core-services Patch
@rocket.chat/cron Patch
@rocket.chat/ddp-client Patch
@rocket.chat/freeswitch Patch
@rocket.chat/fuselage-ui-kit Patch
@rocket.chat/gazzodown Patch
@rocket.chat/http-router Patch
@rocket.chat/livechat Patch
@rocket.chat/model-typings Patch
@rocket.chat/ui-avatar Patch
@rocket.chat/ui-client Patch
@rocket.chat/ui-contexts Patch
@rocket.chat/web-ui-registration Patch
@rocket.chat/account-service Patch
@rocket.chat/authorization-service Patch
@rocket.chat/ddp-streamer Patch
@rocket.chat/omnichannel-transcript Patch
@rocket.chat/presence-service Patch
@rocket.chat/queue-worker Patch
@rocket.chat/stream-hub-service Patch
@rocket.chat/license Patch
@rocket.chat/media-calls Patch
@rocket.chat/omnichannel-services Patch
@rocket.chat/pdf-worker Patch
@rocket.chat/presence Patch
rocketchat-services Patch
@rocket.chat/models Patch
@rocket.chat/network-broker Patch
@rocket.chat/omni-core-ee Patch
@rocket.chat/mock-providers Patch
@rocket.chat/ui-video-conf Patch
@rocket.chat/ui-voip Patch
@rocket.chat/instance-status Patch
@rocket.chat/omni-core Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@aleksandernsilva aleksandernsilva removed this from the 7.11.0 milestone Sep 24, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 24, 2025

Walkthrough

Implements cache lifecycle adjustments for outbound providers: adds unmount-time query cache removal in OutboundMessageWizard, changes useOutboundProvidersList default staleTime to 5 minutes and removes gcTime, and refactors omnichannel query keys for outbound providers and metadata. Adds a changeset noting a patch for stale phone numbers.

Changes

Cohort / File(s) Summary
Changeset metadata
/.changeset/few-flowers-bow.md
Adds a patch changeset for @rocket.chat/meteor describing a fix for stale outbound phone numbers; no code.
Wizard unmount cache cleanup
/apps/meteor/client/components/Omnichannel/OutboundMessage/components/OutboundMessageWizard/OutboundMessageWizard.tsx
Imports useQueryClient and omnichannelQueryKeys; on unmount, removes cached outbound provider queries via queryClient.removeQueries(omnichannelQueryKeys.outboundProviders()).
Outbound providers list hook caching
/apps/meteor/client/components/Omnichannel/OutboundMessage/hooks/useOutboundProvidersList.ts
Sets default staleTime to 5 minutes (from 0); removes support for gcTime option and its use in useQuery.
Omnichannel query keys refactor
/apps/meteor/client/lib/queryKeys.ts
Updates omnichannelQueryKeys.outboundProviders to accept optional { type } filter; outboundProviderMetadata now derives base path from outboundProviders() and appends providerId.

Sequence Diagram(s)

sequenceDiagram
  participant U as User
  participant W as OutboundMessageWizard
  participant Q as React Query (useOutboundProvidersList)
  participant S as Server

  U->>W: Open Outbound Message Wizard
  W->>Q: useQuery(outboundProviders[filter])
  Q->>S: Fetch providers
  S-->>Q: Providers list
  Q-->>W: Cache + provide data

  Note over W,Q: Wizard used with cached providers (staleTime = 5m)

  U-->>W: Navigate away / close wizard
  W->>Q: removeQueries(outboundProviders())

  U->>W: Reopen wizard later
  W->>Q: useQuery(outboundProviders[filter])
  alt Cache missed (removed)
    Q->>S: Refetch providers
    S-->>Q: Fresh providers list
  else Cache hit (within 5m, other views)
    Q-->>W: Serve cached, background update as needed
  end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested labels

stat: ready to merge, stat: QA assured

Suggested reviewers

  • lucas-a-pelegrino
  • MartinSchoeler

Poem

A hop, a cache, a tidy sweep—
I nudge stale numbers off to sleep.
On unmount, whiskers clear the way,
Fresh keys await another day.
Five-minute breeze, no ghosts to see—
Outbound paths now bunny-free. 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Out of Scope Changes Check ⚠️ Warning In addition to the cache-clearing logic required by CTZ-338, the PR modifies the default staleTime in useOutboundProvidersList and updates the signatures of outboundProviders and outboundProviderMetadata in queryKeys, neither of which are referenced in the issue objectives. These changes pertain to hook defaults and query key refactoring rather than fixing stale phone number display, making them unrelated to the bug fix scope. Please separate the staleTime default adjustment and queryKeys refactoring into a different PR so that this change focuses solely on the cache invalidation fix for CTZ-338.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues Check ✅ Passed The PR implements cache invalidation for outboundProviders by clearing the React Query cache on component unmount, which directly addresses CTZ-338’s requirement that disconnected WhatsApp numbers no longer appear in the outbound message form. The implementation ensures that stale cached data is removed and the sender list is refreshed on each mount, satisfying the linked issue’s expected behavior.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title succinctly captures the core issue addressed by the pull request—stale phone numbers persisting in the outbound message flow—and clearly indicates that this is a regression fix. It is concise, specific, and directly reflects the main change without extraneous details.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch regression/outbound-app-disconnect

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


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.

@codecov
Copy link

codecov bot commented Sep 24, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 67.37%. Comparing base (40e2421) to head (b6c38a5).
⚠️ Report is 24 commits behind head on release-7.11.0.

Additional details and impacted files

Impacted file tree graph

@@                Coverage Diff                 @@
##           release-7.11.0   #37053      +/-   ##
==================================================
+ Coverage           67.29%   67.37%   +0.08%     
==================================================
  Files                3337     3328       -9     
  Lines              113326   113384      +58     
  Branches            20576    20641      +65     
==================================================
+ Hits                76261    76391     +130     
+ Misses              34450    34387      -63     
+ Partials             2615     2606       -9     
Flag Coverage Δ
e2e 57.28% <0.00%> (+0.28%) ⬆️
unit 71.13% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@aleksandernsilva aleksandernsilva marked this pull request as ready for review September 26, 2025 14:35
@aleksandernsilva aleksandernsilva requested a review from a team as a code owner September 26, 2025 14:35
Copy link
Contributor

@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

🧹 Nitpick comments (3)
.changeset/few-flowers-bow.md (1)

5-5: Polish the changeset entry for clarity.

Add a period and the issue reference for traceability.

-Fixes stale phone numbers being available for outbound message
+Fixes stale phone numbers being available for outbound messages. (CTZ-338)
apps/meteor/client/lib/queryKeys.ts (1)

79-84: LGTM: key shape enables bulk matching for providers and metadata.

  • outboundProviders now supports an optional filter, which is consistent with other keys.
  • outboundProviderMetadata derives from the base path, so base-key invalidation/removal hits both lists and metadata.

Minor: for naming consistency with contacts, consider renaming the parameter from filter to params in a follow-up. No functional change required.

apps/meteor/client/components/Omnichannel/OutboundMessage/components/OutboundMessageWizard/OutboundMessageWizard.tsx (1)

69-75: Replace removeQueries with invalidateQueries in cleanup
invalidateQueries marks providers and metadata stale (without evicting active observers) and will refetch on next mount.

 useEffect(
   () => () => {
-    // Clear cached providers and metadata on unmount to avoid stale data
-    void queryClient.removeQueries({ queryKey: omnichannelQueryKeys.outboundProviders() });
+    // Mark providers and metadata as stale on unmount to avoid stale data
+    queryClient.invalidateQueries({ queryKey: omnichannelQueryKeys.outboundProviders(), exact: false });
   },
   [queryClient],
 );

Optional: cancel in-flight fetches:

void queryClient.cancelQueries({ queryKey: omnichannelQueryKeys.outboundProviders(), exact: false });
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 40e2421 and b6c38a5.

📒 Files selected for processing (4)
  • .changeset/few-flowers-bow.md (1 hunks)
  • apps/meteor/client/components/Omnichannel/OutboundMessage/components/OutboundMessageWizard/OutboundMessageWizard.tsx (4 hunks)
  • apps/meteor/client/components/Omnichannel/OutboundMessage/hooks/useOutboundProvidersList.ts (1 hunks)
  • apps/meteor/client/lib/queryKeys.ts (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
apps/meteor/client/components/Omnichannel/OutboundMessage/components/OutboundMessageWizard/OutboundMessageWizard.tsx (1)
apps/meteor/client/lib/queryKeys.ts (1)
  • omnichannelQueryKeys (30-84)
apps/meteor/client/lib/queryKeys.ts (1)
packages/core-typings/src/omnichannel/outbound.ts (1)
  • IOutboundProvider (134-139)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: 🚀 Notify external services - draft
  • GitHub Check: CodeQL-Build
  • GitHub Check: CodeQL-Build
🔇 Additional comments (2)
apps/meteor/client/components/Omnichannel/OutboundMessage/components/OutboundMessageWizard/OutboundMessageWizard.tsx (1)

6-6: LGTM: correct query client usage and key imports.

Imports and client instantiation look good and align with the new query key shape.

Also applies to: 16-16, 32-32

apps/meteor/client/components/Omnichannel/OutboundMessage/hooks/useOutboundProvidersList.ts (1)

17-17: LGTM: sensible default staleTime (5 min).

Balances network with freshness, especially with unmount invalidation.

@aleksandernsilva aleksandernsilva changed the base branch from develop to release-7.11.0 September 29, 2025 12:54
@aleksandernsilva aleksandernsilva added this to the 7.11.0 milestone Sep 29, 2025
@aleksandernsilva aleksandernsilva changed the title fix: Stale phone numbers being available for outbound message regression: Stale phone numbers being available for outbound message Sep 29, 2025
@tassoevan tassoevan merged commit 3f22549 into release-7.11.0 Sep 30, 2025
91 of 93 checks passed
@tassoevan tassoevan deleted the regression/outbound-app-disconnect branch September 30, 2025 15:03
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.

3 participants