omise / omise-prestashop

Omise PrestaShop Plugin
https://docs.opn.ooo/prestashop-plugin
MIT License
4 stars 7 forks source link

Create an Omise card token #12

Closed nimid closed 7 years ago

nimid commented 7 years ago

1. Objective

Create an Omise card token from the valid card information that payer filled in the checkout form. The token will be used to process the charge in the next step.

Related information: Related issue: - Related ticket: T1122 Required pull request: #11

2. Description of change

Note: The JavaScript code has followed the PrestaShop coding standard (at the JavaScript code section).

3. Quality assurance

Environments:

Details:

Test case 1: Successfully create Omise card token

The screenshot below shows the Omise card token was defined to be the value for HTML hidden after the payer filled the valid card information.

order_-_omise-prestashop

Test case 2: Display the error message if card information is invalid

The screenshot below shows the popup contained the error message and it is no Omise card token has been created and defined.

order_-_omise-prestashop_and_create_an_omise_card_token_by_nimid_ _pull_request__12_ _omise_omise-prestashop

4. Impact of the change

-

5. Priority of change

Normal

6. Additional Notes

-

guzzilar commented 7 years ago

@nimid, fyi, going to rebase #11 feature/display-checkout-form => here, feature/create-card-token

guzzilar commented 7 years ago

rebased #11 feature/display-checkout-form => feature/create-card-token

guzzilar commented 7 years ago

@nimid LGTM 👍

Btw, is it possible to reduce that document.getElementById() part? something like,

const form = {
  name: document.getElementById('omise_card_holder_name'),
  number: document.getElementById('omise_card_number'),
  expiration_month: document.getElementById('omise_card_expiration_month'),
  expiration_year: document.getElementById('omise_card_expiration_year'),
  security_code: document.getElementById('omise_card_security_code'),
}

const omiseLockCheckoutForm = function omiseLockCheckoutForm(form) {
  form.name.disabled = true;

  // blah
}

const omiseUnlockCheckoutForm = function omiseUnlockCheckoutForm(form) {
  form.name.disabled = false;

  // blah
}

To reduce executing document.getElementById() with the same fields multiple times.

note, I didn't try ☝️ by myself. It might not work because of this line, https://github.com/omise/omise-prestashop/pull/12/files#diff-703d934df9e134d3b80f37155717d048R100

nimid commented 7 years ago

@guzzilar

note, I didn't try ☝️ by myself. It might not work because of this line, https://github.com/omise/omise-prestashop/pull/12/files#diff-703d934df9e134d3b80f37155717d048R100

For the line that you mentioned, omiseUnlockCheckoutForm();, what is make it not work?

I have tried to modify but can not found the problem.

guzzilar commented 7 years ago

@nimid https://github.com/omise/omise-prestashop/pull/12#issuecomment-272685956 Just Im not sure if t's gonna work with that omiseCreateTokenCallback() function.

Because that function will be executed by Omise.createToken('card', card, omiseCreateTokenCallback);but my idea has to pass form object through omiseUnlockCheckoutForm(form).

So, not sure if I can pass form object within that callback function.

nimid commented 7 years ago

@guzzilar Ok, thanks. I will test more further with the callback function as well.

nimid commented 7 years ago

@guzzilar https://github.com/omise/omise-prestashop/pull/12#issuecomment-272290720

Done.

I have modified and tested to the process of save the order. The system can work correctly.

I have modified this pull request description by add a second test case that show the error message if the card information is invalid.

guzzilar commented 7 years ago

@nimid fyi, rebased develop to #12 feature/create-card-token

guzzilar commented 7 years ago

👍