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

Lint fixes #3673

Merged
merged 6 commits into from
Jan 9, 2025
Merged

Lint fixes #3673

merged 6 commits into from
Jan 9, 2025

Conversation

nbloomf
Copy link
Contributor

@nbloomf nbloomf commented Dec 19, 2024

Changes proposed in this Pull Request:

Fixing some lint issues raised by the WordPress code standards. Ideally these would be enforced in a pre-commit hook, but existing issues need to be fixed first.

Some notes:

isset( $_POST['foo'] ) ? wc_clean( $_POST['foo'] ) : null

I've replaced this with

( isset( $_POST['foo'] ) && is_string( $_POST['foo'] ) ) ? sanitize_text_field( $_POST['foo'] ) : null

Testing instructions

I think reading the code and making sure unit tests pass is good enough; this should change any behavior.


  • Covered with tests (or have a good reason not to test in description ☝️)
  • Added changelog entry in both changelog.txt and readme.txt (or does not apply)
  • Tested on mobile (or does not apply)

Post merge

@nbloomf nbloomf self-assigned this Dec 19, 2024
@annemirasol annemirasol requested review from a team and annemirasol and removed request for a team January 2, 2025 18:35
Copy link
Contributor

@annemirasol annemirasol left a comment

Choose a reason for hiding this comment

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

Thanks for taking care of these.

I looked at the changes and did some smoke testing. LGTM 🚢

@@ -1101,7 +1101,7 @@ public function confirm_change_payment_from_setup_intent_ajax() {
throw new WC_Stripe_Exception( 'subscription_not_found', __( "We're not able to process this subscription change payment request payment. Please try again later.", 'woocommerce-gateway-stripe' ) );
}

$setup_intent_id = isset( $_POST['intent_id'] ) ? wc_clean( wp_unslash( $_POST['intent_id'] ) ) : null;
$setup_intent_id = ( isset( $_POST['intent_id'] ) && is_string( $_POST['intent_id'] ) ) ? sanitize_text_field( wp_unslash( $_POST['intent_id'] ) ) : null;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is fine. We do expect a valid $_POST['intent_id'] to be a string.

@nbloomf nbloomf force-pushed the fix/lint-errors-2 branch from e26baa2 to 362476e Compare January 8, 2025 19:31
require_once __DIR__ . '/includes/admin/class-wc-stripe-upe-compatibility-controller.php';
require_once __DIR__ . '/includes/migrations/class-allowed-payment-request-button-types-update.php';
require_once __DIR__ . '/includes/class-wc-stripe-account.php';

new Allowed_Payment_Request_Button_Types_Update();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There were some file path changes that required a rebase here; I used diff to make sure the names match.

@nbloomf nbloomf force-pushed the fix/lint-errors-2 branch from 166df50 to fc98af3 Compare January 8, 2025 21:07
@nbloomf nbloomf merged commit 7bd8824 into develop Jan 9, 2025
34 of 37 checks passed
@nbloomf nbloomf deleted the fix/lint-errors-2 branch January 9, 2025 04:02
hsingyuc pushed a commit that referenced this pull request Jan 27, 2025
* Lint fixes

---------

Co-authored-by: nbloomf <nathan.bloomfield@automattic.com>
Co-authored-by: Anne Mirasol <anne.mirasol@automattic.com>
hsingyuc added a commit that referenced this pull request Jan 29, 2025
* Lint fixes (#3673)

* Lint fixes

---------

Co-authored-by: nbloomf <nathan.bloomfield@automattic.com>
Co-authored-by: Anne Mirasol <anne.mirasol@automattic.com>

* Fix excessive emails when enabling the gateway (#3695)

* Fix duplicate emails when enabling the gateway

During payment gateway initialization, WC core adds hooks that will get
triggered when the main gateway is enabled. Using a centralized option_key for all
'child gateways' makes WC core issue separate email notifications for all of them. To sidestep this problem,
with this change, we use separate option_id for each child gateway but still use the
parent gateway settings when retrieving them.

Changes made:
- Update get_option_key() to return settings specific to the Stripe ID.
- Introduce get_option() method to fetch options from the main Stripe gateway.

---------

Co-authored-by: Mayisha <33387139+Mayisha@users.noreply.github.com>

* New headers for changelog and readme files

* Update docker-compose cmd to docker compose. (#3709)

Co-authored-by: Diego Curbelo <diego@curbelo.com>

* Pass total tax amount to metadata

* Add changelog entry

* Update unit test

---------

Co-authored-by: Nathan Bloomfield <nathan.bloomfield@a8c.com>
Co-authored-by: nbloomf <nathan.bloomfield@automattic.com>
Co-authored-by: Anne Mirasol <anne.mirasol@automattic.com>
Co-authored-by: Malith Senaweera <6216000+malithsen@users.noreply.github.com>
Co-authored-by: Mayisha <33387139+Mayisha@users.noreply.github.com>
Co-authored-by: Wesley Rosa <wesleyjrosa@gmail.com>
Co-authored-by: Rafael Zaleski <rafaelzaleski@users.noreply.github.com>
Co-authored-by: Diego Curbelo <diego@curbelo.com>
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