feat(scm-first-messaging-integration): Adding providerRow component - #121728
feat(scm-first-messaging-integration): Adding providerRow component#121728Abdkhan14 wants to merge 4 commits into
Conversation
github-actions
Bot
added
the
Scope: Frontend
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit f0b6cac. Configure here.
| if (installState.status === 'installing') { | ||
| return 'installing'; | ||
| } | ||
| if (installState.status === 'error') { |
There was a problem hiding this comment.
medium: if the install errors but the integration actually exists (the integrations query is shared, so a successful install on another row can refetch and surface it), this row stays stuck on the error alert and Try again would reinstall an existing integration. worth gating this on viewModel.status === 'installable'?
There was a problem hiding this comment.
Valid bug, the pushed the fix: wrapped both install-error branches in a viewModel.status === 'installable' guard. If a shared-query refetch (triggered by another row's successful install) reveals this provider's integration, the row now falls through to the connected/configured state instead of stranding on the error alert whose "Try again" would reinstall an already-installed integration.
added a test
| messagingSetup: ScmMessagingSetup; | ||
| onInstallComplete: () => void; | ||
| onMessagingSetupChange: (setup: ScmMessagingSetup) => void; | ||
| viewModel: ScmMessagingProviderViewModel; |
There was a problem hiding this comment.
nit: viewModel is not very helpful / descriptive as a prop name IMO. maybe resolvedProvider, with the type renamed to ResolvedMessagingProvider back in #121631? non-blocking, fine to punt given the stack churn
There was a problem hiding this comment.
I'm noting down some of these post merge refactor suggestions, punting for now, will address separately, thanks
| [onMessagingSetupChange] | ||
| ); | ||
|
|
||
| const errorMessage = |
There was a problem hiding this comment.
nit: would be much easier to grok as a function with early returns
There was a problem hiding this comment.
Done, addressed this and the two below which are similar
|
|
||
| expect(screen.getByRole('button', {name: /Edit/})).toBeInTheDocument(); | ||
| expect( | ||
| screen.queryByText(/The .* workspace will remain connected/) |
There was a problem hiding this comment.
nit: the Edit assertion already proves we returned to configured no? i think this negative check can go.
| await userEvent.click(screen.getByRole('button', {name: /Remove/})); | ||
|
|
||
| const [confirmBtn] = screen.getAllByRole('button', {name: /Remove/}); | ||
| await userEvent.click(confirmBtn); |
There was a problem hiding this comment.
seems like there's still a type err here
Argument of type 'HTMLElement | undefined' is not assignable to parameter of type 'Element'.
2 participants
Problem
The new inline messaging step renders one row per curated provider (Slack, Discord, MS Teams), and each row has to represent a lot of states: not yet installed, install in progress, install failed, freshly installed, connected, ineligible (tenant MS Teams), configuring a destination, configured, and confirming removal. The
useScmMessagingProvidershook already exposes the settled data per provider, but there was no component to turn that view model — plus the live install flow and the destination saved in session — into a single coherent row UI.Change
Adds
ScmMessagingProviderRow, a self-contained row component for one messaging provider. It drives installs throughuseAddIntegrationand collapses the view model, install-flow state, and session setup into one purederiveVisualStatefunction, so each render resolves to exactly one visual state (installable, installing, install-error, loading, permission-limited, connected, configuring, configured, removing). The install lifecycle is fully wired — Connect opens the flow, a spinner covers the post-install window, errors surface a retry, and remove asks for confirmation — while