Health-Informatics-UoN / Carrot-Mapper

Carrot: Convenient And Reusable Rapid Omop Transformer.
https://carrot4omop.ac.uk
MIT License
12 stars 3 forks source link

Fix Mapper stops responding when add/delete concept quickly #762

Closed AndrewThien closed 1 week ago

AndrewThien commented 1 week ago

Changes

This PR fixes the bug which is causing the Mapper to stop responding when adding/deleting concepts quickly.

Carrot now can handle deleting or adding concepts by the users who are doing them quickly, by using React.useState to update only the affected records, not revalidating the whole table after the actions, and using React.memo to prevent unnecessary tags loading.

Many many thanks and credits to @AndyRae

Closes #761

Checks

Important: please complete these before merging.

github-actions[bot] commented 1 week ago
Tests Skipped Failures Errors Time
25 1 :zzz: 1 :x: 0 :fire: 4.427s :stopwatch:
spco commented 1 week ago

It would be helpful if you could provide a paragraph here on what the issue was, and what the fix is? For posterity!

AndrewThien commented 1 week ago

posterity

Hi @spco, the fix and the original issue (#761) have been updated on the PR's comment. Thanks!

spco commented 1 week ago

I was hoping for an explanation at a more technical level: what was the cause of the issue, rather than the symptom? And what (at a high level) is the fix? Thanks!

AndrewThien commented 1 week ago

Hi @spco, So the cause of the issue is that the page would re-fetch the whole table and its record's data (which is often heavy), instead of the data of the affected record, after a successful adding or deleting concept. This process took time (and was unnecessary) and while this process happened, if the users added/deleted other concepts, the app would stop responding/crash. Therefore, the fix is making the app re-fetch only the data of the affected record, basically. --> Making the app respond faster and more effectively --> If users want to add/delete quickly concepts, they can do it now --> Fix the bug Hope this helps :D

spco commented 1 week ago

Ok, so just so I understand, is it technically a race condition? And if so, the fix is not to remove a race condition from possibly occurring, but just reduce the amount of work needed so the race condition is less likely to occur?

AndrewThien commented 1 week ago

Ok, so just so I understand, is it technically a race condition? And if so, the fix is not to remove a race condition from possibly occurring, but just reduce the amount of work needed so the race condition is less likely to occur?

Yes, so this is also about the logical fix as well, I suppose. Users can now only add or delete concepts after the previous process (which now is also very fast, thanks to reloading only the affected record) has been done --> so the race may not possibly happen, to the extent that I am trying now.

spco commented 1 week ago

Thanks @AndrewThien