Glasgow | Prati Amalden | Module Legacy Code | New feature: Rebloom #57
Conversation
PratiAmalden
added
the
Needs Review
PratiAmalden
changed the title
PratiAmalden
changed the title
illicitonion
left a comment
illicitonion
left a comment
There was a problem hiding this comment.
This is generally looking good, but there's one big thing about the database structure to look at.
From a UX perspective - it's unclear when a rebloom should sort in a timeline - should it be at time of original post or time of original bloom?
Also on a bloom when it says "Rebloomed 1 times", it may be useful to be able to click to display which users rebloomed or similar.
But generally, this is looking really good, well done!
| ALTER TABLE blooms | ||
| ADD COLUMN rebloom_from BIGINT REFERENCES blooms(id), | ||
| ADD COLUMN rebloom_by INT REFERENCES users(id), | ||
| ADD COLUMN rebloom_count INT DEFAULT 0; |
There was a problem hiding this comment.
Ideally you wouldn't need to track rebloom counts in the database - there is a race condition here where values can get out of date (e.g. if you create a rebloom but haven't updated this count yet).
Instead, use SQL relationships to compute this - in a query, you should be able to compute the count of blooms which have this ID as an rebloom_from
Your Python and JS code for querying a Bloom shouldn't have to change, but it removes the need for you to update the original bloom from the python side.
| await apiService.postRebloom(bloom); | ||
|
|
||
| } catch (error) { | ||
| console.error("Rebloom failed:", error); |
There was a problem hiding this comment.
This isn't a user-facing error - how could you surface this to a user?
illicitonion
added
Reviewed
2 participants
No description provided.