Sheffield | 25-SDC-Nov | Hassan Osman | Sprint 1 | New Feature: Rebloom - New - #262
Sheffield | 25-SDC-Nov | Hassan Osman | Sprint 1 | New Feature: Rebloom - New#262HassanOHOsman wants to merge 19 commits into
Conversation
HassanOHOsman
added
Needs Review
HassanOHOsman
changed the title
illicitonion
left a comment
illicitonion
left a comment
There was a problem hiding this comment.
This generally looks good, but a few thoughts, as well as not needing a reblooms table:
- Right now when you show a rebloom it doesn't look like a rebloom - it looks like a new bloom by the reblooming user - how could you indicate that it's a rebloom (ideally showing what user it's a rebloom from)?
- I think there's a caching issue that means when you rebloom something, your own profile timeline doesn't get invalidated, so if you do a rebloom then go to your own profile it doesn't show up until you refresh
| UNIQUE(hashtag, bloom_id) | ||
| ); | ||
|
|
||
| CREATE TABLE reblooms ( |
There was a problem hiding this comment.
You shouldn't need a separate table for reblooms - you have all the information you need in the blooms table with original_bloom_id, but may need to do some joins to get the data out
illicitonion
added
Reviewed
d42592a to
b2dcd10
Compare
2. add a test content for th erebloom counter
HassanOHOsman
added
Needs Review
|
Not the best... still needs some work. however, the caching issue is now resolved as far as I could tell from my end. Let me know if I still need to change or do anything. Thank you! |
illicitonion
left a comment
illicitonion
left a comment
There was a problem hiding this comment.
Generally looks good, but a few questions and still a few references to the reblooms table
| with db_cursor() as cur: | ||
| cur.execute( | ||
| """ | ||
| SELECT COALESCE(original_bloom_id, id) |
There was a problem hiding this comment.
Why do you need this COALESCE? When would the original_bloom_id not be present as an id for some row?
|
|
||
| cur.execute( | ||
| """ | ||
| INSERT INTO reblooms ( |
There was a problem hiding this comment.
I thought you deleted this table?
| blooms.original_bloom_id, | ||
| ( | ||
| SELECT COUNT(*) | ||
| FROM reblooms |
There was a problem hiding this comment.
This table no longer exists?
| blooms.original_bloom_id, | ||
| ( | ||
| SELECT COUNT(*) | ||
| FROM reblooms |
2 participants
Self checklist
Questions
I just could not display the reblooms on my logged in user's timelines. I've spent a few days on it but to no avail.