Skip to content

Fix payment instruction location in on-hold email (#31880) #31886

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.

Sign up for GitHub

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

Jump to bottom
Merged
jorgeatorres merged 6 commits into from
Dec 8, 2022
Merged

Fix payment instruction location in on-hold email (#31880) #31886

jorgeatorres merged 6 commits into from
Dec 8, 2022

Conversation

@Melebius
Copy link
Contributor

@Melebius Melebius commented Feb 11, 2022

Show payment instructions after the relevant sentence and before the intro to ordered items.

New action 'woocommerce_email_payment_instructions' introduced.

All Submissions:

Changes proposed in this Pull Request:

Closes #31880 using the more thorough solution (option 2). This is the new look of the email:

screenshot

How to test the changes in this Pull Request:

  1. Set up an offline payment method like BACS.
  2. Create an order using the offline payment method.
  3. Open the received on-hold email sent to the customer.

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully run tests with your changes locally? Yes, unit tests.

This change should be safe to use with extensions adding other offline payments methods. They would simply continue to show the payment instructions in the original location until they’re updated to use the new hook.

Changelog entry

Tweak: Corrected the position of payment instructions in on-hold emails

FOR PR REVIEWER ONLY:

  • I have reviewed that everything is sanitized/escaped appropriately for any SQL or XSS injection possibilities. I made sure Linting is not ignored or disabled.

vedanshujain
vedanshujain reviewed Feb 18, 2022
View reviewed changes
Copy link
Contributor

@vedanshujain vedanshujain left a comment

Choose a reason for hiding this comment

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

@barryhughes suggested that we can actually just remove the text 'In the meantime, here’s a reminder of what you ordered: and that will also fix the issue. See inline comment

@Melebius
Copy link
Contributor Author

Melebius commented Feb 18, 2022

@barryhughes suggested that we can actually just remove the text 'In the meantime, here’s a reminder of what you ordered: and that will also fix the issue. See inline comment

Yes, this would be the simplest solution if no one insists on keeping that sentence.

@barryhughes
Copy link
Member

barryhughes commented Feb 18, 2022

Yes, this would be the simplest solution if no one insists on keeping that sentence.

I think simplicity wins, here—the existing solution is comprehensive and would otherwise be a good way to go: but that particular phrase doesn't really seem to be needed (not to mention, it can easily be added back by those who wish to keep it) so reducing the complexity and size of the change seems worthwhile 👍

@Melebius Melebius mentioned this pull request Feb 19, 2022
Closed
5 tasks
@ObliviousHarmony ObliviousHarmony added the plugin: woocommerce Issues related to the WooCommerce Core plugin. label Feb 21, 2022
@Konamiman
Copy link
Contributor

Konamiman commented Jul 11, 2022

Hi @Melebius, it seems that we agreed that just removing the "In the meantime..." part was a better solution. Could you please implement the change so we can unblock this pull request?

@Konamiman Konamiman added the needs: author feedback The issue/PR needs a response from any of the parties involved in the issue. label Jul 11, 2022
@barryhughes barryhughes self-assigned this Sep 22, 2022
Melebius and others added 4 commits September 23, 2022 02:14
@Melebius @barryhughes
Fix payment instruction location in on-hold email
686757c 
Show payment instructions after the relevant sentence
and before the intro to ordered items.

New action 'woocommerce_email_payment_instructions' introduced.
@barryhughes
Roll back introduction of new hook, simplify on-hold email messaging.
7da56ad
@barryhughes
Update template version tags.
644f5df
@barryhughes
Changelog.
f44f02a
@barryhughes barryhughes force-pushed the fix/email-payment-instructions branch from 5a80bc3 to f44f02a Compare September 23, 2022 02:25
@barryhughes
Copy link
Member

barryhughes commented Sep 23, 2022

  • Rebased against trunk since it's been sitting for a while.
  • Rolled back the introduction of a new hook.
  • Simplified the wording (the simpler option discussed earlier).

@barryhughes barryhughes requested review from a team and vedanshujain and removed request for a team September 23, 2022 02:26
@barryhughes barryhughes requested review from a team and jorgeatorres and removed request for a team and vedanshujain November 30, 2022 20:32
@barryhughes barryhughes removed the needs: author feedback The issue/PR needs a response from any of the parties involved in the issue. label Nov 30, 2022
jorgeatorres
jorgeatorres approved these changes Dec 8, 2022
View reviewed changes
Copy link
Member

@jorgeatorres jorgeatorres left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@jorgeatorres jorgeatorres merged commit 02fec37 into woocommerce:trunk Dec 8, 2022
@github-actions github-actions bot added this to the 7.3.0 milestone Dec 8, 2022
samueljseay pushed a commit that referenced this pull request Dec 15, 2022
@Melebius @barryhughes @samueljseay
Fix payment instruction location in on-hold email (#31880) (#31886)
2c446e8 
* Fix payment instruction location in on-hold email
* Update template version tags.
* Changelog.
* Update version tag to 7.3.0.
* Update version tag to 7.3.0.

Co-authored-by: barryhughes <[email protected]>
@github-actions github-actions bot mentioned this pull request Jan 13, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jorgeatorres jorgeatorres jorgeatorres approved these changes

+1 more reviewer

@vedanshujain vedanshujain vedanshujain left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@barryhughes barryhughes

Labels

plugin: woocommerce Issues related to the WooCommerce Core plugin. type: community contribution

Projects

None yet

Milestone

7.3.0

Development

Successfully merging this pull request may close these issues.

Payment instructions placement disrupts text flow in on-hold email

7 participants

@Melebius @barryhughes @Konamiman @jorgeatorres @vedanshujain @psealock @ObliviousHarmony