PLG: Allow users who are already on a team accept invites#63231

taras-yemets · 2024-06-12T15:54:41Z

Closes https://github.com/sourcegraph/sourcegraph/issues/63078
Part of https://github.com/sourcegraph/self-serve-cody/issues/804 (see also its backend counterpart https://github.com/sourcegraph/self-serve-cody/pull/858)
Figma

Summary

Moved the accept invite UI to the "/cody/manage" page.
Handled cases where the invited user is already a Cody Pro user.
Fixed styles in the CodyAlert component to ensure images are visible.

Implementation Details

Added the useInviteState hook, which returns initialInviteStatus and initialUserStatus.

We track the initial statuses to determine the appropriate UI variant to display. For example, if the initial invite status is "sent" (indicating the invite can be accepted) and the initial user status is UserInviteStatus.NoCurrentTeam (indicating the user is not a member of any team), a confirmation banner is shown for the user to accept or decline the invite. After the user responds, the invite query is invalidated, updating the status to "accepted", "canceled", or "errored" based on their action. Depending on the result, a success or error banner is displayed, or the banner is hidden if the invite is canceled. More configuration examples can be found in the Screenshots section. All possible states are detailed in the AcceptInviteBannerContent.
For users who are the sole admin of their current team, the banner is shown on the "/cody/manage" page.

The design requires showing a banner on the "/cody/team/manage" page to suggest transferring the admin role to another team member. However, this page is not yet ready. To sync the banner state with user role changes or deletion actions, the members list query must be invalidated after each action. The current implementation of the "cody/manage/team" page does not support refetching with the useSSCQuery hook. To resolve this, we need to migrate the "cody/manage/team" page to use React Query to allow query invalidation after each action. For now, users who are sole admins see a banner on the "cody/manage" page suggesting transferring the admin role, with a link to the "/cody/manage/team" page.

Screenshots

Description	Screenshot
Failed to define user state OR invite status is not "sent" (thus can't be accepted)
User is not on a Cody Pro team
on the team they've been invited to
User is the sole admin of another team
User is on another team

Test plan

Checkout the branch from https://github.com/sourcegraph/self-serve-cody/pull/858 and run SSC locally
Run Sourcegraph in dotcom mode
As a Cody Pro team admin send invites to users that have different statuses (are not on a team, are members of the team they were invited to, are members of another team, are sole admins of their teams)
As the invited user:
- click the invite link from the email
- modify the hostname in the URL so that it points to the local Sourcegraph instance
- ensure the correct banner is displayed
- ensure user can accept/decline the invite (if applicable for the banner type)

Changelog

…ith-a-team

…aph/sourcegraph into ty/invites-to-users-with-a-team

vdavid

Wow, nice, well-organized so it was easy to follow and review. Great job!

vdavid · 2024-06-14T15:44:27Z

client/web/src/cody/invites/AcceptInvitePage.tsx

Oh, I assumed this page would be deleted altogether. Why do we still need this?

Invite link in the email points to this page. Do you know how we can change that?

Yup, it's easy, its only reference is here: https://sourcegraph.sourcegraph.com/github.com/sourcegraph/self-serve-cody@main/-/blob/cody/backend/internal/api/team_invites.go?L23

Thank you! Will address in a follow-up PR.

vdavid · 2024-06-14T15:46:24Z

client/web/src/cody/invites/useInviteParams.ts

This hook is so nice and clean 👌

vdavid · 2024-06-14T15:55:21Z

client/web/src/cody/invites/useInviteState.ts

This is very nice, too. I spent quite a while understanding it, and it makes sense, I think this is a helpful encapsulation of this logic. 👍

vdavid · 2024-06-14T15:56:43Z

client/web/src/cody/management/api/react-query/callCodyProApi.ts

    const response = await fetch('/-/sign-out', buildRequestInit({ method: 'GET' }))
    if (response.ok) {
-        window.location.href = `/sign-in?returnTo=${window.location.pathname}`
+        window.location.href = `/sign-in?returnTo=${window.location.pathname + window.location.search}`


Oh, good catch

@vdavid, here's an even better catch https://github.com/sourcegraph/sourcegraph/pull/63275 😅

vdavid · 2024-06-14T16:00:06Z

client/web/src/cody/management/api/react-query/subscriptions.ts

    PreviewResult,
    PreviewCreateTeamRequest,
-} from '../types'
+} from '../teamSubscriptions'


Nit: I think the idea is that we don't directly reference those files, and only use types.ts as an abstraction. It doesn't matter all that much, @chrsmith's reasoning in types.ts says

// Export all API types, so consumers we can organize the type definitions // into smaller files, without consumers needing to care about that organization.

and it makes sense, although it's pretty easy to replace any direct references with types.ts references if we want to reorganize stuff.

I don't have any opinions here, and I imagine that there is some marginal efficiency gain by only needing to read from the teamSubscriptions.ts file instead of needing the TypeScript compiler to crawl all of the types.ts (and what it brings in).

The main point was to just make it so we that we don't have to spend time figuring out which particular types are defined in which particular file. And so just import type * as types would "just work". (And, IMHO, needing to prefix API type names with types. also makes them stand out in the code which has some minor benefits too.)

But anyways, I don't think we should try to standardize on only referencing the data types from that particular module or anything.

vdavid · 2024-06-14T16:03:43Z

Two small change requests:

In the PR description, I think the screenshot you have next to "User is on the team they've been invited to" is the wrong one. You might've accidentally merged two rows. It might help @rrhyne's work if you fix this.
In my understanding, PR titles in the monorepo now must follow the "feat(plg): Change description" format. I think following this helps with automated changelog building.

taras-yemets · 2024-06-14T16:12:40Z

Thanks for your review, @vdavid!

In the PR description, I think the screenshot you have next to "User is on the team they've been invited to" is the wrong one.

Nice catch! Thank you!

PR titles in the monorepo now must follow the "feat(plg): Change description" format.

I may have missed it. Do you know if there's a related doc? I can see different PR naming approaches in the monorepo including all or some of the following: change type (feat/fix/etc), scope/feature (api/plg/cody-web) and arbitrary description.

Fixes bug intorduced in https://github.com/sourcegraph/sourcegraph/pull/63231 Encode the URL search params added to `returnTo` search param value. Previously the search param value hasn't been encoded resulting in URLs with broken search params , e.g. *"/sign-in?returnTo=/cody/manage?teamID=1&inviteID=2"* (note the second "?" making the search params invalid).  ## Test plan - CI - Tested manually: - Make `signOutAndRedirectToSignIn` available in the window object, e.g. `window.signOutAndRedirectToSignIn = signOutAndRedirectToSignIn` in the *client/web/src/cody/management/api/react-query/callCodyProApi.ts* - On "/cody/manage" page (any page works actually) ensure that a few search params are added to the URL - call `signOutAndRedirectToSignIn` in the browser console - ensure you have been sign out, signed back in and the search params are still in the URL  ## Changelog

vdavid · 2024-06-14T17:41:48Z

PR titles in the monorepo now must follow the "feat(plg): Change description" format.

I may have missed it. Do you know if there's a related doc? I can see different PR naming approaches in the monorepo including all or some of the following: change type (feat/fix/etc), scope/feature (api/plg/cody-web) and arbitrary description.

@taras-yemets It's in the template for new PR descriptions in the monorepo. Read the ## Changelog section. I think it's only been there for two weeks or so.

taras-yemets · 2024-06-14T17:43:49Z

It's in the template for new PR descriptions in the monorepo.

TIL! Thanks a lot, @vdavid! I completely missed that update 🤦🏻‍♂️

rrhyne · 2024-06-14T20:37:58Z

@taras-yemets sorry I missed this. Some QA here:

https://www.figma.com/design/FMSdn1oKccJRHQPgf7053o/Cody-PLG-GA?node-id=5472-12492&t=ndgEZFI57dVTIBpo-4

Adresses [UI feedback](https://www.figma.com/design/FMSdn1oKccJRHQPgf7053o/Cody-PLG-GA?node-id=5472-12492&t=ndgEZFI57dVTIBpo-4) on https://github.com/sourcegraph/sourcegraph/pull/63231 |Description|Before|After| |--|--|--| | Change dismiss button variant from "secondary" to "link" |<img width="1516" alt="Screenshot 2024-06-14 at 14 34 30" src="https://github.com/sourcegraph/sourcegraph/assets/25318659/d6b53dc4-c7df-45eb-a743-9baddfdd8aa3">|<img width="1170" alt="Screenshot 2024-06-17 at 12 35 53" src="https://github.com/sourcegraph/sourcegraph/assets/25318659/6778a4bf-9a03-4c3b-a83c-57777859b7f1">| | Fix banner content alignment |<img width="1516" alt="Screenshot 2024-06-14 at 14 35 41" src="https://github.com/sourcegraph/sourcegraph/assets/25318659/eaa871ce-acd3-4a7e-a25c-74011a42af58"><img width="1516" alt="Screenshot 2024-06-14 at 14 38 42" src="https://github.com/sourcegraph/sourcegraph/assets/25318659/1382e5a0-4375-4002-93a4-ec25d354317f">|<img width="1165" alt="Screenshot 2024-06-17 at 12 31 14" src="https://github.com/sourcegraph/sourcegraph/assets/25318659/532930ff-8471-46d8-83c0-f2bbeb1a8bcd">|  ## Test plan - Tested manually (screenshots attached)  ## Changelog

Removes "/cody/invites/accept" page. Invite processing is handled on the "/cody/manage" page and "/cody/invites/accept" is used only to redirect to the former. Backend counterpart: sourcegraph/self-serve-cody#886  ## Test plan - Tested manually - checkout branch fromhttps://github.com/sourcegraph/self-serve-cody/pull/886 and run SSC backend locally - run Sourcegraoh instance in dotcom mode locally - log in as a Cody Pro team admin and send an invite to another user - log in as an invited user and follow the link from the email - ensure link navigates to the "/cody/manage/" page and is hanled as expected (see test plan from https://github.com/sourcegraph/sourcegraph/pull/63231)  ## Changelog

Allow users who are already on a team accept invites

f5c2e44

cla-bot bot added the cla-signed label Jun 12, 2024

taras-yemets added 7 commits June 12, 2024 19:03

track only initial user invite status

da7dd61

wip

bf1cf67

wip

02bbb59

wip

4b70084

bazel configure

cc3f751

Merge remote-tracking branch 'origin/main' into ty/invites-to-users-w…

743b415

…ith-a-team

bazel configure

a65e9fe

taras-yemets changed the title ~~Allow users who are already on a team accept invites~~ PLG: Allow users who are already on a team accept invites Jun 14, 2024

wip

d465d63

taras-yemets requested a review from a team June 14, 2024 13:06

taras-yemets added 3 commits June 14, 2024 16:07

undo manage team page change

c4a9f1b

fix lint errors

968c51d

Merge branch 'main' into ty/invites-to-users-with-a-team

f74bf19

taras-yemets marked this pull request as ready for review June 14, 2024 13:09

taras-yemets added 4 commits June 14, 2024 16:21

fix merge errors

6e068fe

Merge branch 'ty/invites-to-users-with-a-team' of github.com:sourcegr…

156a241

…aph/sourcegraph into ty/invites-to-users-with-a-team

fix stylelint

a5bead1

remove unused arg

2151403

vdavid approved these changes Jun 14, 2024

View reviewed changes

taras-yemets added 3 commits June 14, 2024 19:36

remove debug console log

3df3e91

simplify logic

a553eef

Merge branch 'main' into ty/invites-to-users-with-a-team

9f2fd0f

taras-yemets enabled auto-merge (squash) June 14, 2024 16:45

taras-yemets requested a review from rrhyne June 14, 2024 16:46

taras-yemets disabled auto-merge June 14, 2024 16:52

taras-yemets enabled auto-merge (squash) June 14, 2024 16:52

taras-yemets merged commit 50471a6 into main Jun 14, 2024

taras-yemets deleted the ty/invites-to-users-with-a-team branch June 14, 2024 16:56

taras-yemets mentioned this pull request Jun 14, 2024

PLG: encode URL search params #63275

Merged

This was referenced Jun 17, 2024

fix(plg): banner styles #63287

Merged

feat(plg): remove accept invite page #63290

Merged

Conversation

taras-yemets commented Jun 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Implementation Details

Screenshots

Test plan

Changelog

Uh oh!

vdavid left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vdavid commented Jun 14, 2024

Uh oh!

taras-yemets commented Jun 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vdavid commented Jun 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

taras-yemets commented Jun 14, 2024

Uh oh!

rrhyne commented Jun 14, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

taras-yemets commented Jun 12, 2024 •

edited

Loading

taras-yemets commented Jun 14, 2024 •

edited

Loading

vdavid commented Jun 14, 2024 •

edited

Loading