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: bug with file requirements not changing on delete of file #2417

Merged
merged 2 commits into from
Nov 13, 2023

Conversation

Mike-Heneghan
Copy link
Contributor

What:

  • Update the removeFile function to set the updated file list to state.

Why:

  • This was written when the state was being updated directly and was therefore broken when the fileList was deep cloned.

Screen recording:

Bug:

Screen.Recording.2023-11-13.at.15.49.59.mov

Post fix:

Screen.Recording.2023-11-13.at.16.52.55.mov

- Reinstate the `merge` which updates state directly
- This isn't best practice but does show how the bug started happening
- Create a clone of fileList before removing the file
- Remove the file and update state with the setter
@Mike-Heneghan Mike-Heneghan requested a review from a team November 13, 2023 16:54
@Mike-Heneghan Mike-Heneghan self-assigned this Nov 13, 2023
Copy link
Member

@jessicamcinchak jessicamcinchak left a comment

Choose a reason for hiding this comment

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

Thanks for catching this! Would be great to have a test case to cover this scenario going forward, but happy for fix to go in first - your judgement / decision based on time to write test!

Copy link

github-actions bot commented Nov 13, 2023

Removed vultr server and associated DNS entries

@Mike-Heneghan Mike-Heneghan merged commit 521e02d into main Nov 13, 2023
12 checks passed
@Mike-Heneghan Mike-Heneghan deleted the mh/bug-file-upload-and-label-delete branch December 13, 2023 11:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants