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

Force texting hours at the campaign level #36

Conversation

ben-pr-p
Copy link
Contributor

This PR:

  1. Forces texting hours to be set at the campaign level by requiring that campaign.override_texting_hours and campaign.texting_hours_enforced is true, but setting those values on the client to true, and removing the option to remove them. It also removes the database default of US/Eastern on the time zone and makes the time zone required in the yup client side schema.

To easily run this migration, do:

alter table campaign alter column timezone drop default;
  1. Fixes a bug on Postgres introduced by the MySQL branch due to differences between MySQL and Postgres returning(*) on insert.

  2. Introduces a slight optimization to send message by removing the join on organization, which is now possible because we're guaranteed that the campaign has texting hours enforced. Defaults are hardcoded to avoid modifying the rest of the sendMessage validation logic

@ben-pr-p ben-pr-p requested a review from bchrobot February 15, 2019 16:32
@@ -49,8 +49,6 @@ const Campaign = thinky.createModel('campaign', type.object().schema({
timezone: type
.string()
.required()
Copy link
Member

Choose a reason for hiding this comment

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

Drop required() here to match model definition.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah!

@bchrobot bchrobot merged commit 206b3ad into politics-rewired/deploy Feb 15, 2019
@bchrobot bchrobot deleted the politics-rewired/require-campaign-texting-hours branch February 15, 2019 19:42
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.

2 participants