Skip to content

Even existing credit memos should be refundable if their state is open #11550

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

Merged
merged 3 commits into from
Nov 21, 2017

Conversation

AntonEvers
Copy link
Contributor

@AntonEvers AntonEvers commented Oct 19, 2017

Description

Credit memos can have the state open: \Magento\Sales\Model\Order\Creditmemo::STATE_OPEN.
This means that it is possible to have a creditmemo with an ID which still has to be refunded.
I'm creating a module that introduces a validation step for refund payments before the actual refund can take place. It uses the open state of credit memos to wait for approval and then refunds the creditmemo. Right now the credit memo is not refundable once it has an ID. I think this is incorrect in case of open credit memos.

Fixed Issues (if relevant)

  1. none

Manual testing scenarios

  1. This requires custom (plugin) code.

Contribution checklist

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

Credit memos can have the state open: `\Magento\Sales\Model\Order\Creditmemo::STATE_OPEN`.
This means that it is possible to have a creditmemo with an ID which still has to be refunded.
I'm creating a module that introduces a validation step for refund payments before the actual refund can take place. It uses the open state of credit memos to wait for approval and then refunds the creditmemo. Right now the credit memo is not refundable once it has an ID. I think this is incorrect in case of open credit memos.
@ishakhsuvarov
Copy link
Contributor

Hi @ajpevers
Please add the test covering this case. Thanks!

@AntonEvers
Copy link
Contributor Author

@ishakhsuvarov will do

Anton Evers added 2 commits October 22, 2017 13:54
`$creditmemo->getState() = "1"`
`Creditmemo::STATE_OPEN = 1`
@AntonEvers
Copy link
Contributor Author

@ishakhsuvarov tests added

@AntonEvers
Copy link
Contributor Author

@ishakhsuvarov can you review this PR?

@ishakhsuvarov
Copy link
Contributor

Hi @ajpevers
This PR is already in a queue for manual testing. Will be delivering it soon, if all goes well.
Thanks and sorry for the delay.

@okorshenko okorshenko merged commit 480f8ae into magento:2.2-develop Nov 21, 2017
okorshenko pushed a commit that referenced this pull request Nov 21, 2017
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.

5 participants