Skip to content
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

add caveat to README about the usage of get_posts instead of WP_Query #242

Merged
merged 8 commits into from
Apr 25, 2022

Conversation

Sidsector9
Copy link
Contributor

Description of the Change

Added a caveat surrounding the usage of WP_Query on a VIP environment.

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

doc: update README about the usage of WP_Query

Credits

Props @Sidsector9

@Sidsector9 Sidsector9 requested a review from jeffpaul April 21, 2022 18:26
@Sidsector9 Sidsector9 self-assigned this Apr 21, 2022
@jeffpaul jeffpaul added this to the 1.0.14 milestone Apr 21, 2022
@jeffpaul
Copy link
Contributor

@Sidsector9 do you think it's worth including a code snippet here for how that might work?

@jeffpaul jeffpaul requested a review from oscarssanchez April 21, 2022 19:19
jeffpaul
jeffpaul previously approved these changes Apr 21, 2022
Copy link
Contributor

@jeffpaul jeffpaul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, but hoping to get a couple others to review/confirm before merging

README.md Outdated
@@ -113,6 +113,10 @@ Note that you need to add `data-sophi-feature=<widget_name>` to the wrapper div
</div>
```

#### Caveats

While the above query integration works just fine, it has been observed on [WordPress VIP](https://wpvip.com/) infrastructure that `WP_Query` may return latest posts instead of the posts curated by Sophi. A workaround for this is to use [`get_posts()`](https://developer.wordpress.org/reference/functions/get_posts/) instead.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may be worth also calling out here that should an integration need to switch to using get_posts() that we'd recommend a strong comment in that codebase to call out WHY so that some future engineer doesn't come along and swap it out for WP_Query and totally break things.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@jeffpaul jeffpaul merged commit c088064 into develop Apr 25, 2022
@jeffpaul jeffpaul deleted the caveat-note branch April 25, 2022 18:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants