Refactor change stage campaign action to use batches #8969
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Prevent unpublished emails from triggering unpublished campaigns.
This was referenced Jun 29, 2020
Travis tests have failedHey @mohit-rocks, 2nd Buildif [ ${TRAVIS_PHP_VERSION:0:3} == "7.3" ]; then composer test -- --coverage-clover=coverage.xml && bash <(curl -s https://codecov.io/bash); else composer test; fiTravisBuddy Request Identifier: c09b4c50-ba29-11ea-a74d-ef5d1a577ce8 |
|
@mohit-rocks Can you please update the tests for Travis to pass? Thanks! 😊 |
Codecov Re 8000 port
@@ Coverage Diff @@
## 3.3 #8969 +/- ##
============================================
+ Coverage 40.22% 40.27% +0.05%
- Complexity 34043 34044 +1
============================================
Files 1990 1990
Lines 105412 105419 +7
============================================
+ Hits 42404 42460 +56
+ Misses 63008 62959 -49
|
|
@mohit-rocks I tested the bug on mautibox.com/staging to ensure I could replicate it before I applied the PR, and I do not experience the bug you are referencing. Take a look at this screencast: https://youtu.be/QG6JlcUF2QA In short:
@kuzmany were you able to reproduce the bug before you tested the PR? I think we need another test on this before we can decide whether the PR is required. |
|
@RCheesley Nope, I didn't test reproduce. I just test If both actions works properly after patch PR. I don't think before PR campaign was unpublished, but this PR improve other stuff, and this is reason why I approved. |
|
Original issue was already fixed in #8907 - nevertheless this PR might still be relevant. Maybe @alanhartless could explain a bit more like @kuzmany mentioned? |
|
Bumping to 3.1.1 due to the lack of response from original poster and PR author. |
|
@cla-bot check |
|
We require contributors to sign our Contributor License Agreement, and we do not have a record of your signature on file. In order for us to review and merge your code, please head over to https://www.mautic.org/contributor-agreement and complete the form. There may be a short delay while the team add you as a contributor - please be patient :). Any problems contact @RCheesley. CLA has not been signed by @mohit-rocks. |
|
The CLA Bot has been sent on a mission to check against the latest list and will be back shortly with its findings! |
|
@mohit-rocks @kuzmany this one has a conflict to be addressed and I am not sure if we got to the bottom of whether this is needed as a couple of testers were not able to reproduce the issue. @alanhartless could you review the issue and PR and give your thoughts? |
escopecz
requested changes
Apr 16, 2021
There was a problem hiding this comment.
This PR refactors the change stage campaign action to work in batches instead of one event at a time. So it improves performance but should not change the behavior.
Other than adding back the translation string this is good to go based on the code review. I did not test it but we (Acquia) use this in production for 2 years and Zdeno tested it.
@mohit-rocks try to keep the original author of the commit. It creates confusion on who is the author and who should answer questions about the code change.
escopecz
previously approved these changes
Apr 16, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
closes #8521
Original PR by Alan: #7812
Description:
If an email is unpublished or if a contact fails to move a stage, the event triggered a failure that would lead to notifications to users and campaigns being unpublished. This fixes that.
Steps to reproduce the bug:
Steps to test this PR:
List deprecations along with the new alternative:
List backwards compatibility breaks: