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

Add orderBy to findOneBy to return more accurate eventLog result #11636

Merged
merged 1 commit into from
Jan 19, 2023

Conversation

pwned555
Copy link
Contributor

@pwned555 pwned555 commented Oct 26, 2022

Q A
Bug fix? (use the a.b branch) [y ]
New feature/enhancement? (use the a.x branch) [ ]
Deprecations? [ ]
BC breaks? (use the c.x branch) [ ]
Automated tests included? [ ]
Related user documentation PR URL mautic/mautic-documentation#...
Related developer documentation PR URL mautic/developer-documentation#...
Issue(s) addressed Fixes #11635

Description:

Fixes issue with wrong eventLog record being returned when trying to cancel if the user has multiple records with the same eventId.

Steps to test this PR:

Step 1: Create a Campaign that allows users to restart it
Step 2: In that Campaign create an event with a 1 hour delay that removes the user from the Campaign
Step 3: Have user go on Campaign and wait for the user to be removed and event to be fired
Step 4: Have user go on Campaign again, this time try to cancel the event before 1 hour. It should allow you to cancel the event.

@cla-bot cla-bot bot added the cla-signed The PR contributors have signed the contributors agreement label Oct 26, 2022
@codecov
Copy link

codecov bot commented Oct 26, 2022

Codecov Report

Merging #11636 (d1177d7) into 4.4 (68ee866) will increase coverage by 0.63%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##                4.4   #11636      +/-   ##
============================================
+ Coverage     49.33%   49.97%   +0.63%     
- Complexity    35395    35408      +13     
============================================
  Files          2144     2144              
  Lines        105524   106299     +775     
============================================
+ Hits          52056    53118    +1062     
+ Misses        53468    53181     -287     
Impacted Files Coverage Δ
...ndles/CampaignBundle/Controller/AjaxController.php 49.05% <100.00%> (+0.03%) ⬆️
...les/CoreBundle/EventListener/BuildJsSubscriber.php 40.00% <0.00%> (-10.00%) ⬇️
...les/PluginBundle/EventListener/PointSubscriber.php 40.00% <0.00%> (-10.00%) ⬇️
...s/CampaignBundle/EventListener/PointSubscriber.php 40.00% <0.00%> (-10.00%) ⬇️
...Bundle/EventListener/GeneratedColumnSubscriber.php 92.30% <0.00%> (-7.70%) ⬇️
...les/FormBundle/EventListener/WebhookSubscriber.php 72.72% <0.00%> (-7.28%) ⬇️
...les/PageBundle/EventListener/WebhookSubscriber.php 72.72% <0.00%> (-7.28%) ⬇️
...dles/EmailBundle/EventListener/PointSubscriber.php 73.91% <0.00%> (-5.04%) ⬇️
.../PreUpdateChecks/CheckDatabaseDriverAndVersion.php 75.00% <0.00%> (-5.00%) ⬇️
...es/CoreBundle/Templating/Helper/GravatarHelper.php 85.71% <0.00%> (-4.77%) ⬇️
... and 372 more

Copy link
Member

@kuzmany kuzmany left a comment

Choose a reason for hiding this comment

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

Make sense 👍

@kuzmany kuzmany added the pending-test-confirmation PR's that require one test before they can be merged label Oct 27, 2022
@RCheesley RCheesley mentioned this pull request Oct 28, 2022
21 tasks
@escopecz escopecz mentioned this pull request Nov 3, 2022
22 tasks
@beetofly
Copy link
Contributor

beetofly commented Nov 4, 2022

@pwned555 thank you.
I tested it as described and it works.

Copy link
Contributor

@volha-pivavarchyk volha-pivavarchyk left a comment

Choose a reason for hiding this comment

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

Works well as described
image

I am not be able to cancel the event on branch 4.4.

@escopecz escopecz mentioned this pull request Nov 11, 2022
25 tasks
@escopecz escopecz mentioned this pull request Nov 18, 2022
22 tasks
@RCheesley RCheesley added ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged contacts Anything related to contacts bug Issues or PR's relating to bugs labels Nov 25, 2022
@RCheesley RCheesley added this to the 4.4.5 milestone Nov 25, 2022
@RCheesley
Copy link
Sponsor Member

@pwned555 please can you make a duplicate PR on the 5.x branch then this is good to be merged for the release. Thanks!

@RCheesley RCheesley added the pending-companion-pr Waiting for the author to create a PR on the other branch. label Nov 25, 2022
@RCheesley RCheesley removed this from the 4.4.5 milestone Nov 30, 2022
@RCheesley
Copy link
Sponsor Member

Bumping this from the release until we have the 5.x PR to merge alongside.

@mabumusa1 mabumusa1 removed pending-test-confirmation PR's that require one test before they can be merged pending-companion-pr Waiting for the author to create a PR on the other branch. labels Jan 19, 2023
@mabumusa1
Copy link
Member

@escopecz this is ready now

@escopecz escopecz added this to the 4.4.6 milestone Jan 19, 2023
@escopecz escopecz merged commit 3d4c18a into mautic:4.4 Jan 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issues or PR's relating to bugs cla-signed The PR contributors have signed the contributors agreement contacts Anything related to contacts ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants