[GSK-1087] Avoid multiple requests for license/settings #1262
Conversation
…slicense-requests
…slicense-requests
kevinmessiaen
left a comment
kevinmessiaen
left a comment
There was a problem hiding this comment.
Looks good to me! Some small code refactoring possible but it's not directly related to the PR
frontend/src/stores/user.ts
Outdated
| let token = this.token; | ||
| if (!token) { | ||
| const localToken = getLocalToken(); | ||
| if (localToken) { | ||
| this.token = localToken; | ||
| token = localToken; |
There was a problem hiding this comment.
I don't think that we really need a token variable since it's always equals to this.token
It seems (need to verify) that we can simplify everything by just
this.token = this.token || getLocalToken()
There was a problem hiding this comment.
this doesn't work because getLocalToken() may output null.
that's why there is a double check. let me try to find a better solution to that.
There was a problem hiding this comment.
Done! Could you recheck it?
There was a problem hiding this comment.
In that case something like this should work: this.token = this.token || getLocalToken() || ''
frontend/src/stores/user.ts
Outdated
| const response = await api.getUserAndAppSettings(); | ||
| this.isLoggedIn = true; | ||
| this.userProfile = response.user; | ||
| let appConfig = response.app; | ||
| mainStore.setAppSettings(appConfig); |
There was a problem hiding this comment.
This lines are duplicated with lines 76-80. I think the order of if/else it can be improved to make it more concise
if (!this.isLoggedIn) {
if (mainStore.authAvailable) {
this.token = this.token || getLocalToken();
if (!this.token) {
this.removeLogin();
return;
}
}
const response = await api.getUserAndAppSettings();
this.isLoggedIn = true;
this.userProfile = response.user;
mainStore.setAppSettings(response.app);
}(ideally we should break down to sub function to avoid having too much layers of if)
There was a problem hiding this comment.
I refactored this checkLoggedIn method.
Could you recheck it?
There was a problem hiding this comment.
I saw that you forgot about setting this.token, I fixed that so all good to me
…slicense-requests
|
Kudos, SonarCloud Quality Gate passed! |
Hartorn
deleted the
feature/gsk-1087-a-lot-of-subsequent-settingslicense-requests
branch
Description
This PR aims to fix a bug which request many times to the backend to /license and /settings endpoint.
Related Issue
GSK-1087 (available on Linear)
Type of Change