Mutualize fetching functions #2099
Conversation
src/services/utils.js
Outdated
| throw error; | ||
| }); | ||
| }; | ||
| import { fetchAppsAndUrls, fetchEnv } from '@gridsuite/commons-ui'; | ||
|
|
||
| export const getToken = () => { |
There was a problem hiding this comment.
To answer the above : yes it will be necessary to update commons ui in this PR
Regarding getToken maybe we could mutualize it as well, I just think it'd be wiser to limit the number of functions migrated at once to prevent risks of unexpected side effects
There was a problem hiding this comment.
Well it's the point of the ticket, mutualize what we can, it won't be done after if we don't do it now, i'm pretty sure, so well...
You can check with a TL but for me, it need to be done here alos.
There was a problem hiding this comment.
I've just talked about it with @TheMaskedTurtle, we ought to prioritize fixing commons-ui to release gridstudy deadlock.
I can create a following ticket to clean up the rest of utils functions getVersion, auth function, etc...
There was a problem hiding this comment.
This said I can clean up getToken since it's already in commons ui, didn't noticed earlier
There was a problem hiding this comment.
Ok, yes deleting getToken is really fast to do.
Or instead of creating a new ticket, you can juste make 2 PR, this one to unblock the devs, and an other one to finish the ticket ?
There was a problem hiding this comment.
Yes I figured this too later, fine by me I'll do a second PR later then
| export const getToken = () => { | ||
| const state = store.getState(); | ||
| return state.user.id_token; | ||
| export const FetchStatus = { |
There was a problem hiding this comment.
Why did you moved it back ?
There was a problem hiding this comment.
Those are more related to component logic than api services and this one is slightly different than the one currently in commons ui, because it's use in gridexplore has different labels. So first we need to harmonize their values and as said earlier I'd want this PR to not be too bloated, I can include this in the next one
Need #2187
Depends on gridsuite/commons-ui#455