-
Notifications
You must be signed in to change notification settings - Fork 1
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: Add app switcher button #545
Conversation
const { activeProject } = useActiveProject(); | ||
|
||
const query = useRestQuery( | ||
'GET /v1/life-research/projects/:projectId/app-config', | ||
'GET /v1/life-research/projects/:projectId/app-configs/list', |
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.
Somewhere we will need to make this smarter, probably at the API level. Currently this list will contain all configs that exist for the project but an App user should likely only be able to change between configs that they belong to (cohort or user list). This is probably okay until we identify how to manage that logic. I imagine it could end up being different per client.
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.
Total oversight on my part, I'll work on updating API side
97c7b31
to
cf09858
Compare
cf09858
to
dee2c87
Compare
Pull Request Test Coverage Report for Build 8693577141Details
💛 - Coveralls |
appTiles: mockAppTiles, | ||
}, | ||
|
||
// Bug in mocker seems to not be able to tell /:id and /list apart |
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 is thinking that list
is the :id
. Did you try swapping the definition order of the route mocks? It seems like msw/node
prepends the routes so I would think/hope that mocking /:id
then mocking /list
would produce the desired result. Regardless though this seems fine.
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'd expect that if /list
matches multiple routes that the mocking library would prefer the exact match over the wildcard which is sort of why I described this as a bug but perhaps you are right about that.
🎉 This PR is included in version 12.1.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Changes
Screenshots