omise / omise-woocommerce

Omise WooCommerce Plugin
https://docs.opn.ooo/woocommerce-plugin
MIT License
47 stars 27 forks source link

[APM2-579] Add checkout filter currency for mobile banking #249

Closed new4762 closed 2 years ago

new4762 commented 2 years ago

1. Objective

Fix displaying payment options for mobile banking to display only support checkout currency

Task: https://omise.atlassian.net/browse/APM2-579

2. Description of change

add backend filter by checkout currency from woocommerce api

3. Quality assurance

Test this update locally and ensure pyament methods shows up correctly

πŸ”§ Environments:

Specify the details of your test environments, including, for each, the platform version (on which the plugin was run), the Omise plugin version, and the versions of your system software such as PHP or Ruby.

✏️ Details:

Enable Mobile banking through Omise setting and try to checkout selecting mobile banking as payment method, a form should show only support options. (try switching currency in woocommerce setting in Currency options

4. Impact of the change

Before image

After image

image

5. Priority of change

Normal

6. Additional Notes

Any further information that you would like to add.

kan98 commented 2 years ago

not sure if it's worth adding a bit of styling on the mobile bank list? the logo placement seems a little off to me

new4762 commented 2 years ago

not sure if it's worth adding a bit of styling on the mobile bank list? the logo placement seems a little off to me

What about this one πŸ€”

image
kan98 commented 2 years ago

not sure if it's worth adding a bit of styling on the mobile bank list? the logo placement seems a little off to me

What about this one πŸ€” image

@new4762 looks good!

FhanOmise commented 2 years ago

trying to follow this one any suggestions πŸ™

  /**
   * Retrieves details of installment payment backends from capabilities.
   *
   * @return string
   */
  public function getInstallmentBackends( $currency = '', $amount = null ) {

      $params   = array();
      $params[] = $this->capabilities->backendFilter['type']('installment');

      if ( $currency ) {
          $params[] = $this->capabilities->backendFilter['currency']( $currency );
      }
      if ( ! is_null( $amount ) ) {
          $params[] = $this->capabilities->backendFilter['chargeAmount']( $amount );
      }

      return $this->capabilities->getBackends( $params );
  }

@new4762 @kan98 Maybe because they want $params and $params[] to have aligned =. But getBackends() have only 1 assignment. I think we can remove the exceeded space, wdyt? And maybe consider add space after $currency = '' for below too.

https://github.com/omise/omise-woocommerce/blob/ea946fed4f6ee1e4faa56100a407d46057ba7f92/includes/class-omise-capabilities.php#L55

kan98 commented 2 years ago

trying to follow this one any suggestions πŸ™

    /**
     * Retrieves details of installment payment backends from capabilities.
     *
     * @return string
     */
    public function getInstallmentBackends( $currency = '', $amount = null ) {

        $params   = array();
        $params[] = $this->capabilities->backendFilter['type']('installment');

        if ( $currency ) {
            $params[] = $this->capabilities->backendFilter['currency']( $currency );
        }
        if ( ! is_null( $amount ) ) {
            $params[] = $this->capabilities->backendFilter['chargeAmount']( $amount );
        }

        return $this->capabilities->getBackends( $params );
    }

@new4762 @kan98 Maybe because they want $params and $params[] to have aligned =. But getBackends() have only 1 assignment. I think we can remove the exceeded space, wdyt? And maybe consider add space after $currency = '' for below too.

https://github.com/omise/omise-woocommerce/blob/ea946fed4f6ee1e4faa56100a407d46057ba7f92/includes/class-omise-capabilities.php#L55

yea it only had one assignment which was what confused me aha. I think we should go with your suggestions

new4762 commented 2 years ago

trying to follow this one any suggestions πŸ™

  /**
   * Retrieves details of installment payment backends from capabilities.
   *
   * @return string
   */
  public function getInstallmentBackends( $currency = '', $amount = null ) {

      $params   = array();
      $params[] = $this->capabilities->backendFilter['type']('installment');

      if ( $currency ) {
          $params[] = $this->capabilities->backendFilter['currency']( $currency );
      }
      if ( ! is_null( $amount ) ) {
          $params[] = $this->capabilities->backendFilter['chargeAmount']( $amount );
      }

      return $this->capabilities->getBackends( $params );
  }

@new4762 @kan98 Maybe because they want $params and $params[] to have aligned =. But getBackends() have only 1 assignment. I think we can remove the exceeded space, wdyt? And maybe consider add space after $currency = '' for below too. https://github.com/omise/omise-woocommerce/blob/ea946fed4f6ee1e4faa56100a407d46057ba7f92/includes/class-omise-capabilities.php#L55

yea it only had one assignment which was what confused me aha. I think we should go with your suggestions

@FhanOmise @kan98 Fixed πŸ™ -> https://github.com/omise/omise-woocommerce/pull/249/commits/6d3c42c6e0b7d344c1251eeafa294dbf28500196

tanawin-opn commented 2 years ago

test with multiple currency worked correctly krub.

but please check the plugin version number, because it's back and forth between 4.16 and 4.17. I think it might be a problem when release krub.

new4762 commented 2 years ago

test with multiple currency worked correctly krub.

but please check the plugin version number, because it's back and forth between 4.16 and 4.17. I think it might be a problem when release krub.

Thanks you very much kub πŸ™‡