-
Notifications
You must be signed in to change notification settings - Fork 100
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Improve error messages for invalid submissions #2533
base: main
Are you sure you want to change the base?
Conversation
bd658c4
to
23b8286
Compare
Signed-off-by: Kostiantyn Miakshyn <[email protected]>
23b8286
to
a82cc79
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2533 +/- ##
============================================
- Coverage 43.40% 43.38% -0.03%
+ Complexity 882 881 -1
============================================
Files 77 77
Lines 3359 3356 -3
============================================
- Hits 1458 1456 -2
+ Misses 1901 1900 -1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice one :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I dont think this makes sense, what is the use case?
It is nothing shown to end users, that should already be done in the frontend and if it is not then there is a bug in the frontend that should be fixed.
So this is for developers that use the API?
In this case we should refactor the function at all (validateSubmission
) to be void
and throw instead on error.
Either InvalidArgumentException
if we keep this developers only I would prefer this.
Otherwise an \OCP\HintException
which would allow translated error messages for frontend - but here again we should never allow to submit something invalid so a frontend validation is wrong.
Right now it is really hard to understand for end users why form can't be submitted. This PR aims to improve validation messages.