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

Modify Omise PrestaShop to work with PrestaShop 1.7 #47

Merged
merged 95 commits into from Nov 29, 2017

Conversation

nimid
Copy link
Contributor

@nimid nimid commented Nov 24, 2017

1. Objectives

  1. Make the module, Omise PrestaShop, to work with Prestashop 1.7.
  2. Save and display payment method name correctly as selected by payer.
  3. Add PrestaShop order ID to metadata of Omise charge.
  4. Implement a new feature, Omise Webhooks, and handle an event, charge.complete.

Related information:

2. Description of change

Change Description
1. Make the module, Omise PrestaShop, to work with Prestashop 1.7. #30, #31, #32, #33, #34, #35, #36, #37, #38
2. Save and display payment method name correctly as selected by payer. #41
3. Add PrestaShop order ID to Omise charge. #42
4. Implement a new feature, Omise Webhooks, and handle an event, charge.complete. #43, #44, #45, #48, #49

3. Quality assurance

The quality assurance can be found in the above pull requests mentioned in section 2. Description of change.

4. Impact of the change

This modification is NOT compatible with PrestaShop 1.6.

This modification compatible with PrestaShop 1.7.

Due to source code and architecture changes in PrestaShop 1.7, the module, Omise PrestaShop, has implemented to follow the changes.

So, since this pull request, the module, Omise PrestaShop, has been updated to works with PrestaShop 1.7. The merchant that using PrestaShop 1.6 can still uses the module, Omise PrestaShop, by using the version, v1.3.

5. Priority of change

Normal

6. Additional notes

If the module does not allow to be installed on PrestaShop 1.7, when
install it on PrestaShop 1.7, the module installation is failed.
The logo is the same. Only image size has been changed.

The image size has been changed to 32x32. Both PrestaShop 1.6 and 1.7
require the same image size.

References:
- PrestaShop 1.6, http://doc.prestashop.com/display/PS16/Creating+a+first+module#Creatingafirstmodule-Theiconfile
- PrestaShop 1.7, http://developers.prestashop.com/module/05-CreatingAPrestaShop17Module/02-CreatingAFirstModule.html#the-icon-file
Display card payment form by reuse the function displayPayment().
Remove 2 assigned variables that used to display at card payment form.
The removed variables are `action` and `title`.

PrestaShop 1.7 has changed the mechanism to submit payment form.
It will generated a submit button for all payment options.

So, each payment option will be changed to override the action of the
PrestaShop 1.7. instead of has it own action.

The `title` at the top of the payment form has changed to be displayed
at the payment option instead. So, display the title at the top of
payment form is unnecessary and duplicated.
For PrestaShop 1.7, submit payment button is unnecessary. To submit
payment, it will be changed to override the action of PrestaShop 1.7
instead.
The correct function of PrestaShop 1.7 new API that used to display card
payment form is `setForm()`.

It is not function, `setAdditionalInformation()`.

References:
- http://developers.prestashop.com/module/50-PaymentModules/index.html#paymentoption
- https://github.com/PrestaShop/paymentexample/blob/master/paymentexample.php#L143
Define a constant to be the identifier for card payment option. This
constant will be used at the front office, payment step, payment
options.
Modify JavaScript in card payment page to create Omise card token.

The mechanism to submit payment for support PrestaShop 1.7 has been
changed from the module has it own submit payment button to using the
submit payment button that PrestaShop generated.
Re-add a missing JavaScript condition. This condition is lost during
the modification of previous commit.

This condition is check whether the external library, Omise.js,
is accessible. If Omise.js is not accessible, the error message will be
displayed. The reason that Omise.js is not accessible, may be from the
slow connection or timeout.

The Omise.js is an important library that used to create Omise card
token at the client side.

Without this condition, the customer (payer) can not clearly know the
problem when submit payment with Omise card payment. The error will be
not displayed visually. It will be displayed at the console of web browser.
That is not easy for customer to know the problem.
Remove an unnecessary <p> tag in the card payment page, payment.tpl.

This <p> tag contains <div>, which it is improper and it causes an error
when the debug mode has been activated. In normal mode, the error does not
appear.

The error has been displayed at the checkout step, full page.

The debug mode can be activated by go to PrestaShop back office,
from the left menu, CONFIGURE > Advanced Parameters > Performance >
DEBUG MODE.

Reference:
- W3C <p> tag recommendation, https://www.w3.org/TR/html401/struct/text.html#h-9.3.1
Pass the action URL from server side to client side.

At the client side, if the Omise card token has been successfully
created, it will be submitted from client side to server side for create
Omise charge at server side.
PrestaShop 1.7 new payment API has changed the parameters passed to `hookDisplayOrderConfirmation()`.

The impact is key that used to retrieved current order has been changed
from `objOrder` to `order`.

Without this commit, the system will crash, view can not be rendered.

Reference:
- http://build.prestashop.com/news/module-development-changes-in-17/#before
PrestaShop 1.7 has changed the parameter syntax that passed to function
`setTemplate()`.
Modify order confirmation template. Make it consistent with PrestaShop
1.7.
Make it consistent with PrestaShop 1.7.
Override the display of `page_content_container` in order payment error
template, payment-error.tpl.

This override prevents the unnecessary display of blank content that
come from page.tpl which this template, payment-error.tpl, has extended.
In the setting page, it has a variable, `confirmation`. This variable is
used for display the confirmation message when the merchant saved the
setting.

If merchant enabled the debug mode and open the setting page, the system
displays the error message popup, Undefined index: confirmation.

This problem is occurred because at the time that merchant open the setting
page and does not save the setting. The variable, `confirmation`,
has not been assigned any value from server side to display any message.

This problem is not occurred in normal mode.
Change 3-D Secure payment. Make it save an order with its payment method
name.
Check the module name of saved order when return from Omise API.

According to the payment method of saved order has been dynamically
saved follows the payment method that customer selected. Such as card
payment or internet banking payment.

Therefor, payment method of saved order (order->payment) is variable.
It can not be used to check whether the order is the order that made
payment with Omise.

So, using module name of saved order (order->module) to check with
Omise.MODULE_NAME is properly.
Reduced the duplicated mocking source code by move it to the
autoload.php.
Attach order ID to metadata when create Omise internet banking charge.
Append PrestaShop order ID to Omise charge metadata when creating a card
payment (normal card payment and 3DS payment).
This commit affects normal card payment only. 3DS payment is not
affected.

The objective of this commit is to append PrestaShop order ID to the
metadata of Omise charge.

The flow has been changed to save an order as processing at the first
step and uses that generated order ID to append to the metadata of Omise charge.
Add a front office controller that will be used to handle the webhooks
request from Omise server.

For this commit, the controller has no any logic. It does only display
the empty page. The logic will be implemented in the next commits.
This class wrapped the PrestaShop logger class, Logger. The Logger class
requires some parameters which they are not required or compatibled with
module such as $object_id or $object_type. The data type of $object_id is
int but the data type of Omise object id is string or $object_type is
the internal PrestaShop object, etc.

So, the wrapper class, OmiseLogger, has reduced the parameters that it is
not used by provides the necessary parameters, $message and $severity.
And $severity is optional the default value of $severify is info log
level.

The same log message can be added to database because OmiseLogger has
set a parameter, $allow_duplicate, to be true. It has the intent that
the same log message but differnt time can be added to database.
Theses constants will be used for webhooks in the step of checking the
status of charge.
Add a function OmiseTransactionModel.getIdOrder(), to retrieve a
PrestaShop order ID from omise transaction table.

This function is search by using Omise charge ID.
The unit test in this commit has only one test case because in actual
class, OmiseEventHandler, in has a static class attribute dependency,
OmiseEventChargeComplete::KEY, and a hard dependency (new Keyword),
new OmiseEventChargeComplete(), with the same class. This situation is
hard to create mock for unit testing.

So, a unit test case in this commit that test class, OmiseEventHandler,
is the test case that across the dependency that hard to mock.
Remains the PrestaShop order status to be Processing in progress, if
Omise charge is not paid.

This commit has an objective to solve the possible problem of offsite
payment such as internet banking payment that the notification from bank
server to Omise server is delayed and Omise can not knows or updates the
charge status.

For the internet banking payment, when the payer completed their payment
on bank website and bank redirects payer back to PrestaShop site.

In this step, it is possible that the notification from bank server to
Omise server can be deplayed or slower than the redirection of payer.
This situation causes the charge status is pending and charge is not
paid because Omise does not know the status of payment.

So, it is safer that if the charge is not paid the PrestaShop order
status still Processing in progress.

Note:
The implementation of webhooks to prevent this problem by correctly
synchronize the charge information between PrestaShop order and Omise
charge has been completed in the previous commits.
When the payer has been redirected back to PrestaShop, the module, Omise
PrestaShop, will checks the information of Omise charge by request the
information via Omise API.

Before change:
If Omise charge is NOT paid, the empty error message on the order
confirmation page will be displayed. This result is not correct and properly
because in this step, the PrestaShop order is valid and Omise charge is
not failed. The status of Omise charge is not paid. It does not mean
that the payment is error.

After change:
The success order confirmation page is display normally. Regardless of
Omise charge status is paid or unpaid.

The main objective of before change and after change still the same. If the
Omise charge is paid, the PrestaShop order status will be updated
to be success (Payment accepted).
Remove a condition that checking current PrestaShop order status,
when redirect payer back to PrestaShop site.

This commit has an objective to solve a problem of internet banking
payment. The problem occurs in the situation that;

the redirection of payer back to PrestaShop site is slower than the
payment result notification from bank to Omise and Omise charge of this
payment is failed.

This situation causes the invalid result message on PrestaShop order
confirmation page.

Note:
Due to the offsite payment that has the redirection flow such as
internet banking payment, including the implementation of Omise
webhooks, it is possible that;

- Omise charge is failed.
- The notification from bank to Omise was passed or forwarded to the
PrestaShop site via Omise webhooks FASTER THAN the redirection of payer
to PrestaShop.
- Omise PrestaShop handles this webhooks event by change the PrestaShop
order status to be Canceled.

So, this condition or situation causes the invalid result message on
PrestaShop order confirmation page.
@nimid nimid merged commit afe3f80 into master Nov 29, 2017
@nimid nimid deleted the support-prestashop-1.7 branch November 29, 2017 09:32
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.

None yet

1 participant