-
-
Notifications
You must be signed in to change notification settings - Fork 180
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
[TASK-1096] Implementation of queries for organization #5197
base: main
Are you sure you want to change the base?
Conversation
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.
- As funny as they are, I'm ok with naming because they are consistent and descriptive.
- blocked by refactor(account): query-ify OrganizationContext TASK-1194 #5164
- blocked by decision in internal chat
useOrganizationQuery hooks
142d191
to
8f0b4a9
Compare
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.
Just some questions
@@ -147,6 +147,8 @@ export interface Organization { | |||
modified: string; | |||
slug: string; | |||
is_owner: boolean; | |||
is_mmo: boolean; |
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 these be optional until the backend will actually return it? Or just returning false in the query is safe enough
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.
As far as I know, and from my tests, backend is already returning these fields. I'm setting it to false in the current query because when talking to @jamesrkiger we thought it would be a good idea to force the default "false" values until we have the front end properly implemented to avoid issues.
Look good to me now :) |
Checklist
./python-format.sh
to make sure that your code lints and that you've followed our coding styleDescription
This PR implements reactQuery hooks to be used on organization development.
Notes
The
useOrganizationQuery
hook was modified:mmosEnabled
feature flag is istrue
we created a mocked response, so we have better control on frontend development. This mock can be updated by the developer to create proper testing situationsenabled
property was added to the query, to make sure it can only run after thesessionStore
is loaded, since we need thecurrentAccount.organization.url
to load the organization details in the new implementation, and it should not affect the current implementation.Testing
Check for the mock data:
useOrganizationQuery
is currently being used inaccountSidebar
, so one way of testing this implementation is to use:This way one can see the result of the data mocking with and without the
mmosEnabled
feature flag.Check for the conditional query
On can comment out or add a
setTimeout
insession.tsx
to prevent thesessionStorage.isPending
from being set to false, causing the organization query to not run, waiting for it.Other than code-related tests, there should be no changes in the current functionality.