Implement SQL Repos - #74
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
5470d2c to
dbbd19b
Compare
Graphite Automations"Request reviewers once CI passes" took an action on this PR • (08/10/26)2 reviewers were added to this PR based on Henry Chen's automation. |
|
Code looks good, although I wonder if the match cycle repo could use some per-field update methods like match has. Also, we planning on writing any tests for these? Not sure if there's another ticket for them |
dbbd19b to
b38d8dc
Compare
b38d8dc to
1eefc69
Compare
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
thanks for the feedback il include tests in the next PR. Will update this one with the per field methods |
7421818 to
6e2e2a1
Compare
adb4bd4 to
3df3274
Compare
3df3274 to
b63a2c8
Compare
|
|
One other thing is that you're building out everything together layer by layer, which makes sense with how we planned it out and with AI implementing it, but it would be easier to build it out feature by feature once we get it to a little more stable state. Like having the basics set, and then adding filtering and feedback later down, instead of including it from the start. |
| .id(rs.getInt("id")) | ||
| .period(rs.getString("period")) | ||
| .runAt(rs.getObject("run_at", Instant.class)) | ||
| .totalMembers(rs.getInt("total_members")) |
There was a problem hiding this comment.
Technically, we can calculate this by looking at the match table and then counting the rows right?
One issue with this is that if we have a new pending MatchCycle, then the count could be changing as we add more matches. So then we would have to update this sychronously with adding matches to the match table. Having the same information in two different places is a huge pain to keep synced.
Potentially we can fill these numbers in when we change a MatchCycle from pending to live, so that they're cached, so we don't have to compute them each time. If it's not like read only though, it would still require syncing tho. Like if we edit matches after it goes live.
We should also make a new column for the table that is like: isLive, which we flip when we go send the emails out.
| .param("cycle_id", match.getMatchCycleId()) | ||
| .param("match_score", match.getMatchScore()) | ||
| .param("status", match.getStatus()) | ||
| .param("feedback_a", match.getFeedbackA()) |
There was a problem hiding this comment.
I'd actually remove feedback from this table as well, and make a table for feedback specifically.
It'd be:
feedback_id
match_id
member_id
...feedback content
feedback content could be like 1-5 stars, text field for written things or whatever else we think of. But that leaves it to be a lot more flexible.
3 participants
No description provided.