-
Notifications
You must be signed in to change notification settings - Fork 821
Forms: Add Form_Response class #44121
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
base: trunk
Are you sure you want to change the base?
Conversation
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! |
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.
Pull Request Overview
This PR introduces structured classes and tests to standardize handling of form responses before and after persistence.
- Adds a
Response_Field
class with accompanying unit tests. - Adds a
Form_Response
class with unit tests and a test utility for legacy feedback. - Cleans up a typo in
Contact_Form_Test
and updates the changelog.
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
projects/packages/forms/tests/php/contact-form/class-utility.php | Added a test utility for creating legacy feedback posts. |
projects/packages/forms/tests/php/contact-form/Response_Field_Test.php | Added unit tests for the Response_Field class. |
projects/packages/forms/tests/php/contact-form/Form_Response_Test.php | Added unit tests for the Form_Response class. |
projects/packages/forms/tests/php/contact-form/Contact_Form_Test.php | Removed a typo in test data setup (post_contnt ). |
projects/packages/forms/src/contact-form/class-response-field.php | Introduced the Response_Field class. |
projects/packages/forms/src/contact-form/class-form-response.php | Introduced the Form_Response class. |
projects/packages/forms/changelog/update-form-response-storage | Added changelog entry for the new form response classes. |
Comments suppressed due to low confidence (6)
projects/packages/forms/tests/php/contact-form/Form_Response_Test.php:56
- [nitpick] The test method name has an extra 'is'; rename to
test_form_response_matches_empty_data
for clarity and consistency.
public function test_form_response_is_matches_empty_data() {
projects/packages/forms/tests/php/contact-form/Contact_Form_Test.php:762
- The test removed the typo key but no longer provides a
post_content
value; add a correct'post_content' => 'testing'
to ensure the test inserts content.
'post_title' => 'testing',
projects/packages/forms/src/contact-form/class-response-field.php:11
- [nitpick] The class docblock summary is incomplete; provide a full description of what
Response_Field
handles.
* Handles the
projects/packages/forms/src/contact-form/class-response-field.php:108
- The docblock says
@return string
butget_meta()
returns an array; update it to@return array
.
* @return string
projects/packages/forms/src/contact-form/class-form-response.php:133
- Variable name typo:
parese_content
should beparsed_content
for readability.
$parese_content = static::parse_content( $this->post->post_content, $this->post->page_template );
projects/packages/forms/src/contact-form/class-form-response.php:241
- The docblock indicates
@return WP_Post|null
butget_ip()
returns a string or null; update the return type tostring|null
.
* @return WP_Post|null
projects/packages/forms/src/contact-form/class-response-field.php
Outdated
Show resolved
Hide resolved
Code Coverage SummaryCannot generate coverage summary while tests are failing. 🤐 Please fix the tests, or re-run the Code coverage job if it was something being flaky. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This PR add a couple of new classes that make dealing with form seponses easier.
Fixes FORMS-NEW
Currently we don't have a structure way that we deal with Form Responses. This PR in an attempt to improve this.
And also add more test to the code base.
We are also attempting to deal
Proposed changes:
Other information:
Jetpack product discussion
Does this pull request change what data or activity we track or use?
Testing instructions:
Do the tests pass?