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

Ta375/arrived race conditions #228

Merged
merged 2 commits into from
Nov 18, 2024
Merged

Ta375/arrived race conditions #228

merged 2 commits into from
Nov 18, 2024

Conversation

3TTemi
Copy link
Contributor

@3TTemi 3TTemi commented Oct 8, 2024

Summary

This pull request handles race conditions that was most caused by spamming of the I've arrived button for challenges.

Problem was in Internal testing causing users to get more than the maximum points for the challenge

https://www.notion.so/cornell-go/1dab9f87f80b46cf92d4b61e769b7639?v=f246714d0a2b43eba2bfb0466fa7af77&p=0009d2e0093a48fdab6260011190b91e&pm=s

Remaining TODOs:

  • Some further testing when TestFlight is back up

@dti-github-bot
Copy link
Member

[diff-counting] Significant lines: 43.

Copy link
Collaborator

@Shengle-Dai Shengle-Dai left a comment

Choose a reason for hiding this comment

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

Introducing a boolean variable is a nice solution for the race condition!
Also, I think it'll be more robust if we restrict backend logic to verify completed challenges as well.

@shah-esha
Copy link
Collaborator

This is a really simple and effective solution to the race condition! It may also be beneficial to add a check within the onPressed functionality to make sure the user's score does not surpass the maximum.

Copy link
Contributor

@BrianLa0616 BrianLa0616 left a comment

Choose a reason for hiding this comment

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

Great work!

I tested it by spamming the button while having print statements, and it only checks/sends for completion once. Hopefully this is robust enough and we don't need to also have it in the backend.

@3TTemi 3TTemi merged commit 3b1f6ac into master Nov 18, 2024
12 checks passed
@3TTemi 3TTemi deleted the ta375/arrived-race-conditions branch November 18, 2024 22:05
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.

5 participants