-
Notifications
You must be signed in to change notification settings - Fork 404
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
User invite codes #757
User invite codes #757
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Looks great
"user": { "type": "ref", "ref": "#codesDetail" }, | ||
"admin": { "type": "ref", "ref": "#codesDetail" } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's on the horizon to switch-up how admin auth works, and certain user accounts may be marked as admins. Just tossing that out there for a gut check that this API will still match-up with that nicely, given that there could be overlap between "user" and "admin".
I think it still could work— "user" would just have to be specifically for non-admin accounts. Or another way to think about it is whether the invite code is generated from createInviteCode
versus getAccountInviteCodes
.
"required": ["usedBy", "usedAt"], | ||
"properties": { | ||
"usedBy": {"type": "string"}, | ||
"usedAt": {"type": "string"} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could toss a datetime
format here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
> spend 3 weeks revamping lexicon
> immediately forget to use the new features
qb | ||
.selectFrom('invite_code_use') | ||
.groupBy('code') | ||
.select(['code', sql<number>`count(*)`.as('uses')]), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a countAll
helper in db utils that could be used here like countAll.as('uses')
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah nice 👍
|
||
export default function (server: Server, ctx: AppContext) { | ||
server.com.atproto.server.createInviteCode({ | ||
auth: ctx.adminVerifier, | ||
handler: async ({ input, req }) => { | ||
const { useCount } = input.body | ||
const { useCount, forAccount = 'admin' } = input.body |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might want to have an idea about how this will look if/when any normal account might be marked as an administrator, when we upgrade admin auth. I guess it's possible forAccount
could just be attributed to the admin themselves by their did!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah that'd probably be fine 🤔
alternately we could just use null
s for forAccount
that is just made generally. 'admin' kinda functions as a null in a way
packages/pds/src/api/com/atproto/server/getAccountInviteCodes.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, super tidy!
@@ -0,0 +1,32 @@ | |||
{ | |||
"lexicon": 1, | |||
"id": "com.atproto.admin.getInviteCodeUsage", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be useful to add timespan parameters to this query, if that's not too difficult. Then in the dashboard we could give (last week, last month, all time) on these stats
Sweet! Thanks bois |
* getUserInviteCodes lex * small change * implement user invite code creation/getting * transactionally ensure we dont allow duplicate creates * testing & fixes * clean up & allow admin creation for a particular user * fix dev-env * user -> accnt & add admin disable codes route * proposed admin inv schemas * more admin routes for inv codes * tests for invite admin views * pr feedback * refactor & return usedBy + more details on getAccountInviteCodes * adding invite info into moderation views * tests passing
Implements user invite codes.
This goes through a single method
getUserInviteCodes
.This will return all user codes that were created for a specific user, including used up codes. If a user has more invite codes available to generate, it will generate them & return them in the response as well.
A requester can opt to filter out used up codes or avoid generating new codes through the param flags.
A user's ability to generate new codes is decided by the service's
userInviteInterval
cfg value. This is the number of milliseconds for a user to accrue an invite.For instance: if the user has been a server member for 5 wks & the server's interval is set to 2 wks, they will have 2 codes available for creation.
We also apply a limit of 5 active codes per user at any given point.
In the case that a service does not want to distribute codes on an interval, they can use the
createInviteCode
method to distribute codes to individual users.This also adds a route to disable invite codes, either by individual code or by disabling all codes created by a particular user.
Along with a couple of views for getting an overview of the total number of codes available + a paginated view for seeing individual codes & their uses, sortable by either recency or usage.