-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
[GSoC2024] fix and refactor import-dataset-modal code (#7428) #7599
[GSoC2024] fix and refactor import-dataset-modal code (#7428) #7599
Conversation
Refactor code to use useReducer instead of useState. Fixes cvat-ai#7428 'file' and 'fileName' in the uploadParams was not set to null after import. To avoid this changed the code to set 'file' and 'fileName' null after importing is done.
changelog.d/20240313_014112_umangapatel123_refactor_into_usereducer.md
Outdated
Show resolved
Hide resolved
changelog.d/20240313_014112_umangapatel123_refactor_into_usereducer.md
Outdated
Show resolved
Hide resolved
…ducer.md Co-authored-by: Boris Sekachev <[email protected]>
@klakhov can you please review? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you also check the import cypress test? Its failed here
Tests docs
From quick look on the failed test I see the wrong notification is shown. Looks like while importing a project locally incorrect resourse
param is inside uploadParams
.
@klakhov I've made the changes based on your suggestion and resolved the failing Cypress test. Could you please review it now? |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## develop #7599 +/- ##
===========================================
- Coverage 83.46% 83.45% -0.01%
===========================================
Files 373 373
Lines 39739 39739
Branches 3741 3741
===========================================
- Hits 33168 33166 -2
- Misses 6571 6573 +2
|
@klakhov can you please approve the changes? |
@umangapatel123, Thanks for contribution. |
Okay, I will do that. |
Hi @umangapatel123 , can you share with us your progress with test? Are you still going to contribute? |
I am still working on it. yes, I am contributing. I was busy for some days due to university examinations. |
test case for the pr cvat-ai#7599
test case for the pr cvat-ai#7599
Refactor code to use useReducer instead of useState. Fixes #7428
'file' and 'fileName' in the uploadParams were not set to null after import. To avoid this change the code to set 'file' and 'fileName' to null after importing is done.
Motivation and context
This change is required to make the code clear. The previous code uses many
useState
to manage the state but this version usesuseReducer
to manage all the states.In the previous code
file
andfileName
in theuploadParams
were not set to null after importing was done but this version set it to null so it can not use the previous file while importing annotation.it solves issue #7428
How has this been tested?
The changes have been tested manually. The code was tested using different sequences of annotation uploads. The state of each parameter is tested by logging its state to the console.
Checklist
develop
branch[ ] I have updated the documentation accordingly[ ] I have added tests to cover my changes[ ] I have increased versions of npm packages if it is necessary(cvat-canvas,
cvat-core,
cvat-data and
cvat-ui)
License
Feel free to contact the maintainers if that's a concern.