Skip to content

Sheffield | 25-SDC-Nov | Hassan Osman | Sprint 1 | New Feature: Rebloom - New - #262

Open
HassanOHOsman wants to merge 26 commits into
CodeYourFuture:mainfrom
HassanOHOsman:feature/re-bloom
Open

Sheffield | 25-SDC-Nov | Hassan Osman | Sprint 1 | New Feature: Rebloom - New#262
HassanOHOsman wants to merge 26 commits into
CodeYourFuture:mainfrom
HassanOHOsman:feature/re-bloom

Conversation

@HassanOHOsman

@HassanOHOsman HassanOHOsman commented Aug 20, 2026

Copy link
Copy Markdown

Self checklist

  • I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title
  • My changes meet the requirements of the task
  • I have tested my changes
  • My changes follow the style guide

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.

@HassanOHOsman HassanOHOsman added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. Module-Legacy-Code The name of the module. 📅 Sprint 1 Assigned during Sprint 1 of this module labels Aug 20, 2026
@HassanOHOsman HassanOHOsman changed the title Feature/re bloom Sheffield | 25-SDC-Nov | Hassan Osman | Sprint 1 | New Feature: Rebloom - New Aug 20, 2026

@illicitonion illicitonion left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread db/schema.sql Outdated
UNIQUE(hashtag, bloom_id)
);

CREATE TABLE reblooms (

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've solved this now.

@illicitonion illicitonion added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Aug 21, 2026
@github-project-automation github-project-automation Bot moved this from Backlog to Done in Purple Forest Kanban Aug 21, 2026
@HassanOHOsman HassanOHOsman reopened this Aug 21, 2026
@github-project-automation github-project-automation Bot moved this from Done to Backlog in Purple Forest Kanban Aug 21, 2026
@HassanOHOsman HassanOHOsman added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. labels Aug 21, 2026
@HassanOHOsman

Copy link
Copy Markdown
Author

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 illicitonion left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally looks good, but a few questions and still a few references to the reblooms table

Comment thread backend/data/blooms.py Outdated
with db_cursor() as cur:
cur.execute(
"""
SELECT COALESCE(original_bloom_id, id)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you need this COALESCE? When would the original_bloom_id not be present as an id for some row?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my understand is that since I've discarded the re-bloom table, and when i rebloom i need to figure out the original/root id for it. Hence the use of COALESCE(original_bloom_id, id).

Comment thread backend/data/blooms.py Outdated

cur.execute(
"""
INSERT INTO reblooms (

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought you deleted this table?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I did indeed, perhaps missed to get rid of all references to it.

Comment thread backend/data/blooms.py Outdated
blooms.original_bloom_id,
(
SELECT COUNT(*)
FROM reblooms

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This table no longer exists?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I must've missed to remove all reference somehow. That's done now anyway.

Comment thread backend/data/blooms.py Outdated
blooms.original_bloom_id,
(
SELECT COUNT(*)
FROM reblooms

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As above

@HassanOHOsman

Copy link
Copy Markdown
Author

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

RE:

  • A small banner appears above the post saying "🔁 Rebloomed by sample" so it is now clear in the UI both who originally bloomed it, and who re-bloomed it.

  • All re-bloomed blooms now show in sample's timeline without refreshing the page.

Please review and let me know if you still want me to do anything else. Thank you Daniel!

@illicitonion illicitonion added Complete Volunteer to add when work is complete and all review comments have been addressed. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Aug 26, 2026
@illicitonion

Copy link
Copy Markdown
Member

Looks good, well done!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Complete Volunteer to add when work is complete and all review comments have been addressed. Module-Legacy-Code The name of the module. 📅 Sprint 1 Assigned during Sprint 1 of this module

Projects

Status: Backlog

Development

Successfully merging this pull request may close these issues.

2 participants