RTIInternational / SMART

Smarter Manual Annotation for Resource-constrained collection of Training data
MIT License
218 stars 30 forks source link

Handle bug where the same card was appearing multiple times #328

Closed AstridKery closed 3 months ago

AstridKery commented 3 months ago

This MR addresses a bug which was occurring whenever someone either:

Either action would cause SMART to call fetchCards multiple times which would lead to the same card deck being pushed two to three times to the card state on the front end.

I looked around for the reason this was happening. Even if you comment out all calls to fetchCards that run after actions and only leave the one in the CardContainer constructor the double call still occurs (which suggests to me that the constructor call is the source. @andykawabata any thoughts on where this call should go so that fetchCards runs when the component is mounted?).

This MR is a workaround that makes the pushCard action check if a card already exists in the deck before pushing. It also adds more error handling for double-clicking since sometimes the first check doesn't catch it.

andykawabata commented 3 months ago

@AstridKery

Even if you comment out all calls to fetchCards that run after actions and only leave the one in the CardContainer constructor the double call still occurs

On dev branch I tried commenting this line, and it seemed to fix the issue https://github.com/RTIInternational/SMART/blob/0824189ce56b46bc0face38f61c34c7c849bc6f3/frontend/src/actions/card.js#L79

Though I have only been testing for the case of "finished the batch of cards handed out" - not sure what you mean by "double click on a card"

AstridKery commented 3 months ago

Though I have only been testing for the case of "finished the batch of cards handed out" - not sure what you mean by "double click on a card"

@andykawabata I'm talking about if you click a label button twice in quick succession when labeling a card. It's a common thing for users to accidentally do. It sends two calls of annotate_card to the backend which returns an error and forces the deck to refresh (the error is also meant to help in cases where someone's cards were un-assigned by an admin).

For the line you indicated, that's one of the calls that run after actions that I was talking about. There's one of those for skipping or adjudicating cards as well. I tried commenting out all of those but when I added console.log(props.cards) to https://github.com/RTIInternational/SMART/blob/0824189ce56b46bc0face38f61c34c7c849bc6f3/frontend/src/containers/card_container.jsx#L12 it still seemed like the deck was being double-filled at the end of each batch.... I'll check again real quick.

andykawabata commented 3 months ago

@AstridKery Oh I see - I was only testing with a project with 3 labels. It looks like it's more of a issue with projects with label select/dropdown. Will continue looking it to it.