From bf8534bddea6b1da534a6fcdfd110c355087a6c7 Mon Sep 17 00:00:00 2001 From: Zin Kyaw Kyaw <108650842+ajzkk@users.noreply.github.com> Date: Sun, 3 Sep 2023 15:13:41 +0700 Subject: [PATCH 1/9] fix capabilities api calling on every pages --- includes/class-omise-capabilities.php | 32 +++++++++++++ .../class-omise-backend-installment-test.php | 26 ----------- .../class-omise-capabilities-test.php | 46 +++++++++++++++++++ .../gateway/class-omise-offsite-test.php | 1 - tests/unit/omise-woocommerce-test.php | 2 - 5 files changed, 78 insertions(+), 29 deletions(-) create mode 100644 tests/unit/includes/class-omise-capabilities-test.php diff --git a/includes/class-omise-capabilities.php b/includes/class-omise-capabilities.php index 66d65fc9..6306a6fd 100644 --- a/includes/class-omise-capabilities.php +++ b/includes/class-omise-capabilities.php @@ -35,6 +35,15 @@ class Omise_Capabilities { */ public static function retrieve($pKey = null, $sKey = null) { + $shouldCallApi = self::shouldCallApi( + is_admin(), + is_checkout(), + // If endpoint url is `order-received`, it mean thank you page. + is_wc_endpoint_url( 'order-received' ) + ); + + if( !$shouldCallApi ) return null; + $keys = self::getKeys($pKey, $sKey); // Do not call capabilities API if keys are not present @@ -68,6 +77,29 @@ public static function retrieve($pKey = null, $sKey = null) return self::$instance; } + /** + * @param boolean $isCheckout + * @param boolean $isAdmin + * @param boolean $isThankYouPage + * @return boolean + */ + public static function shouldCallApi($isCheckout, $isThankYouPage, $isAdmin,) { + $omisePages = [ 'omise' ]; + + $currentAdminPage = isset( $_GET[ 'page' ] ) ? $_GET[ 'page' ] : ''; + + // checkout page but not thank you page + // By default thank you page is also part of checkout pages + // and we do not need to call capabilities on thank you page. + $isPaymentPage = $isCheckout && !$isThankYouPage; + + // if from admin panel and is omise page. + $isOmiseSettingPage = $isAdmin && in_array( $currentAdminPage, $omisePages ); + + return $isPaymentPage || $isOmiseSettingPage; + } + + /** * @param string|null $pKey * @param string|null $sKey diff --git a/tests/unit/includes/backends/class-omise-backend-installment-test.php b/tests/unit/includes/backends/class-omise-backend-installment-test.php index 3809077a..977def24 100644 --- a/tests/unit/includes/backends/class-omise-backend-installment-test.php +++ b/tests/unit/includes/backends/class-omise-backend-installment-test.php @@ -247,29 +247,3 @@ public function correctly_calculating_monthly_payment_amount_as_merchant_absorbs $this->assertEquals(833.33, $result); } } - -/** - * Mock Omise_Capabilities class. - * NOTE: This might not be an ideal way to mock a class, - * feel free to enhance the test or the core code. - * - * @see includes/class-omise-capabilities - */ -class Omise_Capabilities -{ - /** - * @var self - */ - protected static $the_instance = null; - - public static function retrieve() - { - self::$the_instance = self::$the_instance ?: new self(); - return self::$the_instance; - } - - public function is_zero_interest() - { - return false; - } -} diff --git a/tests/unit/includes/class-omise-capabilities-test.php b/tests/unit/includes/class-omise-capabilities-test.php new file mode 100644 index 00000000..501b86bc --- /dev/null +++ b/tests/unit/includes/class-omise-capabilities-test.php @@ -0,0 +1,46 @@ +assertEquals($expected, $result); + } + + /** + * Data provider for toSubunitReturnCorrectFormat + */ + public function should_call_api_data_provider() + { + return [ + // checkout page and not thank you page + [true, false, false, '', true], + // checkout page and also thank you page + [true, true, false, '', false], + // omise setting page + [true, true, true, 'omise', true], + // other admin page + [true, true, true, 'other-page', false], + // non checkout page and also no-admin page + [false, false, false, 'other-page', false], + // non checkout page, non admin page + [false, false, false, '', false], + ]; + } +} diff --git a/tests/unit/includes/gateway/class-omise-offsite-test.php b/tests/unit/includes/gateway/class-omise-offsite-test.php index 6e163ed4..4712b196 100644 --- a/tests/unit/includes/gateway/class-omise-offsite-test.php +++ b/tests/unit/includes/gateway/class-omise-offsite-test.php @@ -1,7 +1,6 @@ Date: Sun, 3 Sep 2023 15:53:50 +0700 Subject: [PATCH 2/9] add test for retrieve --- includes/class-omise-capabilities.php | 4 +- .../class-omise-capabilities-test.php | 49 +++++++++++++++++++ 2 files changed, 51 insertions(+), 2 deletions(-) diff --git a/includes/class-omise-capabilities.php b/includes/class-omise-capabilities.php index 6306a6fd..7dbddbe7 100644 --- a/includes/class-omise-capabilities.php +++ b/includes/class-omise-capabilities.php @@ -36,10 +36,10 @@ class Omise_Capabilities { public static function retrieve($pKey = null, $sKey = null) { $shouldCallApi = self::shouldCallApi( - is_admin(), is_checkout(), // If endpoint url is `order-received`, it mean thank you page. - is_wc_endpoint_url( 'order-received' ) + is_wc_endpoint_url( 'order-received' ), + is_admin(), ); if( !$shouldCallApi ) return null; diff --git a/tests/unit/includes/class-omise-capabilities-test.php b/tests/unit/includes/class-omise-capabilities-test.php index 501b86bc..f645bddf 100644 --- a/tests/unit/includes/class-omise-capabilities-test.php +++ b/tests/unit/includes/class-omise-capabilities-test.php @@ -4,12 +4,61 @@ class Omise_Capabilities_Test extends TestCase { + private $omiseSettingMock; + + private $omiseCapabilitiesMock; + /** * setup add_action and do_action before the test run */ public function setUp(): void { require_once __DIR__ . '/../../../includes/class-omise-capabilities.php'; + $this->omiseSettingMock = Mockery::mock('alias:Omise_Setting'); + $this->omiseCapabilitiesMock = Mockery::mock('alias:OmiseCapabilities'); + } + + /** + * close mockery after test cases are done + */ + public function tearDown(): void + { + Mockery::close(); + } + + /** + * @runInSeparateProcess + * @covers Omise_Capabilities + */ + public function test_retrieve_should_return_null_when_it_should_not_call_api() + { + function is_admin() { return false; } + function is_checkout() { return false; } + function is_wc_endpoint_url($page) { return true; } + + $result = Omise_Capabilities::retrieve(); + $this->assertEquals(null, $result); + } + + /** + * @runInSeparateProcess + * @covers Omise_Capabilities + */ + public function test_retrieve_should_return_value_when_it_should_call_api() + { + function is_admin() { return false; } + function is_checkout() { return true; } + function is_wc_endpoint_url($page) { return false; } + + $this->omiseSettingMock->shouldReceive('instance')->andReturn($this->omiseSettingMock); + $this->omiseSettingMock->shouldReceive('public_key')->andReturn('pkey_xxx'); + $this->omiseSettingMock->shouldReceive('secret_key')->andReturn('skey_xxx'); + $this->omiseCapabilitiesMock->shouldReceive('retrieve')->once(); + + $result = Omise_Capabilities::retrieve(); + + $this->assertEquals('Omise_Capabilities', get_class($result)); + $this->assertTrue(true); } /** From 66b9a8627162308f48e57cc5f8555ef51f67500d Mon Sep 17 00:00:00 2001 From: Zin Kyaw Kyaw <108650842+ajzkk@users.noreply.github.com> Date: Sun, 3 Sep 2023 15:58:07 +0700 Subject: [PATCH 3/9] fix code smell --- includes/class-omise-capabilities.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/includes/class-omise-capabilities.php b/includes/class-omise-capabilities.php index 7dbddbe7..9a8caf3d 100644 --- a/includes/class-omise-capabilities.php +++ b/includes/class-omise-capabilities.php @@ -36,13 +36,15 @@ class Omise_Capabilities { public static function retrieve($pKey = null, $sKey = null) { $shouldCallApi = self::shouldCallApi( - is_checkout(), + is_checkout(), // If endpoint url is `order-received`, it mean thank you page. is_wc_endpoint_url( 'order-received' ), is_admin(), ); - if( !$shouldCallApi ) return null; + if ( !$shouldCallApi ) { + return null; + } $keys = self::getKeys($pKey, $sKey); From d22d6b3b1edb0b56a1205866c89ab59f0fe78abd Mon Sep 17 00:00:00 2001 From: Zin Kyaw Kyaw <108650842+ajzkk@users.noreply.github.com> Date: Mon, 4 Sep 2023 08:55:13 +0700 Subject: [PATCH 4/9] update function structure --- includes/class-omise-capabilities.php | 18 ++--- .../class-omise-capabilities-test.php | 72 ++++++++----------- 2 files changed, 35 insertions(+), 55 deletions(-) diff --git a/includes/class-omise-capabilities.php b/includes/class-omise-capabilities.php index 9a8caf3d..405f432a 100644 --- a/includes/class-omise-capabilities.php +++ b/includes/class-omise-capabilities.php @@ -35,14 +35,7 @@ class Omise_Capabilities { */ public static function retrieve($pKey = null, $sKey = null) { - $shouldCallApi = self::shouldCallApi( - is_checkout(), - // If endpoint url is `order-received`, it mean thank you page. - is_wc_endpoint_url( 'order-received' ), - is_admin(), - ); - - if ( !$shouldCallApi ) { + if ( !self::shouldCallApi() ) { return null; } @@ -85,18 +78,19 @@ public static function retrieve($pKey = null, $sKey = null) * @param boolean $isThankYouPage * @return boolean */ - public static function shouldCallApi($isCheckout, $isThankYouPage, $isAdmin,) { - $omisePages = [ 'omise' ]; + public static function shouldCallApi() { + $omiseSettingPages = [ 'omise' ]; $currentAdminPage = isset( $_GET[ 'page' ] ) ? $_GET[ 'page' ] : ''; // checkout page but not thank you page // By default thank you page is also part of checkout pages // and we do not need to call capabilities on thank you page. - $isPaymentPage = $isCheckout && !$isThankYouPage; + // If endpoint url is `order-received`, it mean thank you page. + $isPaymentPage = is_checkout() && !is_wc_endpoint_url( 'order-received' ); // if from admin panel and is omise page. - $isOmiseSettingPage = $isAdmin && in_array( $currentAdminPage, $omisePages ); + $isOmiseSettingPage = is_admin() && in_array( $currentAdminPage, $omiseSettingPages ); return $isPaymentPage || $isOmiseSettingPage; } diff --git a/tests/unit/includes/class-omise-capabilities-test.php b/tests/unit/includes/class-omise-capabilities-test.php index f645bddf..3c66a39a 100644 --- a/tests/unit/includes/class-omise-capabilities-test.php +++ b/tests/unit/includes/class-omise-capabilities-test.php @@ -27,68 +27,54 @@ public function tearDown(): void } /** + * @dataProvider retrieve_data_provider * @runInSeparateProcess * @covers Omise_Capabilities */ - public function test_retrieve_should_return_null_when_it_should_not_call_api() - { - function is_admin() { return false; } - function is_checkout() { return false; } - function is_wc_endpoint_url($page) { return true; } - - $result = Omise_Capabilities::retrieve(); - $this->assertEquals(null, $result); - } - - /** - * @runInSeparateProcess - * @covers Omise_Capabilities - */ - public function test_retrieve_should_return_value_when_it_should_call_api() - { - function is_admin() { return false; } - function is_checkout() { return true; } - function is_wc_endpoint_url($page) { return false; } + public function test_retrieve_should_return_value_when_it_should_call_api($isCheckout, $isThankYouPage, $isAdmin, $adminPageName, $expected) + { + // assigning to global variable, so that we can use in child functions + $GLOBALS['isCheckout'] = $isCheckout; + $GLOBALS['isThankYouPage'] = $isThankYouPage; + $GLOBALS['isAdmin'] = $isAdmin; - $this->omiseSettingMock->shouldReceive('instance')->andReturn($this->omiseSettingMock); - $this->omiseSettingMock->shouldReceive('public_key')->andReturn('pkey_xxx'); - $this->omiseSettingMock->shouldReceive('secret_key')->andReturn('skey_xxx'); - $this->omiseCapabilitiesMock->shouldReceive('retrieve')->once(); - - $result = Omise_Capabilities::retrieve(); + // mocking page name + $_GET['page'] = $adminPageName; - $this->assertEquals('Omise_Capabilities', get_class($result)); - $this->assertTrue(true); - } + function is_checkout() { return $GLOBALS['isCheckout']; } + function is_wc_endpoint_url($page) { return $GLOBALS['isThankYouPage']; } + function is_admin() { return $GLOBALS['isAdmin']; } - /** - * @dataProvider should_call_api_data_provider - * @covers Omise_Capabilities - */ - public function test_should_call_capability_api($isCheckout, $isThankYouPage, $isAdmin, $adminPageName, $expected) - { - $_GET['page'] = $adminPageName; - $result = Omise_Capabilities::shouldCallApi($isCheckout, $isThankYouPage, $isAdmin); - $this->assertEquals($expected, $result); + if ($expected) { + $this->omiseSettingMock->shouldReceive('instance')->andReturn($this->omiseSettingMock); + $this->omiseSettingMock->shouldReceive('public_key')->andReturn('pkey_xxx'); + $this->omiseSettingMock->shouldReceive('secret_key')->andReturn('skey_xxx'); + $this->omiseCapabilitiesMock->shouldReceive('retrieve')->once(); + $result = Omise_Capabilities::retrieve(); + $this->assertEquals('Omise_Capabilities', get_class($result)); + } else { + $result = Omise_Capabilities::retrieve(); + $this->assertEquals(null, $result); + } } /** * Data provider for toSubunitReturnCorrectFormat */ - public function should_call_api_data_provider() + public function retrieve_data_provider() { return [ // checkout page and not thank you page [true, false, false, '', true], - // checkout page and also thank you page + // // checkout page and also thank you page [true, true, false, '', false], - // omise setting page + // // omise setting page [true, true, true, 'omise', true], - // other admin page + // // other admin page [true, true, true, 'other-page', false], - // non checkout page and also no-admin page + // // non checkout page and also no-admin page [false, false, false, 'other-page', false], - // non checkout page, non admin page + // // non checkout page, non admin page [false, false, false, '', false], ]; } From 09cb0cd72e51f5851811946b781bf3042b52369d Mon Sep 17 00:00:00 2001 From: Zin Kyaw Kyaw <108650842+ajzkk@users.noreply.github.com> Date: Mon, 4 Sep 2023 09:29:11 +0700 Subject: [PATCH 5/9] update comment --- includes/class-omise-capabilities.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/includes/class-omise-capabilities.php b/includes/class-omise-capabilities.php index 405f432a..9c4e316d 100644 --- a/includes/class-omise-capabilities.php +++ b/includes/class-omise-capabilities.php @@ -83,13 +83,13 @@ public static function shouldCallApi() { $currentAdminPage = isset( $_GET[ 'page' ] ) ? $_GET[ 'page' ] : ''; - // checkout page but not thank you page + // If page is checkout page but not thank you page. // By default thank you page is also part of checkout pages // and we do not need to call capabilities on thank you page. // If endpoint url is `order-received`, it mean thank you page. $isPaymentPage = is_checkout() && !is_wc_endpoint_url( 'order-received' ); - // if from admin panel and is omise page. + // If page is omise setting page from admin panel. $isOmiseSettingPage = is_admin() && in_array( $currentAdminPage, $omiseSettingPages ); return $isPaymentPage || $isOmiseSettingPage; From f29bc9ea6bf4e7eb61f7de302fa512a25330ab3d Mon Sep 17 00:00:00 2001 From: Zin Kyaw Kyaw <108650842+ajzkk@users.noreply.github.com> Date: Mon, 4 Sep 2023 09:30:55 +0700 Subject: [PATCH 6/9] update function comment --- includes/class-omise-capabilities.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/includes/class-omise-capabilities.php b/includes/class-omise-capabilities.php index 9c4e316d..bc447cbf 100644 --- a/includes/class-omise-capabilities.php +++ b/includes/class-omise-capabilities.php @@ -73,9 +73,6 @@ public static function retrieve($pKey = null, $sKey = null) } /** - * @param boolean $isCheckout - * @param boolean $isAdmin - * @param boolean $isThankYouPage * @return boolean */ public static function shouldCallApi() { From f5d11e1f964f1f076a7405299ca946764b502686 Mon Sep 17 00:00:00 2001 From: Zin Kyaw Kyaw <108650842+ajzkk@users.noreply.github.com> Date: Mon, 4 Sep 2023 09:33:00 +0700 Subject: [PATCH 7/9] update comment format due to space and tab --- includes/class-omise-capabilities.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/includes/class-omise-capabilities.php b/includes/class-omise-capabilities.php index bc447cbf..b4d9890a 100644 --- a/includes/class-omise-capabilities.php +++ b/includes/class-omise-capabilities.php @@ -73,8 +73,8 @@ public static function retrieve($pKey = null, $sKey = null) } /** - * @return boolean - */ + * @return boolean + */ public static function shouldCallApi() { $omiseSettingPages = [ 'omise' ]; From e2cff110a37658a8f236b63454a6c398121a66ae Mon Sep 17 00:00:00 2001 From: Zin Kyaw Kyaw <108650842+ajzkk@users.noreply.github.com> Date: Mon, 4 Sep 2023 09:59:13 +0700 Subject: [PATCH 8/9] fix code format due to space and tab --- includes/class-omise-capabilities.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/includes/class-omise-capabilities.php b/includes/class-omise-capabilities.php index b4d9890a..ec7406cc 100644 --- a/includes/class-omise-capabilities.php +++ b/includes/class-omise-capabilities.php @@ -75,7 +75,7 @@ public static function retrieve($pKey = null, $sKey = null) /** * @return boolean */ - public static function shouldCallApi() { + public static function shouldCallApi() { $omiseSettingPages = [ 'omise' ]; $currentAdminPage = isset( $_GET[ 'page' ] ) ? $_GET[ 'page' ] : ''; @@ -89,8 +89,8 @@ public static function shouldCallApi() { // If page is omise setting page from admin panel. $isOmiseSettingPage = is_admin() && in_array( $currentAdminPage, $omiseSettingPages ); - return $isPaymentPage || $isOmiseSettingPage; - } + return $isPaymentPage || $isOmiseSettingPage; + } /** From 881e8dc610ab3d621d41044f8de237b2645a0012 Mon Sep 17 00:00:00 2001 From: Zin Kyaw Kyaw <108650842+ajzkk@users.noreply.github.com> Date: Mon, 4 Sep 2023 10:10:15 +0700 Subject: [PATCH 9/9] improve readability --- includes/class-omise-capabilities.php | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/includes/class-omise-capabilities.php b/includes/class-omise-capabilities.php index ec7406cc..c7de111e 100644 --- a/includes/class-omise-capabilities.php +++ b/includes/class-omise-capabilities.php @@ -77,8 +77,9 @@ public static function retrieve($pKey = null, $sKey = null) */ public static function shouldCallApi() { $omiseSettingPages = [ 'omise' ]; - $currentAdminPage = isset( $_GET[ 'page' ] ) ? $_GET[ 'page' ] : ''; + // If page is omise setting page from admin panel. + $isOmiseSettingPage = is_admin() && in_array( $currentAdminPage, $omiseSettingPages ); // If page is checkout page but not thank you page. // By default thank you page is also part of checkout pages @@ -86,9 +87,6 @@ public static function shouldCallApi() { // If endpoint url is `order-received`, it mean thank you page. $isPaymentPage = is_checkout() && !is_wc_endpoint_url( 'order-received' ); - // If page is omise setting page from admin panel. - $isOmiseSettingPage = is_admin() && in_array( $currentAdminPage, $omiseSettingPages ); - return $isPaymentPage || $isOmiseSettingPage; }