Skip to content
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

Feat/auth #5

Merged
merged 23 commits into from
Nov 29, 2023
Merged

Feat/auth #5

merged 23 commits into from
Nov 29, 2023

Conversation

Siddharth9890
Copy link
Collaborator

@Siddharth9890 Siddharth9890 commented Nov 10, 2023

Added all methods related to Auth and possible unit test for it

@Siddharth9890 Siddharth9890 added the enhancement New feature or request label Nov 10, 2023
@Siddharth9890 Siddharth9890 self-assigned this Nov 10, 2023
src/auth/auth.ts Outdated
}
}

/**
Copy link
Collaborator

Choose a reason for hiding this comment

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

you can remove that one.

src/auth/auth.ts Outdated
* method.
* @returns the result of the `this.sdk.unregisterAuthMethod_mutation` method, which is awaited.
*/
async unregisterAuthMethod({ data, type }: { data: JSON; type: AuthType }) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ideally, instead of JSON, try to type this data.

src/auth/auth.ts Outdated
* @param - - `authId`: A string representing the ID of the authentication method to be migrated.
* @returns the result of the `migrateAuthMethod` mutation.
*/
async migrateAuthMethod({
Copy link
Collaborator

Choose a reason for hiding this comment

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

@NunoMartins21 Do we need that? What is this method?

Copy link
Member

Choose a reason for hiding this comment

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

That's to migrate an alias from one user to another. It's ok to have it on the SDK

Copy link
Collaborator

Choose a reason for hiding this comment

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

remove it

src/auth/auth.ts Outdated
* @param {Chain} chain : a chain optional of type Chain
* @returns the result of the `createWalletNounce` method call, is a message which will be used to confirm wallet.
*/
async createWalletNounce({
Copy link
Collaborator

Choose a reason for hiding this comment

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

why the user needs to open an object for that? change all things like that to pass directly as params

src/auth/auth.ts Outdated
* @param - - `authId`: A string representing the ID of the authentication method to be migrated.
* @returns the result of the `migrateAuthMethod` mutation.
*/
async migrateAuthMethod({
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove it

it(
'create wallet nounce',
async () => {
const { message } = await api.auth.createWalletNounce(
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const { message } = await api.auth.createWalletNounce(
const { message } = await api.auth.createWalletNonce(

src/auth/auth.ts Outdated
* @param {Chain} chain : a chain optional of type Chain
* @returns the result of the `createWalletNounce` method call, is a message which will be used to confirm wallet.
*/
async createWalletNounce(wallet: string, chain?: Chain) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
async createWalletNounce(wallet: string, chain?: Chain) {
async createWalletNonce(wallet: string, chain?: Chain) {

@Siddharth9890 Siddharth9890 merged commit 29694e9 into main Nov 29, 2023
1 check failed
@Siddharth9890 Siddharth9890 deleted the feat/Auth branch November 29, 2023 10:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants