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

[ENGA3-510]: Issue of two invoices of the same order has been fixed. #387

Merged
merged 3 commits into from
Oct 10, 2022

Conversation

aashishgurung
Copy link
Contributor

@aashishgurung aashishgurung commented Oct 10, 2022

1. Objective

Fix the issue of two invoices of the same order.

Jira Ticket: #510

2. Description of change

The default value of Generate invoice at order status was set to \Magento\Sales\Model\Order::STATE_PENDING_PAYMENT. So, comparing the return value of $this->config->getSendInvoiceAtOrderStatus() with Order::STATE_PENDING_PAYMENT for equality was failing. We had to manually change the value to set the proper value which are either pending_payment and processing.

With this PR, we changed the default value to pending_payment. For the merchants who have already installed our plugin, we added an if check that returns pending_payment from Order:STATE_PENDING_PAYMENT if the setting's value is \Magento\Sales\Model\Order::STATE_PENDING_PAYMENT.

After the fix

Screen.Recording.2565-10-10.at.15.16.12.mov

Before the fix

Screen.Recording.2565-10-10.at.15.35.21.mov

3. Quality assurance

  • Platform version: Magento 2.4.5
  • Omise plugin version: Omise-Magento 2.29.1
  • PHP version: 8.1

✏️ Details:

  • Enable webhook
  • Enable credit card
  • Set payment action to Authorize and Capture
  • Checkout with 3DS.

You should see only one invoice and the amount should be paid.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@davidhanh
Copy link

@aashishgurung Do you think you an also include a screencast in the PR's description? It will be easier to understand what the changes are about.

@aashishgurung
Copy link
Contributor Author

@aashishgurung Do you think you an also include a screencast in the PR's description? It will be easier to understand what the changes are about.

I have included the recording of placing an order.

@aashishgurung aashishgurung merged commit e0d0357 into master Oct 10, 2022
@aashishgurung aashishgurung deleted the bug/ENGA3-510-two-invoices-of-same-order branch November 21, 2022 06:29
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.

5 participants