Skip to content

feat: introduce invites and allow for inviting new users without creating an account for them#1155

Merged
kalilsn merged 114 commits into
mainfrom
tfk/invite-signup-rewrite
May 6, 2025
Merged

feat: introduce invites and allow for inviting new users without creating an account for them#1155
kalilsn merged 114 commits into
mainfrom
tfk/invite-signup-rewrite

Conversation

@tefkah
Copy link
Copy Markdown
Member

@tefkah tefkah commented Apr 10, 2025

Issue(s) Resolved

Resolves #1002

High-level Explanation of PR

See https://pubpub.github.io/platform/pr-preview/pr-1155/development/authentication/invites for a semi-accurate diagram of the invite flow.

This PR formalizes our user signup and permission management process by introducing an invite system.

Core Functionality

  • Creates a dedicated invites table to manage user onboarding
  • Invite tokens are specifically for account creation, not for granting sessions
  • Each invite specifies:
    • Target community
    • Role to be granted
    • Optional form access permissions
    • Optional pub/stage access with specific roles

Test Plan

Tests

Since we cant send emails in the preview env, ive created a small new community that has the same set up as the main tests here. You can visit the invite urls and test out the invite flow.

(the tokens are created deterministically, so these links should be safe. )

Naming convention is roughly: ${state}${inviteType}Invite

Note that once you accept an invite for that user, all the other invites will stop working, sorry. Maybe easier to run that locally.

Screenshots (if applicable)

Note

I dont want to redo the screenshots, but the link icon at the bottom is gone. i was accidentally using the link icon rather than next/link

Valid invite with email

image

Invalid invite

image

Expired

image

Already accepted

image

Already rejected

image

Revoked

image

Somehow not sent invite

image

Links:

Valid invite for user

For all the user invites, a user from another community has been invited.
their login is

email: [email protected]
password: password

not logged in

image

logged in

image

https://pr-1155-invite-signup-ip-54-234-165-172.my.preview.run/c/test-community/public/invite?invite=bbbbbbbb-bbbb-4bbb-bbbb-111111111111.bbbbbbbb11111111&redirectTo=/c/test-community/public/forms/evaluation/fill

Not for you

image

Rejecting

image

Try it here:

Notes

@tefkah tefkah force-pushed the tfk/invite-signup-rewrite branch from 7afebdb to 710b281 Compare April 10, 2025 12:03
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 10, 2025

PR Preview Action v1.6.1
Preview removed because the pull request was closed.
2025-05-06 15:19 UTC

@3mcd 3mcd temporarily deployed to gh-654103159-pr-1155 April 30, 2025 11:46 Inactive
@@ -0,0 +1,33 @@
# Signup

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not meant to be exhaustive, just not incorrect

communityId: pub.communityId as CommunitiesId,
lastModifiedBy,
actionRunId: args.actionRunId,
userId: isActionUserInitiated ? args.userId : undefined,
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bc i want to track who invited a user through an email action, if any

Comment thread core/lib/server/member.ts
Comment on lines +287 to +293
for (key in membership) {
if (membership[key] !== acc[key as keyof typeof acc]) {
throw new Error(
`Membership ${key} mismatch between ${membership[key]} and ${acc[key as keyof typeof acc]}`
);
}
}
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kalilsn not so sure about this looking at how you implemented deduping in eg

https://github.com/pubpub/platform/blob/f53f0b70f8dad448545048d522fc5ca7e1793163/core/app/components/Memberships/MembersList.tsx#L20-L33

my understanding was that a user would only have multiple memberships if they were all contributor + form memberships. but in your implementation above it seems that you are just looking for the highest value one, which implies that a user could eg have a contributor AND an editor membership for the same entity. is that intended?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you're right, we have unique indices on each of the membership tables that would prevent this situation from ever happening.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had just forgotten that when I wrote my dedupe function! And I didn't think too hard about it since that code is going to be deleted soon!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually it does seem like there's a bug in this: the || should be an &&. But since we're enforcing this at the database level, I'd rather just remove these checks

Comment thread core/playwright/invites.spec.ts
Comment on lines +14 to +18

import type { CommunitySeedOutput } from "~/prisma/seed/createSeed";
import { createSeed } from "~/prisma/seed/createSeed";
import { seedCommunity } from "~/prisma/seed/seedCommunity";

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is mostlyhere to make testing in the preview env a bit easier. i recommend we remove this seed before the merging

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

going to leave it for now because i may want to use it for my own testing. but if there's some risk i'm unaware of we can discuss when you're back!

Comment thread core/prisma/seed.ts
Comment on lines 65 to +67

const legacyId = "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa" as CommunitiesId;
const starterId = "bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb" as CommunitiesId;
const legacyId = "aaaaaaaa-aaaa-4aaa-aaaa-aaaaaaaaaaaa" as CommunitiesId;
const starterId = "bbbbbbbb-bbbb-4bbb-bbbb-bbbbbbbbbbbb" as CommunitiesId;
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

needs a 4 there to be a proper v4 uuid, which otherwise trips up the z.string().uuid()

Comment on lines +16 to +23
export enum InviteStatus {
created = "created",
pending = "pending",
accepted = "accepted",
completed = "completed",
rejected = "rejected",
revoked = "revoked",
}
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure if it's completely worth having a diff between accepted and completed

Comment on lines +38 to +39
{notice && <Notice type="notice" title={notice} body={body} />}
{error && <Notice type="error" title={error} body={body} />}
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is also a new addition: the login screen now can display notices. by default, if redirected to here bc of getPageLoginData, itll display an error saying "you need to be signed in to access that page". i added this bc i need the user to sign in to their account if its an invite for an existing account, and i think its nice to show why you need to do something on the login page, rather than just being shown a blank one

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

big improvement, i think we'll use this a lot!

@3mcd 3mcd temporarily deployed to gh-654103159-pr-1155 April 30, 2025 12:15 Inactive
@3mcd 3mcd temporarily deployed to gh-654103159-pr-1155 May 1, 2025 11:27 Inactive
@3mcd 3mcd temporarily deployed to gh-654103159-pr-1155 May 1, 2025 11:35 Inactive
@tefkah
Copy link
Copy Markdown
Member Author

tefkah commented May 1, 2025

@kalilsn i made some slight updates to some flows, namely:

  • Invite.status === "completed" is no longer seen as an error, but shows the user a screen by which they can redirect themselves to the appropriate resource
  • make proper the distinction between accepted and completed invite statuses. accepted meaning "user has clicked 'accept' but failed to finish signup", and complete meaning "accepted + no sign up action needed".
  • add some tests for those scenarios
  • re-enable the tests for the rejection flow

@3mcd 3mcd temporarily deployed to gh-654103159-pr-1155 May 1, 2025 13:14 Inactive
@3mcd 3mcd temporarily deployed to gh-654103159-pr-1155 May 6, 2025 05:00 Inactive
Copy link
Copy Markdown
Contributor

@kalilsn kalilsn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

really nice work! i'd write more but the size of this pr is making the editor perform terribly

@kalilsn kalilsn enabled auto-merge (squash) May 6, 2025 15:10
@kalilsn kalilsn merged commit ea61839 into main May 6, 2025
13 of 16 checks passed
@kalilsn kalilsn deleted the tfk/invite-signup-rewrite branch May 6, 2025 15:18
@3mcd 3mcd had a problem deploying to gh-654103159-pr-1155 May 6, 2025 15:19 Error
@kalilsn kalilsn mentioned this pull request May 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

preview Auto-deploys a preview application

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update email action to invite users who don't have accounts

3 participants