(fix) Test Setup Placement and Actual Test #130

thisisayush · 2021-10-11T11:27:18Z

This PR fixes Multi Crypto Testing and Layout issues. Extends #128

thisisayush · 2021-10-11T12:22:35Z

thisisayush · 2021-10-13T08:04:32Z

Updated Test Setup Script and Frontend to send single request to test all enabled ceyptos.

Current Issues:

While clicking "Test Setup" in frontend, the backend will still test for previously enabled cryptos only and will not take new changes in enabled cryptos into account. Possible Solutions: Pass the new cryptos as parameters, or disable test setup button on crypto status change unless save is clicked.

DarrenWestwood · 2021-10-13T09:14:04Z

modules/gateways/blockonomics/blockonomics.php

+        return $test_results;
+    }
+
+    public function test_one_currency($new_api, $currency)


test_one_currency can be renamed to test_one_crypto and refactored to match the latest woocommerce structure, which makes the code easier to read:
https://github.com/blockonomics/woocommerce-plugin/blob/master/php/Blockonomics.php#L343-L357
https://github.com/blockonomics/woocommerce-plugin/blob/master/php/Blockonomics.php#L193-L208

I though so too, but in the plugin code, crypto is being referred as currency in every function, changing it to crypto might cause confusions and human errors.

It will be best to use currency for the fiat currency linked to each order and crypto for the blockonomics cryptocurrencies.

Moving forward lets use this standard. I have opened a new issue for us to correct any existing functions that do not adhere to this #131.

DarrenWestwood · 2021-10-13T09:14:10Z

modules/gateways/blockonomics/testsetup.php

-        $response->errorStr = $error;
+    if (isset($error) && count($error) != 0) {
+        if (count(array_filter($error, function($item) { return $item != false; })) != 0) {
+            $response->error = true;


$response->error can be removed as the errorStr will be an empty array if there are no errors

thisisayush · 2021-10-14T09:01:28Z

Updated and removed the new API Key check and extra params from response.

DarrenWestwood · 2021-10-14T09:16:07Z

modules/gateways/blockonomics/testsetup.php

-if (isset($newApi)) {
-    $blockonomics = new Blockonomics();
+$response = array();
+$error = array();


This is not required as testSetup returns an empty array if there are no errors. The checks below which count the error length can also be removed. The following code should be enough as we check the error array later:

$blockonomics = new Blockonomics(); $error = $blockonomics->testSetup(); echo json_encode($error);

thisisayush · 2021-10-19T06:17:11Z

Refactored the testSetup functionality to individual methods corresponding to Woocommerce Plugin.

thisisayush · 2021-10-25T11:17:20Z

Added:

Checks to see if at least one crypto is enabled
Make BTC Enabled by default
Remove Underline and add cursor pointer in Advanced Settings Button/Link.

…bled

thisisayush · 2021-10-27T07:41:22Z

Added:

Check for Empty API Key
Prevent testsetup to run on protocol mismatch
Default Error Message in getNewAddress
Make Automatic Selection of currency incase of single currency dynamic in payment page

blockonomics · 2021-10-29T13:17:04Z

modules/gateways/blockonomics/blockonomics.php

-                }
+        $callback_secret = $this->getCallbackSecret();
+        $callback_url = $this->getCallbackUrl($callback_secret);
+        $base_url = preg_replace('/https?:\/\//', '', substr($callback_url, 0, -48));


-48 is a very hacky way, please use length of secret instead

blockonomics · 2021-10-29T13:22:36Z

modules/gateways/blockonomics/blockonomics.php

+        if (count($response->data) == 1) {
+            $error_str = $_BLOCKLANG['testSetup']['existingCallbackUrl'];
+        } else {
+            $error_str = $_BLOCKLANG['testSetup']['multipleXpubs'];
+        }


This is not matching woocommerce code . please check

Yes. Because Woocommerce only returns Please add a new store on blockonomics website while WHMCS Plugin returned You have an existing callback URL. Refer instructions on integrating multiple websites and Your have an existing callback URL or multiple xPubs. Refer instructions on integrating multiple websites based on whether user has single or multiple stores.

We can change it to the same as Woocommerce i.e. Single Message for both cases.

https://github.com/blockonomics/whmcs-bitcoin-plugin/pull/130/files/7acd64f6e0adc842b52a639e59fdc1ab11b59237#r739234900

Referencing the comment, I Will change it to a single message.

blockonomics · 2021-10-29T13:30:05Z

modules/gateways/blockonomics/lang/english.php

 $_BLOCKLANG['testSetup']['noXpub'] = 'You have not entered an xpub';
 $_BLOCKLANG['testSetup']['existingCallbackUrl'] = 'Your have an existing callback URL. Refer instructions on integrating multiple websites';


There is no need for messages 59-61. Note the changes in wocoomerce. There is only one message that we use everywhere
https://github.com/blockonomics/woocommerce-plugin/blob/master/php/Blockonomics.php#L141
https://github.com/blockonomics/woocommerce-plugin/blob/master/php/Blockonomics.php#L187

…cret

thisisayush · 2021-10-29T14:30:46Z

Updated:

Merged Error Messages as per WooCommerce Plugin
Make extraction of base_url from callback better by replacing constant -48 with actual string length calculation

blockonomics · 2021-10-30T05:53:56Z

modules/gateways/blockonomics/blockonomics.php

@@ -760,7 +760,7 @@ public function examine_server_callback_urls($response, $currency)

        $callback_secret = $this->getCallbackSecret();
        $callback_url = $this->getCallbackUrl($callback_secret);
-        $base_url = preg_replace('/https?:\/\//', '', substr($callback_url, 0, -48));
+        $base_url = preg_replace('/https?:\/\//', '', substr($callback_url, 0, -1 * (strlen($callback_secret) + 8)));


Why is the 8 added. Please add a code comment so that this code is more understandable

Updated the extraction logic to use strtok instead of manually working on it. it'll now split the string before ? and give the callback URL without needing to use substr

(fix) Test Setup Placement and Actual Test

a8eb6ca

thisisayush added 2 commits October 13, 2021 04:00

(add) Gitignore to make development easier

5303789

(update) Improve testSetup to test all enabled cryptos automatically

da5e21f

DarrenWestwood reviewed Oct 13, 2021

View reviewed changes

(update) Remove new apikey check

af8e2c3

DarrenWestwood reviewed Oct 14, 2021

View reviewed changes

(refactor) Split TestSetup to modular methods.

fbec911

thisisayush mentioned this pull request Oct 19, 2021

Fix issue 121 #132

Closed

(add) Check for Enabled Cryptos and Default BTC (fix) Minor bugs

6b23671

thisisayush added 5 commits October 26, 2021 05:45

(refactor) Remove redundant code

8406f46

(update) Add check for empty API Key

b3f1174

(fix) Prevent TestSetup to execute on protocol mismatch.

a850db9

(add) Default Error Message in getNewAddress

32030d3

(fix) Make auto currency selection dynamic incase one currency is ena…

7acd64f

…bled

blockonomics mentioned this pull request Oct 29, 2021

BCH Test Setup in Admin #122

Closed

blockonomics reviewed Oct 29, 2021

View reviewed changes

thisisayush added 2 commits October 29, 2021 10:22

(update) Make base_url extraction from callback based on length of se…

2b87ebc

…cret

(update) Merge Error Messages in compliance to Woocomerce Plugin

a67d53d

blockonomics reviewed Oct 30, 2021

View reviewed changes

thisisayush added 2 commits October 30, 2021 04:59

(remove) .gitignore

00b1ae3

(update) base_url extraction from callback url

6f712de

(update) BCH Error message in checkout

7af21fe

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

(fix) Test Setup Placement and Actual Test #130

(fix) Test Setup Placement and Actual Test #130

thisisayush commented Oct 11, 2021

thisisayush commented Oct 11, 2021

thisisayush commented Oct 13, 2021

DarrenWestwood Oct 13, 2021

thisisayush Oct 13, 2021

DarrenWestwood Oct 13, 2021

DarrenWestwood Oct 13, 2021

thisisayush Oct 13, 2021

thisisayush commented Oct 14, 2021

DarrenWestwood Oct 14, 2021

thisisayush Oct 26, 2021

thisisayush commented Oct 19, 2021

thisisayush commented Oct 25, 2021 •

edited

Loading

thisisayush commented Oct 27, 2021

blockonomics Oct 29, 2021

thisisayush Oct 29, 2021

blockonomics Oct 29, 2021

thisisayush Oct 29, 2021 •

edited

Loading

thisisayush Oct 29, 2021

blockonomics Oct 29, 2021

thisisayush Oct 29, 2021

thisisayush commented Oct 29, 2021

blockonomics Oct 30, 2021

thisisayush Nov 1, 2021

		$_BLOCKLANG['testSetup']['noXpub'] = 'You have not entered an xpub';
		$_BLOCKLANG['testSetup']['existingCallbackUrl'] = 'Your have an existing callback URL. Refer instructions on integrating multiple websites';

(fix) Test Setup Placement and Actual Test #130

Are you sure you want to change the base?

(fix) Test Setup Placement and Actual Test #130

Conversation

thisisayush commented Oct 11, 2021

thisisayush commented Oct 11, 2021

thisisayush commented Oct 13, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thisisayush commented Oct 14, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thisisayush commented Oct 19, 2021

thisisayush commented Oct 25, 2021 • edited Loading

thisisayush commented Oct 27, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thisisayush Oct 29, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thisisayush commented Oct 29, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thisisayush commented Oct 25, 2021 •

edited

Loading

thisisayush Oct 29, 2021 •

edited

Loading