Skip to content
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

Merged
merged 2 commits into from
Sep 30, 2024
Merged

Conversation

Abban
Copy link
Member

@Abban Abban commented Sep 3, 2024

  • Pull axios out of the DonationCommentPopUp component and wraps it in a resource.
  • Add tests for post response handling
  • 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

@Abban
Copy link
Member Author

Abban commented Sep 3, 2024

We need to wait until #464 is merged to get rid of the build error

- 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
@Abban Abban force-pushed the comment-validation branch from 0253598 to 2556075 Compare September 10, 2024 11:11
Copy link
Member

@gbirke gbirke left a 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.

src/api/CommentResource.ts Outdated Show resolved Hide resolved
@Abban
Copy link
Member Author

Abban commented Sep 13, 2024

Lol, did I manage to only acceptance test the unhappy paths?

@Abban Abban force-pushed the comment-validation branch from 2556075 to d703958 Compare September 13, 2024 05:13
@Abban Abban requested a review from gbirke September 13, 2024 05:13
- 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
Copy link
Member

@gbirke gbirke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works now

@gbirke gbirke merged commit 2f909d0 into main Sep 30, 2024
3 checks passed
@gbirke gbirke deleted the comment-validation branch September 30, 2024 09:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants