-
Notifications
You must be signed in to change notification settings - Fork 0
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
Add validation and error summary to comment form #485
Conversation
d0f3103
to
0253598
Compare
|
- Pull axios out of the DonationCommentPopUp component and wraps it in a resource. - Add tests for post response handling Ticket: https://phabricator.wikimedia.org/T367387
0253598
to
2556075
Compare
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 like the new code, the new approach for pulling out server communication as a service improves quality and the error display works well. However, I user-tested the frontend code with the current PHP code and it does not work, because axios.post
by default sends a JSON body instead of the form-encoded fields that our current endpoint expects.
You can now either use a FormData
object and the correct Axios headers to send the data (see other services in src/api
) or we could take the opportunity to add an API endpoint to the fundraising app that accepts JSON. I've created a suggestion in wmde/fundraising-application#2992 which you can adapt as you see fit.
Lol, did I manage to only acceptance test the unhappy paths? |
2556075
to
d703958
Compare
- Add check for empty comment on submit - Add new error for when the comment is empty - Add error type var to display the correct error - Add tests Ticket: https://phabricator.wikimedia.org/T367387
d703958
to
927f917
Compare
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.
Works now
Ticket: https://phabricator.wikimedia.org/T367387