-
Notifications
You must be signed in to change notification settings - Fork 834
Forms: Fix feedback source data #45231
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
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 fixes the feedback source data handling in forms by enhancing how form submission source information is tracked and stored. The changes improve the feedback system to better handle various contexts where forms can be submitted (posts, pages, widgets, block templates) and provide more accurate source information.
Key changes:
- Enhanced
Feedback_Source
class to support multiple source types (single, widget, block_template, block_template_part) - Added "Edit Form" action in the feedback inbox with proper source-based URLs
- Improved handling of deleted posts by prefixing titles with "[Deleted]"
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
class-feedback-source.php |
Core enhancement to support multiple source types and improved source data tracking |
class-feedback.php |
Updated to use enhanced source data and added edit form URL functionality |
class-contact-form.php |
Modified form processing to use new source tracking system |
class-contact-form-endpoint.php |
Added REST API support for edit form URL field |
dataviews/actions.js |
Added new "Edit Form" action for feedback inbox |
dataviews/index.js |
Integrated edit form action into the inbox interface |
Test files | Updated test expectations to match new source data behavior |
Changelog files | Documentation of the bugfix |
Comments suppressed due to low confidence (1)
projects/packages/forms/src/contact-form/class-feedback-source.php:1
- There are spelling errors in the docblock comment. 'obejct' should be 'object', 'relevent' should be 'relevant', and 'there' should be 'their'.
<?php
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
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! Jetpack plugin: The Jetpack plugin has different release cadences depending on the platform:
If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. |
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
Code Coverage SummaryCoverage changed in 5 files.
Full summary · PHP report · JS report If appropriate, add one of these labels to override the failing coverage check:
Covered by non-unit tests
|
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 think this is an improvement, I'll leave here some results so to double check the expected behavior:
When form is in widget:
- no Edit Form on the actions dropdown
- Source column holds the link to where it was submitted from (links work as expected, even getting to a 404 when page/post has been deleted)

Working with pages/posts:
- links work as expected
- Edit Form is available on actions dropdown
- when post is deleted, Edit Form lands on a default "post is trashed" error page

Working with templates:
- links work as expected
- Edit Form shows in actions dropdown
- could not delete template, link works fine, Edit Form takes me to template edit screen, even when it didn't make sense (template part was "lost" due to theme change, but I got to edit it all the same)

projects/packages/forms/src/contact-form/class-feedback-source.php
Outdated
Show resolved
Hide resolved
projects/packages/forms/src/contact-form/class-feedback-source.php
Outdated
Show resolved
Hide resolved
|
||
if ( current_user_can( 'edit_theme_options' ) ) { | ||
if ( $this->source_type === 'block_template' && \wp_is_block_theme() ) { | ||
return admin_url( 'site-editor.php?p=' . esc_attr( '/wp_template/' . addslashes( $this->id ) ) . '&canvas=edit' ); |
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 remember this template editor linking being particularly difficult because sometimes you need theme namespace added, or sometimes it's self created template. Worth testing well!
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.
So far it has been working well for me.
Should we test this across older versions of WP or Gutenberg?
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.
Jetpack supports current stable WP minus one, so no need to test anything older. I don't think there have been changes in recent versions to these URLs either.
projects/packages/forms/src/dashboard/inbox/dataviews/actions.js
Outdated
Show resolved
Hide resolved
Introduces an 'edit_form_url' property to the feedback source, allowing users to quickly access the edit page for forms, widgets, or block templates from the dashboard. Updates backend logic to serialize and expose this URL via the REST API, and adds a new dashboard action for editing forms. Includes related test updates to ensure source serialization and form source matching.
28e1749
to
dec4b58
Compare
Significance: patch | ||
Type: fixed | ||
|
||
Forms: Store the feedback source info with more context |
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.
(adding this as a comment on the changelog just to allow comment threading)
This is testing well! I did not test legacy theme or widget, but tested a range of scenarios with block themes.
-
404 page. I added a form to my 404 site editor template.
- Confirm bug: On trunk, when submitted, this shows the source as '/' (not accurate). There is no edit form button.
- Confirm fix: On this PR, it shows the source as '404 Not Found' with the url being the actual url I'd entered (/asdf). It takes me to that url, which is of course a 404 page. But this seems correct to me.
- Confirm edit button: On this PR, if I click the 'Edit Form' button, it correctly takes me to the 404 template in the site editor (nice).
-
Form in footer template pattern. I added a form to my footer template. Then created a "Page with No Form".
- Confirm bug: On trunk if I submit the footer form on "Page with no form", it shows the source as '/' (not accurate). There is no edit form button.
- Confirm fix: On this PR, if I submit the footer form, it shows the source a "Page with no form" with the correct URL.
- Confirm edit button: If I click the Edit Form button, it surprisingly takes me to the footer template where I created the form. Actually quite impressed on that.
- Confirm delete page behavior: If I delete "Page with no form", the source still says "Page with no form" and takes me to the accurate but now deleted url, so I get a 404. Seems like a very reasonable action.
-
Blog archive pagination: I set my home page to show blog posts, and then navigated to the third page of the blog archive (/page/3) and submitted the form that lives in the footer template pattern.
- Confirm bug: On trunk, when submitted, this shows the source as '/' (not accurate). There is no edit form button.
- Confirm fix: On this PR, it shows the source as the site title, and links to the correct page including pagination (/page/3).
- Confirm edit button: On this PR, if I click the 'Edit Form' button, it correctly takes me to the footer template pattern when the form lives.
-
Page deletion. I have some other old form pages that I deleted. In the new PR, I now see (deleted) in the link, and the link just goes back to Jetpack Forms admin. That seems reasonable, but also wondering if we should just remove the link?
-
Tests pass. I also confirmed tests pass.
Only question: For deleted pages rather than link circularly back to forms admin, I wonder if we should just remove the link? Not a big deal though.
Verdict on merging: This touches sensitive logic and there are lot of possible permutations, so it's possible we'll find an issue that needs fixing. BUT there's a lot of clearly broken behavior now, and this fixes all of it so far. Given that, I'd say LGTM.
Fixes FORMS-211
Proposed changes:
Other information:
Jetpack product discussion
See Fixes FORMS-211
Does this pull request change what data or activity we track or use?
No
Testing instructions:
Part 1. (Creat Forms)
Create a Form on a page or post.
Using a block theme
Create a Form in a site template (such as 404 page)
Create as in a Template Part such as the footer or header.
Using a Non Block theme.
Add a widget that has the form.
Part 2. Fill out the Forms.
Fill out the different forms that you created.
Then visit the Feedback Inbox.
Are you abel to navigate to the page that the Feedback was created? Are you able to navigate to the page where you can edit the form. ( See Edit Form ) in the dropdown)
Delete the page or post that has the form.
Notice that the source page title stays the same but it tells you that the page or post is deleted.