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

(#4794) Schedule and send reports #5205

Merged
merged 51 commits into from
Nov 22, 2017

Conversation

Maxell92
Copy link
Contributor

@Maxell92 Maxell92 commented Oct 25, 2017

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

Description:

This PR adds an ability to schedule reports and email them to a specified list of users.

Steps to test this PR:

  1. Checkout this branch
  2. Run composer install and app/console d:m:m. Clear cache.
  3. Go to report and edit / create one with sample data.
  4. You should see a new tab - Schedule.
  5. Try to save report with no scheduling - just to make sure it is not broken. Check tables in database - reports (scheduled fields should be NULL) + reports_schedulers (no records here yet).
  6. Run command app/console mautic:reports:scheduler. There should be no error.
  7. Try to save scheduler with no email. You should get an error.
  8. Hack Selectbox with inspections and write incorrect value there. Form cannot be saved with bad data.
  9. Try all possible combination provided by schedule form and check preview dates if they match conditions correctly. Try a week days option pls :)
  10. Save report and check both tables, if saved data are correct. reports_schedulers holds only 1 date of next schedule event. Do this with as much combination as possible.
  11. Put your email to to field and save a report. Make sure you have configured sending emails from your Mautic instance.
  12. Go to reports_schedulers table and edit date of scheduler - set it to some past date.
  13. Run app/console mautic:reports:scheduler and you should get an email with report. Check the attachment that results match report data.
  14. Export report manually (especially CSV and Excel export - they were refactored) and check results.
  15. Select report with large amount of data or edit report_export_max_filesize_in_bytes in app/ReportBundle/Config/config.php to have bigger file size than in settings. Set correct date in reports_schedulers table and send the report. You should get an email with no attached file and link to the report detail.

List deprecations along with the new alternative:

List backwards compatibility breaks:

@Maxell92 Maxell92 added feature A new feature for inclusion in minor or major releases WIP PR's that are not ready for review and are currently in progress labels Oct 25, 2017
@Maxell92 Maxell92 added this to the 2.12.0 milestone Oct 25, 2017
@Maxell92 Maxell92 self-assigned this Oct 25, 2017
@Maxell92 Maxell92 changed the title (#4794) Report schedule base - form and entity WIP (#4794) Report schedule base - form and entity Oct 25, 2017
@Maxell92 Maxell92 changed the title WIP (#4794) Report schedule base - form and entity WIP (#4794) Schedule and send reports Oct 31, 2017
@Maxell92 Maxell92 force-pushed the feature/4794-report-schedule branch 3 times, most recently from 5a38bb0 to 81a069a Compare November 3, 2017 17:19
@Maxell92 Maxell92 added ready-to-test PR's that are ready to test and removed WIP PR's that are not ready for review and are currently in progress labels Nov 6, 2017
@Maxell92 Maxell92 changed the title WIP (#4794) Schedule and send reports (#4794) Schedule and send reports Nov 6, 2017
@Maxell92 Maxell92 removed their assignment Nov 6, 2017
app/migrations/Version20171027134920.php Outdated Show resolved Hide resolved
app/migrations/Version20171027134920.php Outdated Show resolved Hide resolved
app/bundles/ReportBundle/Views/Report/form.html.php Outdated Show resolved Hide resolved
app/bundles/ReportBundle/Entity/Report.php Outdated Show resolved Hide resolved
app/bundles/ReportBundle/Views/Report/form.html.php Outdated Show resolved Hide resolved
app/bundles/ReportBundle/Views/Report/form.html.php Outdated Show resolved Hide resolved
// this up() migration is auto-generated, please modify it to your needs
$this->abortIf($this->connection->getDatabasePlatform()->getName() != 'mysql', 'Migration can only be executed safely on \'mysql\'.');

$this->addSql("ALTER TABLE {$this->prefix}reports ADD is_scheduled TINYINT(1) NOT NULL, ADD schedule_unit VARCHAR(255) DEFAULT NULL, ADD schedule_day VARCHAR(255) DEFAULT NULL, ADD schedule_month_frequency VARCHAR(255) DEFAULT NULL");
Copy link
Member

Choose a reason for hiding this comment

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

@alanhartless didn't we agree that each migration should take care of only one column?

@Maxell92 Maxell92 force-pushed the feature/4794-report-schedule branch 2 times, most recently from 33bc09f to 9e1a369 Compare November 15, 2017 11:10
@javjim
Copy link

javjim commented Nov 16, 2017

Is this ready to be tested?

@alanhartless
Copy link
Contributor

alanhartless commented Nov 16, 2017 via email

@Maxell92 Maxell92 force-pushed the feature/4794-report-schedule branch 3 times, most recently from b9a1077 to 68fdadf Compare November 17, 2017 08:55
@Maxell92
Copy link
Contributor Author

Rebased - resolved conflicts.

app/bundles/ReportBundle/Tests/Fixures.php Outdated Show resolved Hide resolved
app/bundles/ReportBundle/Tests/Fixures.php Outdated Show resolved Hide resolved
app/bundles/ReportBundle/Tests/Fixures.php Outdated Show resolved Hide resolved
app/bundles/ReportBundle/Tests/Fixures.php Outdated Show resolved Hide resolved
app/bundles/ReportBundle/Tests/Model/CsvExporterTest.php Outdated Show resolved Hide resolved
app/bundles/ReportBundle/Tests/Model/CsvExporterTest.php Outdated Show resolved Hide resolved
app/bundles/ReportBundle/Tests/Model/CsvExporterTest.php Outdated Show resolved Hide resolved
app/bundles/ReportBundle/Tests/Model/CsvExporterTest.php Outdated Show resolved Hide resolved
alanhartless
alanhartless previously approved these changes Nov 21, 2017
@Maxell92
Copy link
Contributor Author

Rebased on actual staging.

@dongilbert
Copy link
Member

Don't forget to regen prod assets if not in dev. app/console m:a:g

@dongilbert
Copy link
Member

+1 This is working great. Thanks @Maxell92! My only "complaint" is that in dev mode we get translation errors in the console when sending the report, but they are suppressed in prod env.

@dongilbert dongilbert merged commit c71818f into mautic:staging Nov 22, 2017
@Maxell92 Maxell92 deleted the feature/4794-report-schedule branch November 22, 2017 10:04
@npracht
Copy link
Member

npracht commented Mar 29, 2018

Hello @Maxell92 there is nothing about it in the documentation and especially about the fact that the command app/console mautic:reports:scheduler must be enabled right ?

Thanks a lot for your help !

@Maxell92
Copy link
Contributor Author

Hello @npracht,

yes, you are right.

Thank you very much for your comment! I added documentation of this cron in this PR: mautic/documentation#268.

@npracht
Copy link
Member

npracht commented Mar 29, 2018

thank a lot for your help ! i very appreciate !!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature A new feature for inclusion in minor or major releases pending-test-confirmation PR's that require one test before they can be merged ready-to-test PR's that are ready to test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants