SIT-2708: Save sophi responses in CPT #298

oscarssanchez · 2022-05-31T19:35:23Z

Description of the Change

This approach creates a new CPT sophi-response that will save the sophi response as post meta. To check the expiration date, we store a post meta field and compare it to the current timestamp to check if we need to update the data from Sophi.

Closes #

Alternate Designs

Possible Drawbacks

Verification Process

Checklist:

I have read the CONTRIBUTING document.
My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have added tests to cover my change.
All new and existing tests passed.

Changelog Entry

Credits

Props @

felipeelia · 2022-05-31T19:45:13Z

sophi.php

@@ -60,6 +61,7 @@ function() {
 			// Bootstrap.
 			SophiWP\Core\setup();
 			SophiWP\Settings\setup();
+			SophiWP\PostType\post_type();


To keep consistency, we probably should have a setup() wrapping this function as well @oscarssanchez . What do you think?

Sounds good @felipeelia i've addressed it here 9b104eb

felipeelia · 2022-05-31T19:46:15Z

includes/classes/SiteAutomation/Request.php

 		if ( ! $response ) {
 			return [];
 		}

-		if ( ! $bypass_cache ) {
-			update_option( "sophi_site_automation_data_{$this->page}_{$this->widget}", $response );
+		if ( ! $bypass_cache && post_type_exists('sophi-response') ) {


Can we make post_type_exists('sophi-response') to post_type_exists( 'sophi-response')? :)

Sure addressed it here 8089267

felipeelia · 2022-05-31T19:47:53Z

includes/classes/SiteAutomation/Request.php

+			if ( $post_id ) {
+				update_post_meta( $post_id, "sophi_site_automation_data_{$this->page}_{$this->widget}", $response );
+				update_post_meta( $post_id, 'sophi_site_automation_last_updated', time() );
+			} else {
+				$post_id = wp_insert_post(
+					[
+						'post_type'  => 'sophi-response',
+						'post_title' => "sophi_site_automation_data_{$this->page}_{$this->widget}",
+					]
+				);
+				if ( $post_id ) {
+					update_post_meta( $post_id, "sophi_site_automation_data_{$this->page}_{$this->widget}", $response );
+					update_post_meta( $post_id, 'sophi_site_automation_last_updated', time() );
+				}
+			}


Realizing this is much more of a personal preference than anything, can't we check for ! $post_id and create the post, and only then call the update_post_meta lines (only once)?

I think that simplifies the logic and makes it more readable, I've addressed it here: bd0b247

felipeelia · 2022-05-31T19:53:31Z

includes/classes/SiteAutomation/Request.php

+		if ( ! $bypass_cache && post_type_exists('sophi-response' ) ) {
+			$query = new \WP_Query(
+				[
+					's'                      => "sophi_site_automation_data_{$page}_{$widget}",


Did you explore using post_name__in here instead? Just wondering if someone has ElasticPress and Protected Content enabled (just as an example) if this wouldn't bring the wrong result.

(Obviously, this would require an adjustment in the wp_insert_post call below)

I think it is a more reliable approach indeed, I have addressed it here 39094a0 and here c737ab6

jeffpaul · 2022-06-02T19:15:57Z

@cadic @Sidsector9 @iamdharmesh looking to see if one of you can also review this given your experience on Sophi over the last few months. Thanks in advance!

cadic · 2022-06-03T07:42:40Z

includes/blocks/site-automation-block/register.php

@@ -56,9 +54,27 @@ function render_block_callback( $attributes, $content, $block ) {
 	 */
 	$bypass_cache = apply_filters( 'sophi_bypass_curated_posts_cache', false, $page_name, $widget_name );

-	$curated_posts = get_transient( $curated_posts_transient_key );
+	$sophi_cached_response = new \WP_Query(


When $bypass_cache === true, the if statement at 76 will not check for $curated_posts at all. I think we are safe to skip 57:74 in this case.

In this case I think we should still do the query. The most important check is if $bypass_cache === true, and the next one should be if there's $curated_posts does it makes sense @cadic ?

@oscarssanchez thank you for explaining. I mean we could make the query optional.

When $bypass_cache === true, we will execute contents of if statement in 78:90 regardless the $curated_posts initial value, it will be overwrited with get_posts.

When $bypass_cache === false, this if statement will only be executed if $curate_posts is negative.

This means to me that we only need to take care about the initial value for $curated_posts when $bypass_cache === false

$curated_posts = false; if ( false === $bypass_cache ) { $sophi_cached_response = new \WP_Query( [ 'post_name__in' => [ "sophi-site-automation-data-{$page_name}-{$widget_name}" ], 'post_type' => 'sophi-response', 'posts_per_page' => 1, 'fields' => 'ids', 'no_found_rows' => true, 'update_post_term_cache' => false ] ); if ( $sophi_cached_response->found_posts ) { $last_update = get_post_meta( $sophi_cached_response->posts[0], 'sophi_site_automation_last_updated', true ); if ( $last_update + 5 * MINUTE_IN_SECONDS > time() ) { $curated_posts = get_post_meta( $sophi_cached_response->posts[0], 'sophi_site_automation_data' ); } } }

@oscarssanchez was ^ this addressed in your latest commits?

@cadic Thanks for clarifying, that makes sense to me! I have addressed this here: c40cf6e

includes/blocks/site-automation-block/register.php

includes/classes/SiteAutomation/Request.php

Co-authored-by: Max Lyuchin <maxim.lyuchin@gmail.com>

…ail/sophi-for-wordpress into feature/sophi-response-caching

sonarqubecloud · 2022-06-13T07:49:25Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

Save sophi responses in CPT

b9bfbb2

oscarssanchez requested a review from felipeelia May 31, 2022 19:35

oscarssanchez added 2 commits May 31, 2022 14:36

Change textdomain

9b653fc

Documentation

89140d2

jeffpaul added this to the 1.1.4 milestone May 31, 2022

jeffpaul changed the title ~~Save sophi responses in CPT~~ SIT-2708: Save sophi responses in CPT May 31, 2022

jeffpaul linked an issue May 31, 2022 that may be closed by this pull request

SIT-2708: Should curation result posts be saved in a site option? #153

Closed

jeffpaul assigned oscarssanchez May 31, 2022

felipeelia reviewed May 31, 2022

View reviewed changes

oscarssanchez added 5 commits June 1, 2022 12:10

Change function name

9b104eb

Add spaces

8089267

Simplify request response saving logic

bd0b247

Change response post retrieve approach

39094a0

Use array instead of string

c737ab6

oscarssanchez requested a review from felipeelia June 1, 2022 18:38

Merge branch 'develop' into feature/sophi-response-caching

df93c80

jeffpaul modified the milestones: 1.1.4, 1.2.0 Jun 1, 2022

felipeelia previously approved these changes Jun 2, 2022

View reviewed changes

jeffpaul requested review from cadic, iamdharmesh and Sidsector9 June 2, 2022 19:15

cadic suggested changes Jun 3, 2022

View reviewed changes

oscarssanchez dismissed felipeelia’s stale review via 0c73766 June 6, 2022 15:54

oscarssanchez and others added 5 commits June 6, 2022 10:54

Use $post_id

0c73766

Co-authored-by: Max Lyuchin <maxim.lyuchin@gmail.com>

Define

b2f0456

Define $curated_posts

92f97fd

Delete checking for post type exists

b944508

Merge branch 'feature/sophi-response-caching' of github.com:globeandm…

b5137e2

…ail/sophi-for-wordpress into feature/sophi-response-caching

oscarssanchez requested a review from cadic June 6, 2022 16:12

jeffpaul and others added 4 commits June 8, 2022 10:27

Merge branch 'develop' into feature/sophi-response-caching

5325a42

Check if we are bypassing cache before attempting query

c40cf6e

Merge branch 'feature/sophi-response-caching' of github.com:globeandm…

bc1ddb6

…ail/sophi-for-wordpress into feature/sophi-response-caching

Merge branch 'develop' into feature/sophi-response-caching

5485f6d

cadic approved these changes Jun 13, 2022

View reviewed changes

jeffpaul merged commit e551ace into develop Jun 13, 2022

jeffpaul deleted the feature/sophi-response-caching branch June 13, 2022 14:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SIT-2708: Save sophi responses in CPT #298

SIT-2708: Save sophi responses in CPT #298

oscarssanchez commented May 31, 2022

felipeelia May 31, 2022

oscarssanchez Jun 1, 2022

felipeelia May 31, 2022

oscarssanchez Jun 1, 2022

felipeelia May 31, 2022

oscarssanchez Jun 1, 2022

felipeelia May 31, 2022

felipeelia May 31, 2022

oscarssanchez Jun 1, 2022

jeffpaul commented Jun 2, 2022

cadic Jun 3, 2022

oscarssanchez Jun 6, 2022

cadic Jun 7, 2022

jeffpaul Jun 8, 2022

oscarssanchez Jun 10, 2022

sonarqubecloud bot commented Jun 13, 2022

SIT-2708: Save sophi responses in CPT #298

SIT-2708: Save sophi responses in CPT #298

Conversation

oscarssanchez commented May 31, 2022

Description of the Change

Alternate Designs

Possible Drawbacks

Verification Process

Checklist:

Changelog Entry

Credits

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeffpaul commented Jun 2, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Jun 13, 2022