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: total api calls handling #2583

Merged
merged 2 commits into from
Aug 18, 2023
Merged

fix: total api calls handling #2583

merged 2 commits into from
Aug 18, 2023

Conversation

kyle-ssg
Copy link
Member

@kyle-ssg kyle-ssg commented Aug 4, 2023

Thanks for submitting a PR! Please check the boxes below:

  • I have run pre-commit to check linting
  • I have filled in the "Changes" section below?
  • I have filled in the "How did you test this code" section below?
  • I have used a Conventional Commit title for this Pull Request

Changes

Current problems

  • The promise to retrieve organisation usage was incorrect, this is causing the following error in production.

Currently in production the total api call detection does not work, this is because we are incorrectly parsing the response.
image

  • The action to get organisation usage is in the render function, this should never happen, side affects (especially API and set state calls) should never be there. This is causing the getOrganisation usage to be rapidly called in production. Luckily Redux RTK detects duplicate api call requests and only hits it once.
  • The check to call get organisation usage included whether the total api calls state was 0, the problem with this is that it is infinitely called for new organisations and projects. The only reason why this is not breaking production is because of the first bullet point (It does call around 10 or so times but eventually fails and stops doing it).

Changes

  • Fix the promise of getOrganisationUsage to return just the data of the API call
  • Only call the organisation usage endpoint when we don't have the data for the active organisation (on mount, on organisation switch)

How did you test this code?

Tested retrieving the initial usage and then switching organisations.

@github-actions github-actions bot added the front-end Issue related to the React Front End Dashboard label Aug 4, 2023
@kyle-ssg kyle-ssg changed the title Fix total api calls handling fix total api calls handling Aug 4, 2023
@kyle-ssg kyle-ssg changed the title fix total api calls handling fix: total api calls handling Aug 4, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Aug 4, 2023

Uffizzi Preview deployment-32636 was deleted.

@vercel
Copy link

vercel bot commented Aug 4, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 9, 2023 2:41pm
flagsmith-frontend-preview ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 9, 2023 2:41pm
flagsmith-frontend-staging ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 9, 2023 2:41pm

@matthewelwell
Copy link
Contributor

@kyle-ssg is this separate to the graph in the organisation settings then because that seems to be working?

image

@kyle-ssg
Copy link
Member Author

kyle-ssg commented Aug 9, 2023

It is separate yes, this is for API usage warnings. Can see it erroring

image

However I think there's a more elegant way of managing this so will mark the PR as draft and adjust later today.

@kyle-ssg kyle-ssg marked this pull request as draft August 9, 2023 09:22
@kyle-ssg kyle-ssg merged commit ff0da20 into main Aug 18, 2023
18 checks passed
@kyle-ssg kyle-ssg deleted the fix/usage_totals branch August 18, 2023 07:20
This was referenced Aug 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
front-end Issue related to the React Front End Dashboard
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants