fix: validate length of displayname when setting it#41172
Conversation
DeepDiver1975
force-pushed
the
fix/displayname-length-input-validation
branch
from
c3d3618 to
e1be726
Compare
jvillafanez
left a comment
jvillafanez
left a comment
There was a problem hiding this comment.
Code is fine, but we should refactor the validation methods at some point.
| $resp = $this->validateString($displayName, 64); | ||
| if ($resp) { | ||
| return $resp; | ||
| } |
There was a problem hiding this comment.
We should refactor the validation at some point.
I see 2 options for the validateString (and validateEmail) methods:
- Return
trueorfalseaccording to the validation - Don't return anything and throw an exception if the validation fails. We might need to include a comment saying that an unhandled exception could be thrown from that method.
Right now, the code feels too unnatural, and I had to check what the validateString method does, which is awkward.
There was a problem hiding this comment.
Yes - we had this already on some other pr.
In an ideal world we would simply throw an exception which is caught by a middle ware ....
But to be honest - oc10 will go nowhere from here ... I just want to harden this one api call 🤷
|
delete-merged-branch
bot
deleted the
fix/displayname-length-input-validation
branch
4 participants
Description
Simple input validation when setting the display name
Related Issue
Motivation and Context
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist: