-
Notifications
You must be signed in to change notification settings - Fork 15
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
Orion signup flow rework #321
Orion signup flow rework #321
Conversation
id: ID! | ||
|
||
"Membership associated with the blockchain account (controllerAccount)" | ||
memberships: [Membership!] @derivedFrom(field: "controllerAccount") |
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.
Should we support multiple memberships within Atlas? Seems, like an overkill. Besides that it is possible from runtime perspective, it will only complicate the membership dropdown (we already had some feedback that it is complicated @dmtrjsg)
{ joystreamAccountId: payload.joystreamAccountId } | ||
) | ||
|
||
console.log('after update') |
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.
log
}) | ||
|
||
// Mark the token as used | ||
token.expiry = new Date() |
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.
Can't we just drop the token if used?
75160c2
to
2fba0a5
Compare
6f8a7d0
to
d580268
Compare
@@ -20,33 +31,102 @@ export class AccountResolver { | |||
const account = ctx.account | |||
const em = await this.em() | |||
assert(account, 'Unexpected context: account is not set') | |||
const { id, email, joystreamAccount, membershipId, isEmailConfirmed, notificationPreferences } = | |||
account | |||
const { id, email, joystreamAccount, notificationPreferences } = account |
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.
const { id, email, joystreamAccount, notificationPreferences } = account | |
const { id, email, joystreamAccountId, notificationPreferences } = account |
In authenticate
we don't fetch the account with BlockchainAccount relation, so joystreamAccount
is empty.
Rn I adjusted the Atlas to use joystreamAccountId
, but I can switch it, if we decide to include the relation.
|
||
@Field(() => String, { nullable: false }) | ||
membershipId: string | ||
joystreamAccount!: BlockchainAccount |
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 believe we will have to either introduce new type-graphql class or go with joystreamAccountId
approach.
Plus lets make it nullable, since there is a possibilty that user creates an account, but membership creation fails.
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.
Even if the membership creation fails, the joystreamAccountId
will still not be null, since it is provided in the create account request
|
||
type ReqParams = Record<string, string> | ||
type ResBody = | ||
| components['schemas']['GenericOkResponseData'] | ||
| components['schemas']['GenericErrorResponseData'] | ||
type ReqBody = components['schemas']['RequestTokenRequestData'] | ||
type ResLocals = { authContext: Session } | ||
|
||
export const requestEmailConfirmationToken: ( |
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 need to add even more details to email confirmation link we would need 3 different |
Progress Summary
|
Addresses #320