-
Notifications
You must be signed in to change notification settings - Fork 7
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
SDK updates #115
SDK updates #115
Conversation
tests/test-suite.test.ts
Outdated
@@ -80,6 +80,22 @@ describe('Passage API Requests', () => { | |||
const user = await passage.user.get(userID); | |||
expect(user).toHaveProperty('id', userID); | |||
}); | |||
test('getUserByIdentifier', async () => { | |||
const randomEmail = `${Math.random().toString(36).substr(2, 20)}@gmail.com`; |
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.
sonar cloud is failing bc of math.random
in this test. I think this is acceptable but open to suggestions for changing it.
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.
Acceptable here. I marked it as safe. You can re-run
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 using faker.internet.email()
from faker-js would be preferable
change/@passageidentity-passage-node-be06040c-81d0-4a68-a791-43fcea03063c.json
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.
Can you also add this to the README.
https://github.com/passageidentity/passage-node/blob/main/README.md?plain=1#L103
made this addition |
tests/test-suite.test.ts
Outdated
@@ -80,6 +80,22 @@ describe('Passage API Requests', () => { | |||
const user = await passage.user.get(userID); | |||
expect(user).toHaveProperty('id', userID); | |||
}); | |||
test('getUserByIdentifier', async () => { | |||
const randomEmail = `${Math.random().toString(36).substr(2, 20)}@gmail.com`; |
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 using faker.internet.email()
from faker-js would be preferable
@@ -294,7 +294,8 @@ export interface AppInfo { | |||
*/ | |||
export const AppInfoTypeEnum = { | |||
Complete: 'complete', | |||
Flex: 'flex' | |||
Flex: 'flex', | |||
FlexV2: 'flex_v2' |
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.
this might have to be regenerated since we only support complete
and flex
now and have dropped flex_v2
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.
Is that support live on prod? Also how would I go about regenerating this?
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.
You can download the most recent SDK spec from the Github action and run it with the generate.sh
file.
It would be a good idea for us to create a Github action just to regenerate SDKs.
@@ -0,0 +1,7 @@ | |||
{ | |||
"type": "minor", | |||
"comment": "Added getUserByIdentifier, Flex v2 and nonce errors,", |
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.
A more natural language description might fit our style better since these changelogs will most likely be read by other devs.
"comment": "Added getUserByIdentifier, Flex v2 and nonce errors,", | |
"comment": "Added new method to get user by identifier, getUserByIdentifier", |
also not quite sure how to do it, but a separate changelog for the Nonce
model and its accompanying error types would be good. maybe @vanessa-passage knows?
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.
Good call @ctran88. You can run the scripts again to generate another file.
Quality Gate passedIssues Measures |
No description provided.