Leaderboard Update on Submission

[Gehrkej]

Changes:

In this PR I have implemented functionality that will update the "best_attempts" table upon submitting a drill attempt. Currently, it covers the case where it is a users first time completing the drill as well as comparing if an attempt is already in existence.

Related issues: #150 #95

Concerns:

Our current structure of "best_attempts" requires us to upload a completely new map/object for any given drill document. With this structure, a race case could occur where two users get a new high score and upon each of the uploading a new map/object with their updated score, it could erase one of the records.

Proposed fix:

If we structure each of the users within the drill document as a document itself instead of a map/object, we can update the user document directly which would eliminate the case brought up previously although I am unsure what effects this would have on the current leaderboard implementation.

[FrankreedX]

The reason it’s an object/map is cuz firebase needs a document to be followed by a collection, and then a document, etc. It didn’t line up for best attempts.

[solderq35]

Seems to be working atm except for the potential race condition listed above, as well as the leaderboard updates not working quite right in cases where the order of leaderboard sorting does not match the "lowerisbetter" value from drills table.

E.g. Short Putting, 5-15 feet, which sorts by lowest first on the leaderboard while having "lowerisbetter: false" in drills table. It does not update correctly at the moment (checking for lower rather than higher value in new attempts).

Working use cases tested on new commit (previously not working):

• New submission on leaderboard you have not submitted for yet
• Updating your best score on a leaderboard you have submitted to before (Tested on Short Putting, 3-6 feet, which has "lowerisbetter: true" in drills table)

[solderq35]

Would something from here fix the race conditioned mentioned above?

https://firebase.google.com/docs/firestore/manage-data/add-data#update-data

updateDoc seems to be designed to avoid overwriting existing fields, and also see the "Update fields in nested objects" section

[solderq35]

Sorting by lower / higher on the leaderboard now seems to work properly, as does updating best score (based on lowerIsBetter flag)

• sort + update by lowest

  • initial score
    • 
  • updated score (lower)
    • 

• sort + update by highest

  • initial score
    • 
  • updated score (higher)
    • 

[Gehrkej]

I have added numbers to the leaderboard and have accounted for the case there is a tie. For the tie, this is using the full attempt value as of right now and not the truncated display value.

** 2 way Tie Example**

3 way Tie Example

[solderq35]

Need to test this more in actual race case conditions (try to submit from 2 diff accounts at same time). One on emulator, one on iphone or something.

[solderq35]

It seems that setDoc only needs to be called when best_attempts > drillId doesn't exist yet (which should be out of scope for now, only relevant if we want to give the coach ability to create new leaderboards and drills on the fly in the future).

In all other use cases (empty leaderboard, user's first submission on a non-empty leaderboard, user updating their best time), updateDoc can be used. Since only setDoc triggers the race condition between different users, this issue ought to be resolved.

I also removed unused code and the old updateLeaderboard hook to avoid confusion / make it easier to delete existing leaderboard entries for testing (just delete any entries from best_attempts > drillId doc if needed, from Firebase console)

I did a little bit of manual testing (open 2 terminals on PC, run 1 terminal for ios and 1 terminal for android, try to submit from 2 user accounts on the 2 devices at the same time). Everything seemed to update as expected with no race case, both for empty leaderboards and leaderboards with existing entries (although Android emulator does lag a bit, so I wasn't sure if they really uploaded at same time).

[FrankreedX]

Great work!

[solderq35]

Firebase transactions work for get + update, should avoid race conditions in future

Reference: First example code here: https://firebase.google.com/docs/firestore/manage-data/transactions#transactions

Screenshots

[solderq35]

I refactored the functionality for invalidating multiple query keys to a hook (previously all inside the refreshInvalidate component): #149

In addition, I reworked invalidateMultipleQueries hook to only log the successfully invalidated queries, and filter out duplicates:

I kept the original logs commented out as a potential tool for more thorough debugging

[solderq35]

rebased (no conflicts)

[FrankreedX]

I added a commit so check it before merging

[solderq35]

added back react-native-gesture-handler due to this error:

Everything else looked good when testing leaderboard functionality

EDIT:
Ended up installing it via npx as shown: https://stackoverflow.com/a/77962693