-
Notifications
You must be signed in to change notification settings - Fork 12
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
Update the BCH URL structure #126
Conversation
Looking forward to looking through it |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, functions as expected. I've added some comments for minor improvements.
</tr> | ||
</table> | ||
</div> | ||
</div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
very small detail, perhaps add a new line at the end of the file?
<script type="text/javascript" src="{$WEB_ROOT}/modules/gateways/blockonomics/assets/js/qrcode-generator/qrcode.js"></script> | ||
<script type="text/javascript" src="{$WEB_ROOT}/modules/gateways/blockonomics/assets/js/qrcode-generator/qrcode_UTF8.js"></script> | ||
<script type="text/javascript" src="{$WEB_ROOT}/modules/gateways/blockonomics/assets/js/angular-qrcode/angular-qrcode.js"></script> | ||
<script type="text/javascript" src="{$WEB_ROOT}/modules/gateways/blockonomics/assets/js/reconnecting-websocket.min.js"></script> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
very small detail, perhaps add a new line at the end of the file?
|
||
}); | ||
})(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
very small detail, perhaps add a new line at the end of the file?
$scope.order = data; | ||
if(data.blockonomics_currency === 'btc'){ | ||
function proccess_order_data() { | ||
if($scope.crypto.code === 'btc'){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be simplified as const subdomain = ($scope.crypto.code === 'btc') ? 'www' : $scope.crypto.code
//Redirect to order received page if message from socket | ||
window.location = $scope.finish_order_url(); | ||
//Redirect to order confirmation page if message from socket | ||
window.location = Url.get_wc_endpoint({'finish_order' : $scope.order.id_order}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Url.get_wc_endpoint()
seems to be connected to WooCommerce? Perhaps it's clearer to keep the old function call $scope.finish_order_url();
?
//Wait for 2 seconds for order status to update on server | ||
}, 2000, 1); | ||
} | ||
} | ||
else if ($scope.order.status >= 0){ | ||
//Goto order confirmation as payment is already in process or done | ||
window.location = Url.get_wc_endpoint({'finish_order' : $scope.order.id_order}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as the comment above
}else if($scope.currency.code === 'bch'){ | ||
}else if($scope.crypto.code === 'btc'){ | ||
if (data.error && data.error.toLowerCase().indexOf("gap limit") !== -1) | ||
$scope.btc_gaplimit_error = data.error; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The gap_limit error doesn't work. It's because data.error
comes back as undefined. data
comes back as an object with the keys consisting of ints and values of one letter strings. One way of making it work is to set the condition as below:
if (data && Object.values(data).join('').toLowerCase().indexOf("gap limit") !== -1)
$scope.btc_gaplimit_error = true;
</div> | ||
<!-- Display Error --> | ||
<div id="display-error" class="bnomics-display-error" ng-hide="no_display_error"> | ||
<h2>Display Error</h2> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should probably use $_BLOCKLANG
as the other error messages.
</div> | ||
<!-- Gap limit Error --> | ||
<div id="address-error-btc-gaplimit" ng-show="btc_gaplimit_error" ng-cloak> | ||
<h2>Could not generate new Bitcoin address</h2> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should probably use $_BLOCKLANG
as the other error messages.
I am getting little scared by so many changes as WHMCS is working quite well currently. This is a bit low priority. We can take this up in stages after we are done with following changes in wordpress: |
Issue #94
Updates the URL structure to match Woocommerce
Updates the templates, css and js to latest versions