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

Support dynamic price_suffix in in-app marketplace #31026

Merged
merged 2 commits into from
Nov 16, 2021

Conversation

andfinally
Copy link
Contributor

@andfinally andfinally commented Oct 25, 2021

All Submissions:

Changes proposed in this Pull Request:

Adds support for a price_suffix property in each product in the WooCommerce.com search and featured endpoints, which provide the data for the product cards in the in-app marketplace. This will allow us to list products whose fees are per month.

How to test the changes in this Pull Request:

Note for GlobalStep testers

The required data changes for this PR haven't been implemented on WooCommerce.com yet, and the test below requires using the test data in a GitHub gist. For now, it's fine if we simply continue to see "per year" on the prices of products in the Marketplace, for example $29.00 per year, where the product is not free.

Test using dummy data

  1. Check out this branch.
  2. Edit wp-content/plugins/woocommerce/plugins/woocommerce/includes/admin/class-wc-admin-addons.php:143 to point to this gist with test data: https://gist.githubusercontent.com/andfinally/84015c40afc8dc628f2496c9c4faf0f8/raw/search.json.
$raw_extensions = wp_safe_remote_get(
	'https://gist.githubusercontent.com/andfinally/84015c40afc8dc628f2496c9c4faf0f8/raw/search.json',
	array( 'headers' => $headers )
);
  1. Go to wp-admin/admin.php?page=wc-addons&section=payment-gateways.
  2. Check that Authorize.Net has a price "per month".

image

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully run tests with your changes locally?

Changelog entry

Add - Support for dynamic price period in in-app marketplace product cards.

FOR PR REVIEWER ONLY:

  • I have reviewed that everything is sanitized/escaped appropriately for any SQL or XSS injection possibilities. I made sure Linting is not ignored or disabled.

… allowing prices per month. This was added in #30840, but was accidentally omitted when we fixed conflicts.
@andfinally andfinally self-assigned this Oct 25, 2021
@andfinally andfinally marked this pull request as draft October 25, 2021 13:29
@andfinally andfinally changed the title Restored support for price suffix in in-app marketplace product card,… Support dynamic price_suffix in in-app marketplace Oct 25, 2021
@andfinally andfinally marked this pull request as ready for review November 11, 2021 10:39
@andfinally andfinally requested review from a team and barryhughes and removed request for a team November 11, 2021 10:43
@andfinally andfinally added the needs: author feedback The issue/PR needs a response from any of the parties involved in the issue. label Nov 11, 2021
Copy link
Member

@barryhughes barryhughes left a comment

Choose a reason for hiding this comment

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

Just need a minor tweak to satisfy linting rules 👍

Co-authored-by: Barry Hughes <3594411+barryhughes@users.noreply.github.com>
Copy link
Member

@barryhughes barryhughes left a comment

Choose a reason for hiding this comment

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

LGTM!

(Triggered a re-run of a couple of checks, however the failures were likely unrelated to this change.)

@barryhughes barryhughes merged commit 0df6cc1 into trunk Nov 16, 2021
@barryhughes barryhughes deleted the update/in-app-marketplace-price-suffix branch November 16, 2021 01:34
@github-actions github-actions bot added this to the 6.0.0 milestone Nov 16, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Nov 16, 2021

Hi @barryhughes, thanks for merging this pull request. Please take a look at these follow-up tasks you may need to perform:

  • Add the status: needs changelog label
  • Add the status: needs testing instructions label

@barryhughes barryhughes added release: add changelog Mark all PRs that have not had their changelog entries added. [auto] release: add testing instructions PRs that have not had testing instructions added to the wiki. [auto] and removed needs: author feedback The issue/PR needs a response from any of the parties involved in the issue. labels Nov 16, 2021
@andfinally
Copy link
Contributor Author

Great! Thanks for the review Barry.

@ObliviousHarmony ObliviousHarmony added changelog added and removed release: add changelog Mark all PRs that have not had their changelog entries added. [auto] labels Nov 21, 2021
@rodelgc rodelgc added testing instructions added and removed release: add testing instructions PRs that have not had testing instructions added to the wiki. [auto] labels Dec 6, 2021
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.

4 participants