Skip to content

WestMidlands | SDC Nov 2025| Sara Tahir | Sprint 1| ExtraLong Bloom#110

Open
SaraTahir28 wants to merge 1 commit intoCodeYourFuture:mainfrom
SaraTahir28:feature/bloomlength
Open

WestMidlands | SDC Nov 2025| Sara Tahir | Sprint 1| ExtraLong Bloom#110
SaraTahir28 wants to merge 1 commit intoCodeYourFuture:mainfrom
SaraTahir28:feature/bloomlength

Conversation

@SaraTahir28
Copy link
Copy Markdown

@SaraTahir28 SaraTahir28 commented Feb 4, 2026

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

  1. Added server‑side length check to ensure bloom content does not exceed 280 characters.
  2. Returns a 400 error with a clear message when the limit is exceeded.
  3. Aligns backend behaviour with existing frontend maxlength restriction.
  4. Manually verified by removing the maxlength attribute in DevTools and confirming the backend now rejects oversized blooms.

Questions

I initially planned to use the more robust request.get_json().get("content") pattern, but since this backend consistently accesses JSON via request.json["..."], I kept the implementation aligned with the current style. I want to know whats the best approach while working with legacy code, do we always keep our implementationa aligned with the current style ?

@SaraTahir28 SaraTahir28 added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Feb 4, 2026
@jenny-alexander jenny-alexander self-requested a review February 19, 2026 13:21
@jenny-alexander jenny-alexander added Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Feb 19, 2026
@jenny-alexander
Copy link
Copy Markdown

jenny-alexander commented Feb 20, 2026

Nice work! Changes look good.

@jenny-alexander
Copy link
Copy Markdown

jenny-alexander commented Feb 20, 2026

To answer your question about using request.get_json().get("content") or request.json["..."]:
I suggest that when working in legacy codebase, if there is something especially fragile/buggy, it's worthwhile fixing. If you don't, it tends to add to technical debt.

However, In this specific case, the following check is done at the top of the send_bloom function:

    type_check_error = verify_request_fields({"content": str})

    if type_check_error is not None:
        return type_check_error

It acts as a defensive check so that it's okay to use request.json["..."] further on in the function. When we get to request.json["..."], it's guaranteed that the key exists and it's the right type.

@jenny-alexander jenny-alexander added Reviewed Volunteer to add when completing a review with trainee action still to take. Complete Volunteer to add when work is complete and all review comments have been addressed. and removed Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. labels Feb 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jenny-alexander jenny-alexander Awaiting requested review from jenny-alexander

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

Complete Volunteer to add when work is complete and all review comments have been addressed. Reviewed Volunteer to add when completing a review with trainee action still to take.

Projects

Purple Forest Kanban
Status: Backlog

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@SaraTahir28 @jenny-alexander