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

change alerts command. #1038

Merged
merged 26 commits into from
Oct 6, 2015
Merged

change alerts command. #1038

merged 26 commits into from
Oct 6, 2015

Conversation

stopfstedt
Copy link
Member

vagrant@ilios:/vagrant$ bin/console ilios:messaging:send-change-alerts --help
Usage:
  ilios:messaging:send-change-alerts [options]

Options:
      --dry-run            Print out alerts instead of emailing them. Useful for testing/debugging purposes.
  -h, --help               Display this help message
  -q, --quiet              Do not output any message
  -V, --version            Display this application version
      --ansi               Force ANSI output
      --no-ansi            Disable ANSI output
  -n, --no-interaction     Do not ask any interactive question
  -s, --shell              Launch the shell.
      --process-isolation  Launch commands from shell as a separate process.
  -e, --env=ENV            The Environment name. [default: "dev"]
      --no-debug           Switches off debug mode.
  -v|vv|vvv, --verbose     Increase the verbosity of messages: 1 for normal output, 2 for more verbose output and 3 for debug

Help:
 Sends out change alert message to configured email recipients.

fixes #992.

@stopfstedt
Copy link
Member Author

  • ported pretty much as-is, warts'n'all. I left some @todo comments in there for a further revisit.

$subject = $offering->getSession()->getCourse()->getExternalId() . ' - '
. $offering->getStartDate()->format('m/d/Y');

$school = $offering->getSession()->getCourse()->getSchool();
Copy link
Member

Choose a reason for hiding this comment

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

I feel like the template should be driven by the alert recipients as opposed to the primary school of the course. I'm probably wrong though.

Copy link
Member Author

Choose a reason for hiding this comment

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

you are correct. good catch. fixing it...

@stopfstedt
Copy link
Member Author

@jrjohnson can be reviewed again.

@jrjohnson
Copy link
Member

👍

@jrjohnson jrjohnson assigned dartajax and unassigned jrjohnson and dartajax Oct 6, 2015
@jrjohnson
Copy link
Member

@homu r+

@homu
Copy link
Contributor

homu commented Oct 6, 2015

📌 Commit 640f11a has been approved by jrjohnson

@homu
Copy link
Contributor

homu commented Oct 6, 2015

⌛ Testing commit 640f11a with merge 0649005...

homu added a commit that referenced this pull request Oct 6, 2015
change alerts command.

```
vagrant@ilios:/vagrant$ bin/console ilios:messaging:send-change-alerts --help
Usage:
  ilios:messaging:send-change-alerts [options]

Options:
      --dry-run            Print out alerts instead of emailing them. Useful for testing/debugging purposes.
  -h, --help               Display this help message
  -q, --quiet              Do not output any message
  -V, --version            Display this application version
      --ansi               Force ANSI output
      --no-ansi            Disable ANSI output
  -n, --no-interaction     Do not ask any interactive question
  -s, --shell              Launch the shell.
      --process-isolation  Launch commands from shell as a separate process.
  -e, --env=ENV            The Environment name. [default: "dev"]
      --no-debug           Switches off debug mode.
  -v|vv|vvv, --verbose     Increase the verbosity of messages: 1 for normal output, 2 for more verbose output and 3 for debug

Help:
 Sends out change alert message to configured email recipients.
```

fixes #992.
@homu
Copy link
Contributor

homu commented Oct 6, 2015

☀️ Test successful - status

@homu homu merged commit 640f11a into ilios:master Oct 6, 2015
@stopfstedt stopfstedt deleted the 992_change_alerts branch October 12, 2015 20:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add change alerts email console command
4 participants