[PM-30799] Validation added for Domain name#6856
Conversation
|
Great job! No new security vulnerabilities introduced in this pull request |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6856 +/- ##
=======================================
Coverage 56.00% 56.01%
=======================================
Files 1965 1966 +1
Lines 86886 86911 +25
Branches 7740 7745 +5
=======================================
+ Hits 48663 48680 +17
- Misses 36422 36431 +9
+ Partials 1801 1800 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| } | ||
|
|
||
| // Check length constraints | ||
| if (domainName.Length > 253) |
There was a problem hiding this comment.
Just thinking out loud: Generally speaking, I prefer meaningful variable and method names over comments for things like this. It's easier to maintain one source of truth instead of two (the code and the comment). That said, I'm not sure if that's still the accepted best practice with the new advances in AI.
There was a problem hiding this comment.
you make a good point jimmy. I will go easy on the comments - specially when the code is not complex

🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-30799
📔 Objective
The Domain name is a string and it is not validated.
This PR addes a DomainNameValidation attribute to the model, thereby forcing the model to be valid and if not, returns a 400 Bad Request.
The attribute pattern for validation is commonly used on the server side.
The client side already handles errors gracefully.
📸 Screenshots
Request to the server returns a 400
⏰ Reminders before review
🦮 Reviewer guidelines
:+1:) or similar for great changes:memo:) or ℹ️ (:information_source:) for notes or general info:question:) for questions:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion:art:) for suggestions / improvements:x:) or:warning:) for more significant problems or concerns needing attention:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt:pick:) for minor or nitpick changes