-
Notifications
You must be signed in to change notification settings - Fork 875
Forms: Add source filter to feedback REST API #48027
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
Merged
Merged
Changes from 4 commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
0341577
Forms: Add source filter to feedback REST API for filtering responses…
enejb dcc1422
Forms: Use add_post_meta for new feedback and lazily backfill source …
enejb 8437598
Forms: Fix test failures for WorDBless and Phan static analysis
enejb ee168ba
Update the tests
enejb 9bfb188
Forms: Improve test quality based on audit recommendations
enejb e3905dc
Forms: Address PR review feedback
enejb 82a726b
Forms: Address remaining review suggestions
enejb e26bb8d
Forms: Use Feedback::POST_TYPE constant in post_type guard checks
enejb 06c4d11
Forms: Remove deprecated setAccessible call for PHP 8.5 compat
enejb 4a354cb
Forms: Guard setAccessible with PHP version check
enejb 6a15b3e
Forms: Add TTL safety net to source IDs cache
enejb File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
4 changes: 4 additions & 0 deletions
4
projects/packages/forms/changelog/fix-forms-source-filter-responses
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| Significance: minor | ||
| Type: added | ||
|
|
||
| Add source filter parameter to feedback REST API for filtering responses by source post ID, with fallback to post_parent for legacy data. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -35,6 +35,122 @@ class Feedback { | |
| */ | ||
| public const STATUS_READ = 'closed'; | ||
|
|
||
| /** | ||
| * Meta key used to store the source post ID on feedback posts. | ||
| * | ||
| * @var string | ||
| */ | ||
| public const SOURCE_META_KEY = '_feedback_source_post_id'; | ||
|
|
||
| /** | ||
| * Cache key for the source post IDs list. | ||
| * | ||
| * @var string | ||
| */ | ||
| private const SOURCE_IDS_CACHE_KEY = 'jetpack_forms_source_post_ids'; | ||
|
|
||
| /** | ||
| * Cache group for forms data. | ||
| * | ||
| * @var string | ||
| */ | ||
| private const CACHE_GROUP = 'jetpack_forms'; | ||
|
|
||
| /** | ||
| * Returns all distinct source post IDs for feedback entries. | ||
| * | ||
| * Uses the _feedback_source_post_id meta for new feedback, with a fallback | ||
| * to post_parent for old feedback that doesn't have the meta yet (excluding | ||
| * jetpack_form parents). | ||
| * | ||
| * @return array Array of unique source post IDs. | ||
| */ | ||
| public static function get_all_source_post_ids() { | ||
| $source_ids = wp_cache_get( self::SOURCE_IDS_CACHE_KEY, self::CACHE_GROUP ); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Object cache with no TTL: if any invalidation path is missed, the cache becomes permanently stale. A safety-net TTL (e.g., HOUR_IN_SECONDS) would prevent that: wp_cache_set( self::SOURCE_IDS_CACHE_KEY, $source_ids, self::CACHE_GROUP, HOUR_IN_SECONDS ); |
||
|
|
||
| if ( false !== $source_ids ) { | ||
| return $source_ids; | ||
| } | ||
|
|
||
| global $wpdb; | ||
|
|
||
| $meta_key = self::SOURCE_META_KEY; | ||
| $statuses = array( 'draft', 'publish', 'spam', 'trash' ); | ||
| $placeholders = implode( ',', array_fill( 0, count( $statuses ), '%s' ) ); | ||
|
|
||
| // phpcs:disable WordPress.DB.DirectDatabaseQuery.DirectQuery,WordPress.DB.DirectDatabaseQuery.NoCaching,WordPress.DB.PreparedSQL.InterpolatedNotPrepared,WordPress.DB.PreparedSQLPlaceholders.ReplacementsWrongNumber | ||
| $source_ids = $wpdb->get_col( | ||
| $wpdb->prepare( | ||
| "SELECT DISTINCT source_id FROM ( | ||
| SELECT CAST(pm.meta_value AS UNSIGNED) AS source_id | ||
| FROM {$wpdb->postmeta} pm | ||
| INNER JOIN {$wpdb->posts} p ON p.ID = pm.post_id | ||
| WHERE pm.meta_key = %s | ||
| AND p.post_type = 'feedback' | ||
| AND p.post_status IN ({$placeholders}) | ||
| AND pm.meta_value != '0' AND pm.meta_value != '' | ||
| UNION | ||
| SELECT p.post_parent AS source_id | ||
| FROM {$wpdb->posts} p | ||
| LEFT JOIN {$wpdb->postmeta} pm ON pm.post_id = p.ID AND pm.meta_key = %s | ||
| LEFT JOIN {$wpdb->posts} parent_post ON parent_post.ID = p.post_parent | ||
| WHERE p.post_type = 'feedback' | ||
| AND p.post_status IN ({$placeholders}) | ||
| AND p.post_parent > 0 | ||
| AND pm.meta_id IS NULL | ||
| AND (parent_post.post_type IS NULL OR parent_post.post_type != %s) | ||
| ) AS combined_sources", | ||
| array_merge( | ||
| array( $meta_key ), | ||
| $statuses, | ||
| array( $meta_key ), | ||
| $statuses, | ||
| array( Contact_Form::POST_TYPE ) | ||
| ) | ||
| ) | ||
| ); | ||
| // phpcs:enable | ||
|
|
||
| $source_ids = array_map( 'intval', $source_ids ); | ||
| wp_cache_set( self::SOURCE_IDS_CACHE_KEY, $source_ids, self::CACHE_GROUP ); | ||
|
|
||
| return $source_ids; | ||
| } | ||
|
|
||
| /** | ||
| * Invalidates the source post IDs cache when a feedback post is deleted. | ||
| * | ||
| * @param int $post_id The deleted post ID. | ||
| * @param \WP_Post $post The deleted post object. | ||
| */ | ||
| public static function invalidate_source_ids_cache_on_delete( $post_id, $post ) { | ||
| if ( $post->post_type === self::POST_TYPE ) { | ||
| wp_cache_delete( self::SOURCE_IDS_CACHE_KEY, self::CACHE_GROUP ); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Backfills the source post ID meta from the feedback object's resolved source. | ||
| * | ||
| * For old feedback parented to a jetpack_form that doesn't have | ||
| * _feedback_source_post_id set yet, this writes the meta so future | ||
| * queries can filter by source without the post_parent fallback. | ||
| * | ||
| * @param int $post_id The feedback post ID. | ||
| * @param Feedback $feedback The feedback object (already has source resolved from parsed content). | ||
| */ | ||
| public static function maybe_backfill_source_meta( $post_id, $feedback ) { | ||
| $existing = get_post_meta( $post_id, self::SOURCE_META_KEY, true ); | ||
| if ( $existing ) { | ||
| return; | ||
| } | ||
|
|
||
| $source_id = $feedback->get_entry_id(); | ||
| if ( is_numeric( $source_id ) && (int) $source_id > 0 ) { | ||
| add_post_meta( $post_id, self::SOURCE_META_KEY, (int) $source_id, true ); | ||
|
enejb marked this conversation as resolved.
Outdated
|
||
| } | ||
| } | ||
|
|
||
| /** | ||
| * The form field values. | ||
| * | ||
|
|
@@ -1345,6 +1461,13 @@ public function save() { | |
| ) | ||
| ); | ||
|
|
||
| // Store source post ID as meta for queryable source filtering. | ||
| $source_id = $this->source->get_id(); | ||
| if ( is_numeric( $post_id ) && (int) $post_id > 0 && is_numeric( $source_id ) && (int) $source_id > 0 ) { | ||
| add_post_meta( $post_id, self::SOURCE_META_KEY, (int) $source_id, true ); | ||
| wp_cache_delete( self::SOURCE_IDS_CACHE_KEY, self::CACHE_GROUP ); | ||
| } | ||
|
|
||
| // If this feedback does not have a jetpack_form parent, | ||
| // it's a classic form — mark the state accordingly. | ||
| if ( empty( $this->form_id ) ) { | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.