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

Do not startEnvironmentEmulation for current store #1578

Merged
merged 17 commits into from
Sep 1, 2022
Merged

Do not startEnvironmentEmulation for current store #1578

merged 17 commits into from
Sep 1, 2022

Conversation

luigifab
Copy link
Contributor

Description

This PR do not run startEnvironmentEmulation when the targeted store is the current store.
I don't know if this is the best way, for now our emails seem to keep working.

When a customer place an order, the new order email is sent.
To do that, startEnvironmentEmulation is triggered.

Three screenshots from BlackFire, before:
a1
a2
After:
b2

OpenMage 20.0.10 / PHP 7.4.6

Manual testing scenarios

  • Front: place an order, check new order email sent
  • Back: go to any order / invoice / shipment / credit memo, click on send email button, check emails sent

Contribution checklist

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All automated tests passed successfully (all builds are green)

@github-actions github-actions bot added Component: Core Relates to Mage_Core Component: Sales Relates to Mage_Sales labels Apr 30, 2021
@kiatng kiatng added the performance Performance related label Apr 30, 2021
@fballiano
Copy link
Contributor

I wouldn't have changed the $exception variable name but other than that wow... this is a great PR.

@luigifab luigifab marked this pull request as ready for review May 13, 2021 08:11
fballiano
fballiano previously approved these changes Aug 3, 2022
@fballiano
Copy link
Contributor

@luigifab did you check the phpstan notes?

@luigifab
Copy link
Contributor Author

Yes for all, if $appEmulation is not defined, $initialEnvironmentInfo is also not defined.

fballiano
fballiano previously approved these changes Aug 31, 2022
kiatng
kiatng previously approved these changes Sep 1, 2022
Copy link
Contributor

@sreichel sreichel left a comment

Choose a reason for hiding this comment

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

isset() could be merged

@luigifab luigifab dismissed stale reviews from kiatng and fballiano via a3b7c34 September 1, 2022 04:56
luigifab and others added 8 commits September 1, 2022 06:56
Co-authored-by: sv3n <github-sr@hotmail.com>
Co-authored-by: sv3n <github-sr@hotmail.com>
Co-authored-by: sv3n <github-sr@hotmail.com>
Co-authored-by: sv3n <github-sr@hotmail.com>
Co-authored-by: sv3n <github-sr@hotmail.com>
Co-authored-by: sv3n <github-sr@hotmail.com>
Co-authored-by: sv3n <github-sr@hotmail.com>
Co-authored-by: sv3n <github-sr@hotmail.com>
@fballiano
Copy link
Contributor

isset() could be merged

niceeee I didn't know that syntax!

@fballiano fballiano merged commit 712a3ee into OpenMage:1.9.4.x Sep 1, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Sep 1, 2022

Unit Test Results

1 files  ±0  1 suites  ±0   0s ⏱️ ±0s
0 tests ±0  0 ✔️ ±0  0 💤 ±0  0 ❌ ±0 
7 runs  ±0  5 ✔️ ±0  2 💤 ±0  0 ❌ ±0 

Results for commit 712a3ee. ± Comparison against base commit acef8f0.

@luigifab luigifab deleted the startEnvironmentEmulation branch September 1, 2022 08:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Core Relates to Mage_Core Component: Sales Relates to Mage_Sales performance Performance related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants