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

Fix maintenance cleanup with gdpr option #7403

Merged

Conversation

Dcoutelle
Copy link
Contributor

@Dcoutelle Dcoutelle commented Apr 10, 2019

Please be sure you are submitting this against the staging branch.

Q A
Bug fix? Y
New feature? N
Automated tests included? N
Related user documentation PR URL
Related developer documentation PR URL
Issues addressed (#s or URLs) N
BC breaks? N
Deprecations? N

Description:

Command mautic:maintenance:cleanup -g is broken and lead to SQL error :

 [Doctrine\DBAL\DBALException]                                                                                                                                                                                    
  An exception occurred while executing 'SELECT id FROM leads l WHERE (l.last_active <= :date) OR ((l.date_added <= :date2) AND (l.last_active IS NULL)) LIMIT 10000 OFFSET 0' with params ["2016-04-10 07:09:05"  
  ]:                                                                                                                                                                                                               
                                                                                                                                                                                                                   
  Value for :date2 not found in params array. Params array key should be "date2"

Steps to reproduce the bug:

Run command mautic:maintenance:cleanup -g

Steps to test this PR:

  1. Apply Patch
  2. run mautic:maintenance:cleanup -g and see command running properly

Copy link
Member

@npracht npracht left a comment

Choose a reason for hiding this comment

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

👍

@npracht npracht added bug Issues or PR's relating to bugs code-review-needed PR's that require a code review before merging pending-test-confirmation PR's that require one test before they can be merged labels Apr 10, 2019
@npracht npracht added this to the 2.16.0 milestone Apr 10, 2019
@npracht
Copy link
Member

npracht commented Apr 10, 2019

@Woeler i don't know why in our tests last time if was working... The variable was not correctly named, i think just code review is enough ;)

EDIT: @Woeler the dry run option is not running the same variables, that can explain why.

@npracht npracht requested a review from Woeler April 10, 2019 07:19
@npracht npracht modified the milestones: 2.16.0, 2.15.2 Apr 10, 2019
@npracht npracht added this to Ready to Test (confirmation) in Mautic 2 Apr 11, 2019
Mautic 2 automation moved this from Ready to Test (confirmation) to Ready to Test (first time) Apr 17, 2019
Copy link
Contributor

@Woeler Woeler left a comment

Choose a reason for hiding this comment

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

Whoops sorry, been a bit busy moving haha. Yea I'm not entirely sure anymore. I might have also looked at the wrong data then, anyway, this should definitely do good.

@npracht npracht added ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged and removed code-review-needed PR's that require a code review before merging pending-test-confirmation PR's that require one test before they can be merged labels Apr 18, 2019
@npracht npracht moved this from Ready to Test (first time) to Ready to Commit (passed testing) in Mautic 2 Apr 18, 2019
Mautic 2 automation moved this from Ready to Commit (passed testing) to Ready to Test (first time) Apr 19, 2019
@kuzmany kuzmany merged commit f9ad684 into mautic:staging Apr 19, 2019
Mautic 2 automation moved this from Ready to Test (first time) to Merged Apr 19, 2019
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 ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged
Projects
No open projects
Mautic 2
  
Merged
Development

Successfully merging this pull request may close these issues.

None yet

4 participants