jetpack icon indicating copy to clipboard operation
jetpack copied to clipboard

Forms: Refactor to Use JWT for Form Reconstruction

Open enejb opened this issue 8 months ago • 4 comments

This PR fixes the an issue where forms that are placed inside form different templates and don't get processed as expected.

Fixes FORMS-109

Proposed changes:

  • Refactors the contact form submission handler to always reconstruct the form instance from the JWT (jetpack_contact_form_jwt) provided in the POST data.
  • Removes legacy logic for reconstructing forms based on widget, block template, or block template part context, as well as the use of form hashes.
  • Simplifies the code by eliminating the need to parse the page context or retrieve shortcodes/attributes from post meta.
  • Improves error handling: returns a clear error if the JWT is missing or invalid.

Other information:

  • [ ] Have you written new tests for your changes, if applicable?
  • [ ] Have you checked the E2E test CI results, and verified that your changes do not break them?
  • [ ] Have you tested your changes on WordPress.com, if applicable (if so, you'll see a generated comment below with a script to run)?

Jetpack product discussion

Does this pull request change what data or activity we track or use?

No, this PR does not change what data or activity we track or use. It only changes the internal mechanism for reconstructing the form on submission.

Testing instructions:

  • Go to a page or post with a contact form (including forms in widgets, block templates, or template parts).
  • Submit the form with various field values.
  • Verify that the form submission is processed correctly and that the success message is displayed as expected.
  • Check that feedback is stored and visible in the admin as before.
  • Try submitting forms in different contexts (widget, block, template part) to ensure all are processed correctly.
  • Try submitting a form with a missing or tampered JWT and verify that a clear error is shown.

enejb avatar May 28 '25 21:05 enejb

Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.

  • To test on WoA, go to the Plugins menu on a WoA dev site. Click on the "Upload" button and follow the upgrade flow to be able to upload, install, and activate the Jetpack Beta plugin. Once the plugin is active, go to Jetpack > Jetpack Beta, select your plugin (Jetpack), and enable the fix/forms-submit-in-templates branch.
  • To test on Simple, run the following command on your sandbox:
bin/jetpack-downloader test jetpack fix/forms-submit-in-templates

Interested in more tips and information?

  • In your local development environment, use the jetpack rsync command to sync your changes to a WoA dev blog.
  • Read more about our development workflow here: PCYsg-eg0-p2
  • Figure out when your changes will be shipped to customers here: PCYsg-eg5-p2

github-actions[bot] avatar May 28 '25 21:05 github-actions[bot]

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

  • :white_check_mark: Include a description of your PR changes.
  • :white_check_mark: Add a "[Status]" label (In Progress, Needs Review, ...).
  • :white_check_mark: Add a "[Type]" label (Bug, Enhancement, Janitorial, Task).
  • :white_check_mark: Add testing instructions.
  • :white_check_mark: Specify whether this PR includes any changes to data or privacy.
  • :white_check_mark: Add changelog entries to affected projects

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 :robot:


Follow this PR Review Process:

  1. Ensure all required checks appearing at the bottom of this PR are passing.
  2. Make sure to test your changes on all platforms that it applies to. You're responsible for the quality of the code you ship.
  3. You can use GitHub's Reviewers functionality to request a review.
  4. When it's reviewed and merged, you will be pinged in Slack to deploy the changes to WordPress.com simple once the build is done.

If you have questions about anything, reach out in #jetpack-developers for guidance!


Jetpack plugin:

No scheduled milestone found for this plugin.

If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack.

github-actions[bot] avatar May 28 '25 21:05 github-actions[bot]

Code Coverage Summary

Cannot generate coverage summary while tests are failing. :zipper_mouth_face:

Please fix the tests, or re-run the Code coverage job if it was something being flaky.

Full summary · PHP report · JS report

jp-launch-control[bot] avatar May 28 '25 21:05 jp-launch-control[bot]

This is looking good, love seeing all that code gone.

I do see we're losing the form ID (which used to be the post ID), I'm always getting an incremental on each form at the post starting at 1. I can guess we were not using this ID in any critical way as responses seem to arrive fine and are listed as expected (parent post seems to be the one ruling the relation between responses and post where the form is published: 'post_parent' => $post ? (int) $post->ID : 0,).

That said, it would be good to have an identifier on the form, independent from the post ID or not, so to have a way of identifying the originating form when processing the submit. I think we'll get back at the idea of having a hash that can identify the form and also be it a meta on the post, but that process was not documented and, likely, not used.

I do wonder if removing the widget and template checks wouldn't incur in broken backwards compatibility, but I can't find the use case for those.

Need to look for old forms to verify things keep working, but it's good that we have a better way of packing all the contextual information.

CGastrell avatar Jun 16 '25 17:06 CGastrell

I am closing this PR in favour of https://github.com/Automattic/jetpack/pull/44360

enejb avatar Jul 18 '25 15:07 enejb