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

Implements App Enabled Emails #211

Conversation

discorick
Copy link
Member

Merges into discorick/canceled_and_expired_analytics
References https://github.com/huboard/corp/issues/48

The Scope of this this PR changed quite a bit during its life-time. From where we stand today this accomplishes the following:

  • Sets up Action::Mailer in the Saas Engine with some very basic layouts
  • Configures mailer previews and injects an interceptor to allow attachment previews (for things like images)
  • Adds a new user document in couchdb for every OAuth'd user for future opportunities to communicate with them
  • Adds mailcatcher and a puppet provisioner for it

@discorick
Copy link
Member Author

@rauhryan @dahlbyk , Wanted to have some discussion around how we want to structure our couchdb docs - going to need to start saving docs for individual users to coordinate sending out helpful emails.


def self.action(action)
@action = action
end
Copy link
Member

Choose a reason for hiding this comment

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

I remember having trouble subclassing class attributes

Why aren't you using the class_attr stuff here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not 100% following you. This is the same pattern we have been using with the Analytics Jobs, and the Issue Jobs for Child Class instance vars.

Copy link
Member

Choose a reason for hiding this comment

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

We use this pattern in the healthchecks

https://github.com/huboard/huboard-web/blob/master/lib/health_checking/health_checks/health_check.rb#L5-L40

I remember there was a reason for it

discorick and others added 21 commits March 8, 2016 11:16
…ing for child emails, adds a commented InlinePreviewInterceptor
@discorick discorick force-pushed the discorick+petedo/app_enabled_emails branch from 0ebbcd9 to 5ca810a Compare March 8, 2016 18:21
@discorick discorick force-pushed the discorick+petedo/app_enabled_emails branch from 5ca810a to ec376cf Compare March 8, 2016 18:24
@@ -1,4 +1,12 @@
Rails.application.configure do
#Mailer Previews
config.action_mailer.preview_path = Saas::Engine.root.join('test/mailers')
Copy link
Member

Choose a reason for hiding this comment

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

Help me understand what this is doing.

Copy link
Member Author

Choose a reason for hiding this comment

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

This line points the Action::Mailer previewer to look for mailer previews in the Saas::Engine. Otherwise when we attempt to use previews Rails is not able to find them.

@dahlbyk dahlbyk mentioned this pull request Mar 15, 2016
nokogiri (1.6.6.2)
mini_portile (~> 0.6.0)
rack (1.6.0)
rack (1.6.4)
rack-test (0.6.3)
rack (>= 1.0)
rails (4.2.0)
Copy link
Member

Choose a reason for hiding this comment

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

Alarm bells are ringing, thus #237.

Copy link
Member Author

Choose a reason for hiding this comment

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

Gotcha, thanks for opening that!

@dahlbyk dahlbyk force-pushed the discorick+petedo/app_enabled_emails branch from 8385b9b to f2795e2 Compare March 15, 2016 15:24
@dahlbyk
Copy link
Member

dahlbyk commented Mar 15, 2016

Pulled master in due to BaseLoginJob overlap in #232 - I told Git to use what's in master, but you might want to double-check: f2795e2#diff-409415fd755afb7ec1e211a3c1b9b519

@discorick
Copy link
Member Author

@dahlbyk f2795e2#diff-409415fd755afb7ec1e211a3c1b9b519 did go a little bit sideways. I am not sure the how go about fixing it.

@dahlbyk dahlbyk force-pushed the discorick+petedo/app_enabled_emails branch from f2795e2 to f79f194 Compare March 15, 2016 16:17
@dahlbyk dahlbyk force-pushed the discorick+petedo/app_enabled_emails branch from f79f194 to 9746411 Compare March 15, 2016 16:18
@dahlbyk
Copy link
Member

dahlbyk commented Mar 15, 2016

f2795e2#diff-409415fd755afb7ec1e211a3c1b9b519 did go a little bit sideways. I am not sure the how go about fixing it.

How does 9746411#diff-409415fd755afb7ec1e211a3c1b9b519 look?

@discorick discorick self-assigned this Mar 15, 2016
@dahlbyk dahlbyk deployed to huboard-rails-pr-211 March 16, 2016 00:20 Active
@dahlbyk dahlbyk closed this Mar 16, 2016
@dahlbyk
Copy link
Member

dahlbyk commented Mar 16, 2016

dahlbyk closed this 3 minutes ago

Oops. GitHub used to prevent deleting branches targeted by open PRs. 👿

firefox_2016-03-15_22-57-07

@discorick
Copy link
Member Author

Should I just reopen this against master at this point ?

@dahlbyk
Copy link
Member

dahlbyk commented Mar 16, 2016

Should I just reopen this against master at this point ?

Yup.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants