feat: #74 — Bug: Copy/paste invite code shows "No mappable coordinates found" instead of connection progress animation#109
Conversation
…coordinates found" instead of connection progress animation
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
| if (!cpState.isActive || cpState.isComplete) return; | ||
|
|
||
| let cancelled = false; | ||
|
|
||
| async function runConnection() { | ||
| const steps = [...cpState.steps]; | ||
|
|
||
| // Step 1: Connecting to server (index 1) | ||
| steps[1] = { ...steps[1]!, status: 'active' }; | ||
| setCpState((prev) => ({ ...prev, steps: [...steps] })); | ||
|
|
||
| const result = await syncRemoteArchive(cpState.serverId, { | ||
| baseUrl: cpState.baseUrl, | ||
| token: cpState.token, | ||
| }); | ||
| if (cancelled) return; | ||
|
|
||
| if (!result.success) { | ||
| steps[1] = { ...steps[1]!, status: 'error' }; | ||
| setCpState((prev) => ({ | ||
| ...prev, | ||
| steps: [...steps], | ||
| errorMessage: result.error ?? 'Sync failed', | ||
| })); | ||
| return; | ||
| } | ||
|
|
||
| // Step 1: Connecting complete | ||
| steps[1] = { ...steps[1]!, status: 'completed' }; | ||
| setCpState((prev) => ({ ...prev, steps: [...steps] })); | ||
|
|
||
| // Step 2: Syncing data (index 2) | ||
| steps[2] = { ...steps[2]!, status: 'active' }; | ||
| setCpState((prev) => ({ ...prev, steps: [...steps] })); | ||
|
|
||
| await Promise.all([ | ||
| queryClient.invalidateQueries({ queryKey: ['projects'] }), | ||
| queryClient.invalidateQueries({ queryKey: ['observations'] }), | ||
| queryClient.invalidateQueries({ queryKey: ['alerts'] }), | ||
| ]); | ||
| if (cancelled) return; | ||
|
|
||
| steps[2] = { ...steps[2]!, status: 'completed' }; | ||
| setCpState((prev) => ({ ...prev, steps: [...steps] })); | ||
|
|
||
| // Step 3: Preparing dashboard (index 3) | ||
| steps[3] = { ...steps[3]!, status: 'active' }; | ||
| setCpState((prev) => ({ ...prev, steps: [...steps] })); | ||
|
|
||
| // Brief pause for the "Preparing" step to be visible | ||
| await new Promise((resolve) => setTimeout(resolve, 500)); | ||
| if (cancelled) return; | ||
|
|
||
| steps[3] = { ...steps[3]!, status: 'completed' }; | ||
| setCpState((prev) => ({ | ||
| ...prev, | ||
| steps: [...steps], | ||
| isComplete: true, | ||
| })); | ||
| } | ||
|
|
||
| void runConnection(); | ||
| return () => { | ||
| cancelled = true; | ||
| }; | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [cpState.isActive, cpState.isComplete]); |
There was a problem hiding this comment.
"Try Again" never re-runs
runConnection
When syncRemoteArchive fails, cpState.isActive stays true and cpState.isComplete stays false. Clicking "Try Again" calls startConnectionProgress, which sets isActive: true and isComplete: false — identical to their current values. The useEffect dependency array [cpState.isActive, cpState.isComplete] sees no change and does not re-run runConnection. The retry button is a no-op: the error screen persists and the connection is never attempted again.
A counter or a distinct trigger field (e.g., a monotonically incrementing retryCount) needs to be part of the dependency array so the effect re-fires on each retry attempt.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/screens/Home/AddArchiveServerDialog.tsx
Line: 263-329
Comment:
**"Try Again" never re-runs `runConnection`**
When `syncRemoteArchive` fails, `cpState.isActive` stays `true` and `cpState.isComplete` stays `false`. Clicking "Try Again" calls `startConnectionProgress`, which sets `isActive: true` and `isComplete: false` — identical to their current values. The `useEffect` dependency array `[cpState.isActive, cpState.isComplete]` sees no change and does not re-run `runConnection`. The retry button is a no-op: the error screen persists and the connection is never attempted again.
A counter or a distinct trigger field (e.g., a monotonically incrementing `retryCount`) needs to be part of the dependency array so the effect re-fires on each retry attempt.
How can I resolve this? If you propose a fix, please make it concise.| // When connection progress is active, show it inside the dialog | ||
| if (cpState.isActive) { | ||
| return ( | ||
| <Modal | ||
| open={isOpen} | ||
| onOpenChange={() => { | ||
| // Prevent closing during connection progress | ||
| }} | ||
| title={intl.formatMessage(messages.title)} | ||
| > | ||
| <div className="flex flex-col items-center gap-6 py-6"> | ||
| <ConnectionProgress | ||
| steps={cpState.steps} | ||
| heading={intl.formatMessage(messages.connectionProgressHeading)} | ||
| isComplete={cpState.isComplete} | ||
| /> | ||
| {cpState.errorMessage && ( | ||
| <div className="mt-2 flex flex-col items-center gap-3"> | ||
| <p className="text-sm text-red-600" role="alert"> | ||
| {cpState.errorMessage} | ||
| </p> | ||
| <div className="flex gap-2"> | ||
| <Button | ||
| type="button" | ||
| variant="secondary" | ||
| size="sm" | ||
| onClick={handleClose} | ||
| > | ||
| {intl.formatMessage(messages.cancel)} | ||
| </Button> | ||
| <Button | ||
| type="button" | ||
| variant="primary" | ||
| size="sm" | ||
| onClick={() => { | ||
| startConnectionProgress( | ||
| cpState.serverId, | ||
| cpState.baseUrl, | ||
| cpState.token, | ||
| ); | ||
| }} | ||
| > | ||
| Try Again | ||
| </Button> | ||
| </div> | ||
| </div> | ||
| )} | ||
| </div> | ||
| </Modal> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Stale connection-progress state persists across dialog sessions
After a successful connection, onAdded fires and HomeScreen closes the dialog by dispatching CLOSE_ADD_SERVER_DIALOG — it does not call onClose(), so handleClose (which resets UI state) is never invoked. cpState is left as { isActive: true, isComplete: true }. The next time the user opens "Add Archive Server", the component mounts with that stale state, immediately renders the completed connection-progress screen, and never shows the invite-code input form.
cpState should be reset to INITIAL_CP_STATE whenever the dialog transitions from open to closed (e.g., by reacting to isOpen changing to false via a useEffect, or by resetting inside handleClose and ensuring onAdded also calls handleClose after firing).
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/screens/Home/AddArchiveServerDialog.tsx
Line: 570-620
Comment:
**Stale connection-progress state persists across dialog sessions**
After a successful connection, `onAdded` fires and `HomeScreen` closes the dialog by dispatching `CLOSE_ADD_SERVER_DIALOG` — it does not call `onClose()`, so `handleClose` (which resets UI state) is never invoked. `cpState` is left as `{ isActive: true, isComplete: true }`. The next time the user opens "Add Archive Server", the component mounts with that stale state, immediately renders the completed connection-progress screen, and never shows the invite-code input form.
`cpState` should be reset to `INITIAL_CP_STATE` whenever the dialog transitions from open to closed (e.g., by reacting to `isOpen` changing to `false` via a `useEffect`, or by resetting inside `handleClose` and ensuring `onAdded` also calls `handleClose` after firing).
How can I resolve this? If you propose a fix, please make it concise.| > | ||
| Try Again | ||
| </Button> |
There was a problem hiding this comment.
The "Try Again" button label is a hard-coded English string. Every other button in this component uses
intl.formatMessage. A missing i18n message key here means this text will not be translated for non-English users.
| > | |
| Try Again | |
| </Button> | |
| > | |
| {intl.formatMessage(messages.tryAgain)} | |
| </Button> |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/screens/Home/AddArchiveServerDialog.tsx
Line: 611-613
Comment:
The "Try Again" button label is a hard-coded English string. Every other button in this component uses `intl.formatMessage`. A missing i18n message key here means this text will not be translated for non-English users.
```suggestion
>
{intl.formatMessage(messages.tryAgain)}
</Button>
```
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| onAdded={(_serverId) => { | ||
| dispatch({ type: 'CLOSE_ADD_SERVER_DIALOG' }); | ||
| const server = useAuthStore | ||
| .getState() | ||
| .servers.find((s) => s.id === serverId); | ||
| if (server) { | ||
| dispatch({ | ||
| type: 'START_CONNECTION_PROGRESS', | ||
| serverId, | ||
| baseUrl: server.baseUrl, | ||
| token: server.token, | ||
| steps: buildConnectionProgressSteps(intl), | ||
| }); | ||
| } | ||
| // Connection progress is now handled inside the dialog. | ||
| // Just invalidate queries to refresh data. | ||
| void queryClient.invalidateQueries({ queryKey: ['projects'] }); | ||
| void queryClient.invalidateQueries({ queryKey: ['observations'] }); | ||
| void queryClient.invalidateQueries({ queryKey: ['alerts'] }); | ||
| }} |
There was a problem hiding this comment.
Duplicate
invalidateQueries calls
AddArchiveServerDialog already calls queryClient.invalidateQueries for ['projects'], ['observations'], and ['alerts'] during step 2 ("Syncing data") of the connection progress. The same three invalidations are repeated here in every onAdded handler (appears 4 times across HomeScreen.tsx). This doubles the refetch work roughly 1500 ms after the first invalidation, which is redundant. The HomeScreen handlers could simply close the dialog and let the dialog's own invalidation handle the refresh.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/screens/Home/HomeScreen.tsx
Line: 1119-1126
Comment:
**Duplicate `invalidateQueries` calls**
`AddArchiveServerDialog` already calls `queryClient.invalidateQueries` for `['projects']`, `['observations']`, and `['alerts']` during step 2 ("Syncing data") of the connection progress. The same three invalidations are repeated here in every `onAdded` handler (appears 4 times across `HomeScreen.tsx`). This doubles the refetch work roughly 1500 ms after the first invalidation, which is redundant. The `HomeScreen` handlers could simply close the dialog and let the dialog's own invalidation handle the refresh.
How can I resolve this? If you propose a fix, please make it concise.
Summary
Automated implementation of #74.
Closes #74
Implemented by the agent implementation worker.
Greptile Summary
This PR moves the connection-progress animation from
HomeScreenintoAddArchiveServerDialogso that pasting/typing an invite code now shows the progress steps instead of navigating to the map (the "No mappable coordinates found" error from #74). TheonAddedcallback inHomeScreenis simplified to just close the dialog.AddArchiveServerDialoggains aConnectionProgressStateslice, twouseEffecthooks that drive the step sequence (verify → connect → sync → prepare), and a conditional render that replaces the form with theConnectionProgresscomponent while the flow is running.HomeScreenremoves theSTART_CONNECTION_PROGRESSdispatch from all fouronAddedhandlers but still redundantly re-invalidates the same query keys the dialog already invalidates.useEffectdeps do not change on a failed retry), andcpStateis never reset when the dialog closes after success, causing the next "Add Server" open to render the completed-progress screen immediately.Confidence Score: 3/5
The core fix works, but two functional regressions are introduced: the error-recovery path is broken and reopening the dialog after a successful add shows a stale completed-progress screen.
The Try Again button is silently non-functional after a sync failure, and the dialog state is never cleaned up after a successful add, so every subsequent open of Add Archive Server renders the old completed-progress UI instead of the invite form. Both issues affect user flows directly reachable from the happy path.
src/screens/Home/AddArchiveServerDialog.tsx — the connection-progress useEffect dependency array and the missing cpState reset on dialog close.
Important Files Changed
Sequence Diagram
sequenceDiagram participant User participant Dialog as AddArchiveServerDialog participant API as redeemEncryptedInvite participant Sync as syncRemoteArchive participant QC as QueryClient participant HS as HomeScreen User->>Dialog: paste invite code and click Add Dialog->>API: redeemEncryptedInvite(code) API-->>Dialog: baseUrl and token Dialog->>Dialog: addServer(baseUrl, token) Dialog-->>Dialog: startConnectionProgress Dialog->>Sync: syncRemoteArchive(serverId) Sync-->>Dialog: success Dialog->>QC: invalidateQueries projects/observations/alerts Dialog-->>Dialog: isComplete true Dialog->>HS: onAdded(serverId) after 1500ms HS->>HS: dispatch CLOSE_ADD_SERVER_DIALOG HS->>QC: invalidateQueries projects/observations/alerts againPrompt To Fix All With AI
Reviews (1): Last reviewed commit: "feat: implement #74 — Bug: Copy/paste in..." | Re-trigger Greptile