Repository navigation
London | 26-SDC-Jul | Raihan Sharif | Sprint 1 | Bug Report: Extra long blooms - #307
RaihanSharif wants to merge 2 commits into
Conversation
ParthibanRajasekaran
left a comment
There was a problem hiding this comment.
Thanks, Raihan! It looks like a solid PR. Good use of the named constant to enforce the 280-character limit and block over-length blooms before storage.
Just a couple of small comments (one on scope, one on boundary testing). Let me know once you've had a chance to check them out
| @@ -1 +1,2 @@ | |||
| .DS_Store | |||
| .vscode No newline at end of file | |||
There was a problem hiding this comment.
I noticed that this .vscode change is unrelated to the extra-long-bloom fix. Keeping a pull request focused on one task makes it easier to understand, test and review. Could you remove this change from the PR, or let me know if it is required for the fix?
| if type_check_error is not None: | ||
| return type_check_error | ||
|
|
||
| if len(request.json["content"]) > MAXIMUM_BLOOM_LENGTH: |
There was a problem hiding this comment.
The boundary condition looks correct: a bloom with exactly 280 characters is allowed, while anything longer is rejected. How did you verify the behaviour at 280 and 281 characters? Checking both sides of a limit is a useful way to guard against off-by-one errors
Learners, PR Template
Self checklist
Task code
CYF-1168
Changelist
Added backend validation to ensure that bloom text cannot be longer than some constant value (MAXIMUM_BLOOM_LENGTH), if it is, send a 400 response.