ZA | 25-SDC-JULY | Luke Manyamazi | Sprint 1 | New Feature Rebloom by Luke-Manyamazi · Pull Request #58 · CodeYourFuture/Module-Legacy-Code

Luke-Manyamazi · 2025-12-13T18:35:00Z

Learners, PR Template

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

Changelist

Add a "re-bloom" button so users can share a bloom to their own feed and followers.
UI should clearly show original author and who re-bloomed it, distinguishing it from the user’s own blooms.
Display rebloom count on each bloom; timestamp can reflect rebloom time to indicate when it was shared.

Questions

Ask any questions you have for your reviewer.

illicitonion · 2026-01-21T15:31:01Z

+            # Returning 0 or raising an error are typical ways to indicate failure
+            return 0


I don't think any other code in this codebases uses magic return values to indicate errors, and the code that calls this doesn't pass this error back to the client. You need to handle errors differently here.

Good point — using magic return values hides failures from the caller.
I’ve refactored the rebloom logic to raise exceptions instead.

illicitonion · 2026-01-21T15:32:22Z

+        print(f"Error adding rebloom: User with ID {user_id} not found.")
+        return None


This doesn't bubble the error back up to the caller - the caller will need to know this failed.

This applies throughout your change.

illicitonion · 2026-01-21T18:21:52Z

+        })
+
+@jwt_required()
+def check_rebloom_status(bloom_id_str):


What are the trade-offs of having this be a stand-alone function that's called per bloom vs including this information up-front when getting a list of blooms?

illicitonion · 2026-01-21T18:26:20Z

+        try:
+            # First create the new bloom
+            rebloom_bloom_id = add_bloom(
+                sender=sender_user,  # <<< Use the fully hydrated User object


AFAICT add_bloom only actually uses the sender ID, an no other data from the sender - might it be simpler to make add_bloom take just the id, and change the other call-sites from sender to sender.id, rather than needing to add a whole extra database roundtrip to turn the sender ID into a sender, just to take only the ID back out of it?

illicitonion · 2026-01-21T18:26:48Z

+    This is necessary for operations like reblooming where only the ID is known.
+    """
+    with db_cursor() as cur:
+        # NOTE: Adjust column names if they are different in your 'users' table


What does this comment mean?

illicitonion · 2026-01-21T18:30:15Z

+        rebloom_id = blooms.add_rebloom(
+            user_id=current_user.id,
+            original_bloom_id=bloom_id,
+            content=original_bloom.content  # FIX: .content not ['content']


What is this comment about?

illicitonion · 2026-01-21T18:33:20Z

    app.json = CustomJsonProvider(app)

-    # Configure CORS to handle preflight requests
+    # --- CORS FIX APPLIED HERE ---


Things seemed to be working ok without this change - why did you need to change it?

illicitonion · 2026-01-21T18:34:21Z

 );
+
+-- Add rebloom_count to blooms table (to track total reblooms)
+ALTER TABLE blooms ADD COLUMN rebloom_count INTEGER DEFAULT 0;


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 original_bloom_id

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.

illicitonion · 2026-01-21T18:35:02Z

+ALTER TABLE blooms ADD COLUMN original_bloom_id BIGINT REFERENCES blooms(id);
+
+-- Create reblooms table to track the rebloom relationship
+CREATE TABLE reblooms (


This table shouldn't need to exist - all of the information should already be present in the blooms table for you to do some joins to infer this information.

As far as I can tell this table isn't used any more, but it's still created? Please remove.

illicitonion · 2026-01-21T18:35:26Z

+
+  const rebloomIndicator = document.createElement('div');
+  rebloomIndicator.className = 'rebloom-indicator';
+  rebloomIndicator.innerHTML = `


There's an XSS vulnerability here by using innerHTML

Luke-Manyamazi · 2026-02-15T13:20:03Z

Hi Daniel

I have done the following:

Fixed rebloom creation by fetching full User objects and preventing missing argument errors.
Removed rebloom_count updates in Python; counts are now computed dynamically via SQL to avoid race conditions.
Secured frontend against XSS by replacing innerHTML with safe DOM methods and properly encoding hashtags and usernames.
Improved endpoint error handling, type checks, and JWT integration for login, registration, and rebloom actions.

illicitonion

A lot of this is looking a lot better, but there's still a lot of unrelated changes in here, and a bunch of unused code. Please take a look, and make sure:

You've addressed and resolved all comments.
All of the code in this PR is needed and related to this change.

It's much harder for a reviewer to see if your code is working or look for issues when half of the change isn't related.

illicitonion · 2026-02-17T16:47:33Z

This file seems to have a lot of unrelated changes to it?

illicitonion · 2026-02-17T16:49:34Z

+
+    already_rebloomed = blooms.has_user_rebloomed(current_user.id, bloom_id)
+
+    if already_rebloomed:


This kind of "toggle" behaviour can be complicated to support well in clients - they may retry requests (e.g. if the network blipped) which can accidentally undo changes.

In general I'd recommend API design where a user clearly expresses intent to rebloom or un-rebloom something, and will get an error if that's not possible, rather than sometimes undoing the action. The UI can re-use the same button, but it should typically talk to a different backend endpoint.

illicitonion · 2026-02-17T16:51:03Z

+ALTER TABLE blooms ADD COLUMN original_bloom_id BIGINT REFERENCES blooms(id);
+
+-- Create reblooms table to track the rebloom relationship
+CREATE TABLE reblooms (


As far as I can tell this table isn't used any more, but it's still created? Please remove.

Luke-Manyamazi · 2026-02-18T06:01:13Z

Mistakenly closed this PR

Luke-Manyamazi · 2026-02-18T06:01:34Z

Opening it to finish with review edits

illicitonion

This generally looks decent, but it doesn't look like it works for me?

I ran it locally, and when I first logged in I got an error prompt.
I dismissed it and tried to rebloom something, and got an error alert" Unexpected token '<', "<!DOCTYPE "... is not valid JSON

Luke-Manyamazi · 2026-03-07T17:41:20Z

"Hi Daniel, I’ve refactored the Rebloom feature to address your feedback:

Fixed XSS: Replaced innerHTML with createTextNode for safe rendering.

Removed Race Conditions: rebloom_count is now calculated via SQL subqueries instead of a manual column.

JSON Fix: Standardized all error responses to jsonify() to resolve the Unexpected token < crash.

Efficiency: Streamlined add_bloom to accept sender_id directly, removing redundant DB lookups.

Luke-Manyamazi · 2026-03-07T18:59:01Z

Hi Daniel, I was getting an error while trying to rebloom, but it's fixed now.

Here’s a summary of the changes:

Fix: Rebloom Functionality & Auth Persistence
I resolved the 404/422 errors by aligning the frontend and backend layers:
Database: Added the missing rebloom_id column to the blooms table.
Routing: Corrected the fetch URL in bloom.mjs (removed the /api prefix) to match the Flask route.
Auth: Fixed the "Token: null" issue by ensuring apiService saves the JWT to localStorage during login/signup.
The feature now successfully creates reblooms with a valid 200 OK response.

github-project-automation Bot moved this to Backlog in Purple Forest Kanban Dec 13, 2025

github-project-automation Bot added this to Purple Forest Kanban Dec 13, 2025

Luke-Manyamazi added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Dec 13, 2025

illicitonion reviewed Jan 21, 2026

View reviewed changes

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 Jan 21, 2026

Luke-Manyamazi 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. labels Feb 15, 2026

illicitonion reviewed Feb 17, 2026

View reviewed changes

illicitonion removed the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Feb 17, 2026

Luke-Manyamazi closed this Feb 17, 2026

Luke-Manyamazi force-pushed the New-feature-rebloom branch from dca08db to 921f365 Compare February 17, 2026 20:57

github-project-automation Bot moved this from Backlog to Done in Purple Forest Kanban Feb 17, 2026

Luke-Manyamazi reopened this Feb 18, 2026

github-project-automation Bot moved this from Done to Backlog in Purple Forest Kanban Feb 18, 2026

illicitonion reviewed Feb 25, 2026

View reviewed changes

Comment thread backend/endpoints.py Outdated

Luke-Manyamazi force-pushed the New-feature-rebloom branch from 67513e4 to 3c9f07b Compare March 7, 2026 17:22

Luke-Manyamazi added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. 📅 Sprint 1 Assigned during Sprint 1 of this module labels Mar 7, 2026

Luke-Manyamazi closed this Apr 17, 2026

Luke-Manyamazi force-pushed the New-feature-rebloom branch from e9a955d to 921f365 Compare April 17, 2026 10:55

github-project-automation Bot moved this from Backlog to Done in Purple Forest Kanban Apr 17, 2026

Luke-Manyamazi deleted the New-feature-rebloom branch April 21, 2026 15:50

		# Returning 0 or raising an error are typical ways to indicate failure
		return 0

		print(f"Error adding rebloom: User with ID {user_id} not found.")
		return None


		already_rebloomed = blooms.has_user_rebloomed(current_user.id, bloom_id)

		if already_rebloomed:

Uh oh!

Uh oh!

Conversation

Luke-Manyamazi commented Dec 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Learners, PR Template

Changelist

Questions

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Luke-Manyamazi commented Feb 15, 2026

Uh oh!

illicitonion left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Luke-Manyamazi commented Feb 18, 2026

Uh oh!

Luke-Manyamazi commented Feb 18, 2026

Uh oh!

illicitonion left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Luke-Manyamazi commented Mar 7, 2026

Uh oh!

Luke-Manyamazi commented Mar 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Luke-Manyamazi commented Dec 13, 2025 •

edited

Loading