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.

Sign up for GitHub

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

Jump to bottom

Bugfix/too many api calls #152

Merged
merged 3 commits into from
Feb 9, 2021
Merged

Bugfix/too many api calls #152

merged 3 commits into from
Feb 9, 2021

Conversation

saulipurhonen
Copy link
Contributor

Description

Home page used to make an API call for each folderID in user data. In this fix we get the data with single API call.

Original problem also introduced problems with async calls and component unmounting which resulted in React error messages for cleaning up useEffect -methods. This same behavior caused errors in auto-save feature in form submissions and was prone to memory leaks.

Related issues

Fixes #150 and #151. Depending on CSCfi/metadata-submitter#193

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Changes Made

  • Get folder and map folder data with single GET call to /folders
  • Fixed auto-save timer reset problem in form submission.

Testing

  • Tests do not apply

@saulipurhonen saulipurhonen added the bug Something isn't working label Feb 8, 2021
@saulipurhonen saulipurhonen self-assigned this Feb 8, 2021
@saulipurhonen saulipurhonen force-pushed the bugfix/too-many-api-calls branch from 74f2c72 to 2d7ff8c Compare February 8, 2021 06:52
@saulipurhonen saulipurhonen force-pushed the bugfix/too-many-api-calls branch from 2d7ff8c to 801049c Compare February 8, 2021 06:54
@saulipurhonen saulipurhonen force-pushed the bugfix/too-many-api-calls branch from 801049c to d3090c6 Compare February 8, 2021 07:01
@saulipurhonen
Copy link
Contributor Author

saulipurhonen commented Feb 8, 2021 •
edited
Loading

E2E test for home.spec.js doesn't seem to pass since the backend doesn't have the fix for folder data yet.

hannyle
hannyle approved these changes Feb 8, 2021
View reviewed changes
Copy link
Contributor

@hannyle hannyle left a comment

Choose a reason for hiding this comment

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

Looks good to me. Much better now that we have only 1 api call to get all folders.

@saulipurhonen saulipurhonen merged commit 848f79f into develop Feb 9, 2021
@saulipurhonen saulipurhonen deleted the bugfix/too-many-api-calls branch February 9, 2021 09:54
@blankdots blankdots mentioned this pull request Feb 12, 2021
Merged
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hannyle hannyle hannyle approved these changes

@blankdots blankdots blankdots approved these changes

@lilachic lilachic Awaiting requested review from lilachic

Assignees

@saulipurhonen saulipurhonen

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@saulipurhonen @blankdots @hannyle