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

18615 updates structure for last_trans_id to be varchar 255 which is … #18621

Merged
merged 4 commits into from
Nov 24, 2018

Conversation

iancassidyweb
Copy link
Contributor

Description (*)

  • Resolves 18615
  • Klarna orders are not being stored correctly in 'sales_order_payment'.'last_trans_id' due to the structure of the field having a limit of 32 characters where as the klarna reference is currently 36 characters.
  • The structure for last_trans_id has been updated from Varchar 32 to Varchar 255
  • This is inline with the structures for:
    • klarna_core_order order_id
    • amazon_sales_order amazon_order_reference_id
  • This should provide greater compatibility with 3rd party payments.

Fixed Issues (if relevant)

  1. Field restriction incompatibilities between klarna_core_order and sales_order_payment last_trans_id #18615: Field restriction incompatibilities between klarna_core_order and sales_order_payment last_trans_id

Manual testing scenarios (*)

  1. Configure klarna payment gateway in sandbox mode https://playground.eu.portal.klarna.com
  2. Place test order using Klarna
  3. Observe the 'sales_order_payment'.'last_trans_id' and record reference
  4. Compare this reference with the value stored in 'klarna_core_order'.'klarna_order_id'
  5. Configure amazon payment gateway in sandbox mode
  6. Place test order using Amazon
  7. Observe the 'sales_order_payment'.'last_trans_id' and record reference
  8. Compare this reference with the value stored in 'amzon_sales_order'.'amazon_order_reference_id'

Contribution checklist (*)

  • [ x] Pull request has a meaningful description of its purpose
  • [ x] All commits are accompanied by meaningful commit messages
  • [ x] All new or changed code is covered with unit/integration tests (if applicable)
  • [ x] All automated tests passed successfully (all builds on Travis CI are green)

…inline with klarna_core_order order_id and amazon_sales_order amazon_order_reference_id. Limit was 32 characters where as a klarna_order reference is 36 characters at present.
@magento-cicd2
Copy link
Contributor

magento-cicd2 commented Oct 15, 2018

CLA assistant check
All committers have signed the CLA.

@magento-engcom-team
Copy link
Contributor

Hi @iancassidyweb. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

  • @magento-engcom-team give me test instance - deploy test instance based on PR changes
  • @magento-engcom-team give me $VERSION instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Assistant documentation

@@ -2007,7 +2007,7 @@ public function install(SchemaSetupInterface $setup, ModuleContextInterface $con
)->addColumn(
'last_trans_id',
\Magento\Framework\DB\Ddl\Table::TYPE_TEXT,
32,
255,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should go in an upgrade script with a version bump. Leaving this change here will only make it work for new installs and won't be reflected if anyone already installed M2 and is upgrading.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. will update at weekend.

@iancassidyweb
Copy link
Contributor Author

@miguelbalparda I got the following error from Codacy

"Codacy/PR Quality Review — Codacy was unable to analyse your pull request."

Let me know if you need anything else.

Ian

@miguelbalparda
Copy link
Contributor

Codacy seems to be disabled for 2.2. I'll check this internally, thanks for the heads up!

@magento-engcom-team magento-engcom-team added this to the Release: 2.2.8 milestone Nov 19, 2018
@magento-engcom-team
Copy link
Contributor

Hi @miguelbalparda, thank you for the review.
ENGCOM-3494 has been created to process this Pull Request

@magento-engcom-team
Copy link
Contributor

Hi @iancassidyweb. Thank you for your contribution.
We will aim to release these changes as part of 2.2.8.
Please check the release notes for final confirmation.

Please, consider to port this solution to 2.3 release line.
You may use Porting tool to port commits automatically.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants