feat: upgrade Apollo Client 3 to 4 - #4144
Draft
akanshaaa19 wants to merge 24 commits into
Draft
akanshaaa19 wants to merge 24 commits into
akanshaaa19 wants to merge 24 commits into
Conversation
…tch 3) Follows Apollo's deprecation of onCompleted/onError on query and mutation hooks. Migrates FormLayout/List consumers and templates to await the mutate/query call directly and derive query state via useEffect. Files: TemplateOptions, RaiseToGupShup, FlowEditor, FlowTranslation, InteractiveMessage, TranslateButton, KnowledgeBaseOptions (AssistantOptions), CollectionList. PromptGeneratorModal (CreateAssistant), AssistantList, and Organization were already compliant. Closes #3953
…tch 4) Follows Apollo's deprecation of onCompleted/onError on query and mutation hooks, and on per-call mutate() options. Migrates Settings and WA Groups consumers to await the mutate/query call directly and derive query state via useEffect. Files: MyAccount, Billing, WhatsAppFormList, Configure, VersionHistory, GroupCollectionList, WaPollsList, GroupDetails. Organization and WaManagedPhones were already compliant. Closes #3954
…tch 5) Follows Apollo's deprecation of onCompleted/onError on query and mutation hooks. Migrates the remaining consumers to await the mutate/query call directly and derive query state via useEffect. Files: NotificationList, BlockContactList, ContactFieldList, AdminContactManagement, UploadContactsDialog, SavedSearchToolbar, OrganizationList, SheetIntegrationList, BulkAction, ExportTicket, ExportConsulting, CollectionContactList. GroupMessageSubscription and WalletBalance were flagged in the issue but only use subscribeToMore's own onError (a distinct, non-deprecated Apollo API) — no changes needed there. The "Upgrade Apollo Client to target version" checklist item is a separate major-version migration (3.x -> 4.x) and is out of scope for this callback-removal PR. Closes #3955
Adds tests for previously-untested success/error branches introduced by replacing onCompleted/onError with async/await (getFreeFlow success path, export/reset/publish flow error and success paths, file upload success and failure, auto-translate success/trim-warning, createKnowledgeBase success). Also fixes two pre-existing test-suite footguns surfaced while writing these: - FlowTranslation.test.tsx and KnowledgeBaseOptions.test.tsx held module-level Apollo/notification spies that leaked call history across tests (or got silently detached by an earlier vi.restoreAllMocks()), letting several tests pass without actually exercising the code they claimed to cover. - InteractiveMessage.test.tsx's file-upload test never triggered a real file-change event; replaced with a reliable direct input-change trigger. - FlowTranslation.tsx's ImportButton flow is now exercised via a mocked ImportButton instead of relying on FileReader completion in jsdom. Simplifies a few call sites that were checking `data?.` after mutations that either resolve with data or throw (never resolve with `data` undefined), matching the original onCompleted destructuring and removing unreachable branches.
…tch 4) Adds tests for previously-untested success/error branches introduced by replacing onCompleted/onError with async/await (password update success/failure, subscription 3D-secure failure and unexpected mutation failure, customer-portal redirect, form activate success/failure, poll delete failure, form-revert failure, form-publish failure). Also fixes a real regression from the original refactor: the "pending" state's "Visit Stripe portal" button still called the raw getCustomerPortal lazy query instead of the new visitCustomerPortal handler, so window.open never fired for that button. Restructures the Billing test's Stripe confirmCardSetup mock into a shared vi.fn() so individual tests can override its resolved value (previously it was a fresh closure per render, making the 3D-secure failure path untestable), and strengthens two pre-existing weak tests (`open customer portal`, `subscription status is already in pending state`) that clicked buttons but asserted nothing.
Add tests for previously-unexercised error/edge paths introduced by the onCompleted/onError -> await+try/catch migration: network-error handling in ContactFieldList, AdminContactManagement, UploadContactsDialog, NotificationList, OrganizationList, and SheetIntegrationList. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
stripe is already narrowed non-null by the early return a few lines above (if (!stripe || !elements) return;), so the inner if (stripe) branch was always true and flagged by static analysis as dead code. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- InteractiveMessage.tsx: fix uploadingFile flag stuck true after a successful upload (missing finally reset), surface real errors via setErrorMessage, translate success message - FlowTranslation.tsx: check importFlowLocalization.success before closing the import dialog/reloading the editor; report exceptions via setErrorMessage - CollectionList.tsx: fix "occured" -> "occurred" typo - RaiseToGupShup.tsx: wrap Yup validation messages with t() - TranslateButton.tsx: wrap success notifications with t() - FlowEditor.test.tsx: actually await the findByText lookup instead of asserting on an unawaited promise - InteractiveMessage.test.tsx: strengthen upload success/failure tests to assert on real UI state, not just that a notification fired; fix a mismatched mock that was silently falling through to a generic "no mock" rejection Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- MyAccount.tsx: apply the interface-language change only after the update-language mutation actually succeeds, instead of optimistically switching i18n/cache and then finding out the server rejected it - Billing.tsx: pass 'noopener' to the customer-portal window.open; use the just-created paymentMethod.id for 3D-secure confirmation instead of the stale paymentMethodId state (set one render too late); drop the now-dead paymentMethodId state entirely - GroupCollectionList.tsx: wrap the group-removal mutation in try/catch so a rejected mutation is surfaced via setErrorMessage instead of silently closing the dialog with no feedback; only close the dialog and reset selection on success so the user can retry on failure - Configure.tsx / VersionHistory.tsx / WhatsAppFormList.tsx: check the `errors` payload on publish/save/revert/activate mutations before treating them as successful (these mutations return errors inside resolved data, not as thrown exceptions) - mocks/WhatsAppForm.tsx: fix the activate-success mock returning 'INACTIVE' instead of 'PUBLISHED', which let a broken response shape pass as success Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- BlockContactList.tsx, CollectionContactList.tsx: wrap the unblock/remove-from-collection mutations in try/catch and route failures through setErrorMessage instead of letting a rejected mutation silently fall through - OrganizationList.tsx: check updateOrganizationStatus.errors before showing a success toast, and report rejected mutations - BulkAction.tsx: check updateTicketStatusBasedOnTopic.success before closing the dialog, and report rejected mutations - AdminContactManagement.tsx: fix moveContactsErrors truthy-on-empty- array check (an empty errors array read as a failure and skipped setShowStatus/left the upload button stuck loading) - UploadContactsDialog.tsx: fix reading errors from data.errors[0] instead of the actual data.importContacts.errors, which made a returned import error throw and fall into the generic catch instead of showing the real message; clear uploadingContacts on that path - NotificationList.tsx: convert the contact-upload-report download to async/await + try/catch (was still using .then/.catch) - ContactFieldList.tsx, NotificationList.tsx: wrap remaining raw notification strings with t() - Strengthen two tests (ContactFieldList, UploadContactsDialog) that were asserting on state already true before the mutation resolved, so a regression in the failure handler wouldn't have failed them Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- MyAccount.tsx: don't let a successful language-change mutation reset an in-progress password-change flow. setShowOTPButton(true) had moved into the shared success helper in the previous fix, so changing language while the OTP/password fields were open would incorrectly hide them. Moved it back into the password-specific success path only. - Billing.tsx: visitCustomerPortal ignored the lazy query's error field entirely, giving no feedback and never opening the portal on failure; now checks `error` and `data?.customerPortal?.url` and reports failures via setNotification. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Pulls the async afterImport callbacks out of the ImportButton JSX in FlowTranslation and TranslateButton into handleImport functions, matching the sibling handler pattern already used in both files. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
# Conflicts: # src/i18n/en/en.json # src/i18n/hi/hi.json
# Conflicts: # src/i18n/en/en.json # src/i18n/hi/hi.json
# Conflicts: # src/i18n/en/en.json # src/i18n/hi/hi.json
Ran @apollo/client-codemod-migrate-3-to-4's imports+links codemods against .ts and .tsx separately (each needs its own --parser flag, per the migration guide). Mechanically moves React hook imports to @apollo/client/react, MockedProvider to @apollo/client/testing/react, and link creator functions to their class equivalents where matched. Followed by yarn format to normalize the codemod's double-quote output to this repo's single-quote convention. Does not yet compile — see follow-up commits for the remaining manual work (addTypename removal, GraphQL result typing, auth link rewrite).
Replaces three unmaintained third-party link packages with native v4 implementations, since none of them support v4 (apollo-link-token-refresh imports fromPromise from @apollo/client/core, which v4 removed entirely - confirmed as a hard build failure, not just a type mismatch): - apollo-link-token-refresh -> a plain ApolloLink that checks checkAuthStatusService() and awaits renewAuthToken() before forwarding. Much simpler than the original's queue-based implementation because services/TokenRenewalService already deduplicates concurrent renewal calls into a single in-flight promise and already persists the refreshed session - this link only needs to wait for it. - @apollo/link-context's setContext / @apollo/link-error's onError -> native SetContextLink / ErrorLink classes. ErrorLink's callback now receives a single unified (CombinedGraphQLErrors.is(error) to distinguish GraphQL errors from network errors) instead of separate graphQLErrors/networkError properties. - apollo-absinthe-upload-link -> a native ApolloLink using extractFiles (ported verbatim from that package - it's framework-agnostic, no Apollo dependency) and the browser's fetch() for the multipart request, since the original package's own fetch fallback used rxjs/ajax and imported now-removed @apollo/client/core internals. Removes all four now-unused packages from package.json.
…ks, bugs)
Follow-up to the apolloclient.ts rewrite and codemod migration: closes out
the remaining tsc and test failures left by the v3->v4 jump.
- config/gql.ts wraps `gql` as TypedDocumentNode<any, any> so untyped
documents (none of this codebase's queries use codegen) don't collapse
useQuery/useMutation `data` to `{}` under v4's stricter default generics;
swapped the ~66 gql imports in src/graphql to use it.
- Configured LocalState on the real Apollo client and via MockedProvider's
defaultProps in setupTests.ts - v4 throws on any query with @client
fields (NOTIFICATION, ERROR_MESSAGE, SEARCH_OFFSET, SCROLL_HEIGHT) once
it misses the cache, unless LocalState is present.
- Set MockLink.defaultOptions.delay = 1 globally - v4's MockedProvider
defaults to a randomized "realistic" network delay instead of resolving
on the next macrotask, which broke synchronous assertions across ~30
test files.
- Fixed a real bug this exposed: MockedResponse has no `variableMatcher`
field (silently ignored, never real Apollo API) across 17 files - moved
the matcher into `request.variables` as v4 actually expects.
- Migrated remaining onCompleted/onError usages (useQuery/useLazyQuery
dropped both in v4; useMutation keeps them) and moved useLazyQuery's
`variables` from hook options to the execute() call.
Also fixed genuine bugs surfaced along the way, not just migration noise:
- App.tsx: v4 skips the entire link chain (including refreshTokenLink) for
queries that are 100% @client fields, so the proactive token-refresh
check that used to piggyback on ErrorHandler's query needed to move to
its own mount effect.
- Billing.tsx, FlowTranslation.tsx, CollectionList.tsx: `const { data,
error } = await mutate()` assumed GraphQL errors resolve with an
`error` field; v4's default errorPolicy rejects the promise instead, so
these silently swallowed failures until wrapped in try/catch.
- common/notification.ts: setErrorMessage read error.networkError/
.graphQLErrors, which no longer exist now that ApolloError is gone in
favor of a single `error` - now checks CombinedGraphQLErrors.is(error).
- List.tsx: the row-fetch effect depended on the `filters` prop by
reference; several callers pass a fresh object literal every render,
which under v4's stricter fetchMore/cache-only rules turned into
runaway refetches that exhausted test mock buffers. Now keyed on
JSON.stringify(filters).
- ChatMessages.tsx: switched its search query from cache-only to
cache-first, since v4 no longer allows fetchMore on a cache-only query.
Net result: tsc --noEmit clean (was 892 errors mid-migration), full
vitest suite green (1828/1828 non-skipped, stable across repeat runs).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
Glific
 |
||||||||||||||||||||||||||||||||||||||||||||||
| Project |
Glific
|
| Branch Review |
apollo-client-v4-upgrade
|
| Run status |
|
| Run duration | 09m 04s |
| Commit |
|
| Committer | Akansha Sakhre |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
 |
2
|
 |
1
|
 |
0
|
 |
0
|
 |
37
|
| View all changes introduced in this branch ↗︎ | |
Tests for review
| Test | Artifacts | |
|---|---|---|
| Interactive message quick reply > should edit quick reply |
Test Replay
Screenshots
|
|
| Test | Artifacts | |
|---|---|---|
| Role - Staff - Collection > should remove member from collection |
Test Replay
Screenshots
|
|
| Test | Artifacts | |
|---|---|---|
| Role - Staff - Collection > should add member to collection |
Test Replay
Screenshots
|
|
|
🚀 Deployed on https://deploy-preview-4144--glific-frontend.netlify.app |
The advanced-search date fields render asynchronously (behind a query that resolves instantly on a fast local machine but not necessarily on a slower/contended CI runner), and these tests read them with a single non-retrying queryByTestId right after the dialog's title appears. On CI this raced: the field lookup returned null, the conditional fill was silently skipped, and the resulting empty date field failed the submit without ever calling navigate - all 3 tests failed consistently in CI while passing locally every time. Switched to findByTestId (retries until the field exists) and bumped the final navigate-assertion waitFor to 5000ms to absorb the remaining multi-hop async chain (filter submit -> refetch -> render -> navigate) under CI's slower scheduling. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ients clientForContact/clientForCollection/clientForGroup construct their own ApolloClient directly (not via MockedProvider), so the MockedProvider defaultProps.localState fix in setupTests.ts doesn't reach them. Apollo Client 4 throws once a query with @client fields (SCROLL_HEIGHT, in ConversationList) misses the cache without LocalState configured - didn't fail any assertion here (the exception is thrown async, after the relevant test already completed), but it printed an uncaught Invariant Violation on every run. Same fix already applied to GroupInterface.test.tsx. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
A bare vi.fn() returns undefined, and Apollo Client 4's HttpLink calls .then() on the fetch call directly - any query that isn't fully mocked and genuinely reaches this (e.g. ConversationList.test.tsx's raw ApolloClient + a real fetchMore() call with no matching mock) threw a synchronous TypeError buried in rxjs/Apollo internals instead of a normal catchable async rejection. Returning a rejected promise instead gives a clear "fetch is not mocked in tests" error when this happens. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The date-range filter used dayjs(...).utc() before formatting, which shifts the calendar date backward for any timezone ahead of UTC (e.g. IST) - so the query variables sent to the backend never matched what the CI runner (UTC) actually computed. Drop the erroneous .utc() call and correct the mocks to the un-shifted dates. Also add missing .catch() handlers to several imperative Apollo promise chains in ChatMessages/ConversationList/AskGlific, and filter Apollo Client 4's benign "AbortError"/"QueryManager stopped" rejection that fires whenever an ObservableQuery is torn down before test cleanup - both were surfacing as unhandled-rejection test failures. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #4144 +/- ##
==========================================
+ Coverage 82.91% 83.68% +0.77%
==========================================
Files 368 370 +2
Lines 16171 16317 +146
Branches 3865 3900 +35
==========================================
+ Hits 13408 13655 +247
+ Misses 1657 1555 -102
- Partials 1106 1107 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
akanshaaa19
marked this pull request as draft
August 18, 2026 04:54
Codecov's patch-coverage gate required 82.91% but the migration diff sat at 78.42%, with 118 changed lines across ~20 files untested - mostly the rewritten apolloclient.ts link chain, extractFiles.ts (the new multipart-upload helper), and error/edge-case branches in several containers that changed shape under Apollo v4's stricter mutation error handling. Adds src/config/apolloclient.test.ts and src/config/extractFiles.test.ts (neither had coverage before), plus targeted error-path tests and mock factories across ConversationList, ChatMessages, Configure, Billing, and eight smaller containers. One test-hygiene fix along the way: Billing.test.tsx had a test that returned before awaiting its success UI, leaving a shared mock's in-flight call to race the next test. A handful of lines (ConversationList.tsx:286-287,455 and ChatMessages.tsx:716) remain uncovered - traced and confirmed as genuinely unreachable dead code rather than missing tests. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
retryIf treated any error without a statusCode of 500/400/401 as retryable, which includes CombinedGraphQLErrors - a normal, successful HTTP response carrying a GraphQL-level error like "name already exists" or "contact is blocked". Retrying a deterministic error just repeats the same failure up to 5 times with exponential backoff (~9s worst case) before it ever reaches the UI. This was masked in the unit suite (no wall-clock assertion budget) but surfaced in CI's Cypress run as widespread "element/toast never appeared within 8000ms" failures across several unrelated specs (Flow, FlowEditor, StaffManagement, Search, Collection) - the retries were eating into Cypress's default assertion timeout. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This branch was previously deployed
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
1 participant
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.
Summary
Upgrades
@apollo/clientfrom 3.x to 4.2.12, closing out issue #3688's final checklist item. Builds on the already-merged onCompleted/onError cleanup batches (#4132, #4133, #4134).src/config/apolloclient.ts's link chain natively (v4 dropped support forapollo-link-token-refreshandapollo-absinthe-upload-link, both unmaintained with no v4-compatible releases): native upload link (portedextractFilesfromapollo-absinthe-upload-link), native token-refresh link built on the existingTokenRenewalManagersingleton, and v4's ownSetContextLink/ErrorLinkclasses in place of@apollo/link-context/@apollo/link-error.src/config/gql.tswrapsgqlasTypedDocumentNode<any, any>so untyped GraphQL documents (this codebase has no codegen) don't collapseuseQuery/useMutationresult types to{}under v4's stricter default generics — restores v3's actual (implicitany) behavior rather than introducing new type safety. Proper per-query typed documents would be a separate, much larger initiative (codegen + schema introspection) and is intentionally out of scope here.LocalState(both on the real Apollo client and viaMockedProvider'sdefaultPropsin test setup) — v4 throws once a query containing@clientfields (NOTIFICATION,ERROR_MESSAGE,SEARCH_OFFSET,SCROLL_HEIGHT) misses the cache, unlessLocalStateis present. This app never used local resolvers (writes@clientfields directly viacache.writeQuery), so a resolver-lessLocalStateis enough.onCompleted/onErrorusages that batches 3-5 didn't cover (v4 dropped both fromuseQuery/useLazyQuery; kept foruseMutation) and moveduseLazyQuery'svariablesfrom hook options to theexecute()call.@apollo/client/react,MockedProviderimports moved to@apollo/client/testing/react,addTypenameremoved from ~94 test files.Real bugs fixed along the way (not just migration noise)
App.tsx: v4 skips the entire custom link chain (includingrefreshTokenLink) for queries that are 100%@clientfields, so the proactive token-refresh check that used to piggyback onErrorHandler's query needed its own mount effect.Billing.tsx,FlowTranslation.tsx,CollectionList.tsx:const { data, error } = await mutate()assumed GraphQL errors resolve with anerrorfield; v4's defaulterrorPolicyrejects the promise instead, so these silently swallowed failures until wrapped intry/catch.common/notification.ts:setErrorMessagereaderror.networkError/.graphQLErrors, which no longer exist now thatApolloErroris gone — now checksCombinedGraphQLErrors.is(error).List.tsx: the row-fetch effect depended on thefiltersprop by reference; several callers pass a fresh object literal every render, which under v4's stricter fetchMore/cache-only rules turned into runaway refetches. Now keyed onJSON.stringify(filters).ChatMessages.tsx: switched its search query fromcache-onlytocache-first, since v4 no longer allowsfetchMoreon a cache-only query.variableMatcherfield onMockedResponsethat was never real Apollo API (silently ignored) — the real API isrequest.variablesas a matcher function.Test infrastructure changes
src/setupTests.ts:MockLink.defaultOptions.delay = 1— v4'sMockedProviderdefaults to a randomized "realistic" network delay instead of resolving on the next macrotask, which broke synchronous assertions across ~30 test files expecting near-immediate mock resolution.Test plan
tsc --noEmitclean (was 892 errors mid-migration)yarn format/ prettier cleanvitest rungreen, verified stable across repeated runs (1828/1828 non-skipped tests passing)yarn devboots and serves correctlyapolloclient.ts(auth token refresh, file upload, WebSocket subscriptions, error handling) against a live backend — recommend running thee2e-test-engineersuite before merge, since this touches security-sensitive auth/token logic that can't be fully verified by unit tests alone🤖 Generated with Claude Code