-
Notifications
You must be signed in to change notification settings - Fork 13
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
project scaffold changes #6
Conversation
initial auth scaffold and what sam needs to begin with
Sam/account page
Sam/account page
Sam/account page
Sam/mock login
quick fix for test data for ux review
/gcbrun |
Temporary Url for review: https://strr-ui-dev--pr-6-rbhzjchl.web.app |
re-enable auth and linting cleanup
/gcbrun |
Temporary Url for review: https://strr-ui-dev--pr-6-rbhzjchl.web.app |
}) | ||
}) | ||
} | ||
|
||
/** Get the user's account list */ | ||
async function getUserAccounts (keycloakGuid: string) { | ||
const apiURL = useRuntimeConfig().public.authApiURL |
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 this the right endpoint to get user accounts? Please check whether${apiURL}/users/orgs
should be used instead.
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.
@kris-daxiom the /users/{kcguid_id}/orgs (similar to /settings used earlier), results in a 404. Perhaps there is a better endpoint for this though to find details of orgs - we are currently hitting the /orgs/{org_id} endpoint to fetch the mailing addresses per Org. We have discussed to in a future sprint move this handling to the STRR api regardless.
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.
If you are planning to have a wrapper in strr-api that is fine. But otherwise the endpoint is users/orgs ( no kcguid_id ). The user details will be taken from the token
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.
Oh perfect @kris-daxiom we will make changes in upcoming work mentioned above to adapt to use this endpoint, thanks
@@ -59,7 +64,8 @@ export default defineNuxtConfig({ | |||
payApiURL: `${process.env.VUE_APP_PAY_API_URL || ''}${process.env.VUE_APP_PAY_API_VERSION || ''}`, | |||
btrApiURL: `${process.env.VUE_APP_BTR_API_URL || ''}${process.env.VUE_APP_BTR_API_VERSION || ''}`, |
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 this needed?
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.
@samIndustrio please include in any update you are in the middle of
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 good to me. A few questions
userAccounts = testData | ||
} | ||
|
||
const existingAcccountsTitle = `${t('account.existing-account-section.title')} (${userAccounts.length})` |
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.
Nit: typo in the variable name
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.
@samIndustrio with your ongoing change
|
||
let userAccounts = activeUserAccounts | ||
|
||
if ('test' in query && query.test === 'true') { |
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 the getUserAccounts function called anywhere?
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.
@samIndustrio with your ongoing change
fixing tests
*Issue:*https://github.com/bcgov/entity/issues/
Description of changes:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the namex license (Apache 2.0).