Skip to content
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

🐛 fix: organization name doesn't update after another user update it … #432

Merged
merged 3 commits into from
Jul 14, 2023

Conversation

mucoban
Copy link
Contributor

@mucoban mucoban commented Jul 5, 2023

getCurrentOrganization() method checks if organization data exits in local storage. If it doesn't exist, fetches organization data from api and saves it to local storage. This causes the organization name to remain same after another user changes it if there is organization data which is saved is local storage already.
I canceled local storage check for organization data in getCurrentOrganization() method, and therefore the getCurrentOrganization() fetches organization data from api on page load, even if it is saved in local storage.

Screen.Recording.2023-07-05.at.19.44.26.mov

fixed #331

@@ -67,21 +67,12 @@ export class OrganizationService {

getCurrentOrganization(): Observable<IOrganization> {
return new Observable(observer => {
const orgStr = localStorage.getItem(CURRENT_ORGANIZATION());
if (this.organizations.length === 0 || !orgStr || JSON.parse(orgStr)?.plan === undefined) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this changes removed a feature: when one user has more than one organization, when he logout and then login again, the original code would keep the user loging into the same org.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest this change:
image

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the org data exists in localstorage, we take it (with the latest data from backend) otherwise, we would take the first one of the organization list

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pushed a commit applying your suggestion

Copy link
Contributor

@cosmos-explorer cosmos-explorer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you

@cosmos-explorer
Copy link
Contributor

@mucoban can you resolve the conflicts?

@mucoban
Copy link
Contributor Author

mucoban commented Jul 14, 2023

I resolved the conflicts @cosmos-explorer

@cosmos-explorer
Copy link
Contributor

I resolved the conflicts @cosmos-explorer

Thank you

@cosmos-explorer cosmos-explorer merged commit 41a57d3 into featbit:main Jul 14, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants