Augment gameboards with "saved to account" info on book pages & My Assignments

[axlewin]

No description provided.

[codecov]

Codecov Report

❌ Patch coverage is 0% with 55 lines in your changes missing coverage. Please review.
✅ Project coverage is 40.31%. Comparing base (30cfdb2) to head (325d08d).
⚠️ Report is 9 commits behind head on main.

Files with missing lines	Patch %	Lines
.../ac/cam/cl/dtg/isaac/api/managers/GameManager.java	0.00%	24 Missing ⚠️
.../cl/dtg/isaac/dao/GameboardPersistenceManager.java	0.00%	24 Missing ⚠️
...n/java/uk/ac/cam/cl/dtg/isaac/api/PagesFacade.java	0.00%	5 Missing ⚠️
...a/uk/ac/cam/cl/dtg/isaac/api/AssignmentFacade.java	0.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #789      +/-   ##
==========================================
- Coverage   40.39%   40.31%   -0.08%     
==========================================
  Files         547      547              
  Lines       23722    23773      +51     
  Branches     2882     2893      +11     
==========================================
+ Hits         9582     9584       +2     
- Misses      13245    13288      +43     
- Partials      895      901       +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[jsharkey13]

I am sure this would work fine, but I'd like to both make it more efficient, and try and simplify the sheer number of different methods here.

[jsharkey13]

This loop, like the one below it, will make many database queries. For book pages, this might be limited to 4 or 5, but for My Assignments it could be hundreds for long-term users (and those in our year-long programmes or events with weekly assignments).

A new method that does this lookup in bulk has been on my mind for a while.

SELECT gameboard_id FROM user_gameboards WHERE user_id = ? AND gameboard_id = ANY(?);

would work, although the performance of this will drop off rapidly once the list goes about maybe 50 elements.

Perhaps the right approach is then two new public methods on the GameboardPersistenceManager, and to replace isBoardLinkedToUser. One takes a list of board IDs and returns a set of those the user is connected to, the other just returns all gameboard IDs connected to the user. We then use the second method whenever the first method is provided with too many gameboard IDs. This is the same approach PgQuestionAttempts::getMatchingLightweightQuestionAttempts takes.

public Set<String> getGameboardIdsLinkedToUser(final Long userId, final Collection<String> gameboardIds)

public Set<String> getGameboardIdsLinkedToUser(final Long userId)

The only gotcha is documenting in the JavaDoc for the first one that the returned set may contain more IDs than the filtered list, so set membership must still be checked.

Then isBoardLinkedToUser just becomes calling the first method with a singleton list and then checking whether the ID is in the returned set. A quick look at the query plans with EXPLAIN ANALYSE suggests that using the query above for a single board ID produces exactly the same query plan and duration as the count(*) approach it would replace.

[jsharkey13]

We use a mix of Collection<String> and List<String> for gameboardIds in the data manager at the moment, so I don't mind which you pick. Since we're returning a set, order can't matter, so flexibility seems best? But there may be something I have missed.

That PgQuestionAttempts method has a good overview of how to use SQL arrays for the = ANY(?) business. We do use conn.createArrayOf(...) in getGameboardsByIds in this file, but unlike the other places we don't call .free() afterwards and I'm not sure if that's bad but I think it might be.

[jsharkey13]

Then once these exist, you will need to use them. I think only one method should add user information; separate out the concerns of "augment with questions" and "augment with user link data". (You can both of these with a singleton list for the "get one gameboard fully augmented" case, and one or the other for the other cases).

I.e. I think the method augmentGameboardWithQuestionAttemptInformationAndUserInformation needs to go.

[jsharkey13]

This for loop has the same issue.

[jsharkey13]

(Might also be worth checking that there are no other sneaky for loops doing this augmentation board-by-board as part of this, too!)