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

Add unsubscribe option #5

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions app/controllers/subscribers_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,24 @@ def create
end
end

def unsubscribe
Copy link
Owner

Choose a reason for hiding this comment

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

Could you please extract this action and the cancel_subscription to another controller? This one is turning a little cumbersome.

Copy link
Author

Choose a reason for hiding this comment

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

Like an unsubscribe controller?

Copy link
Owner

Choose a reason for hiding this comment

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

Yup

subscriber = Subscriber.find_by(email: params[:unsubscribe][:email])

if subscriber
UnsubscribeMailSender.call(subscriber)
redirect_to new_post_path, notice: 'Check your email to find your unsubscribe url!'
else
redirect_to new_post_path, notice: 'Email not found!'
end
end

def cancel_subscription
Copy link
Owner

Choose a reason for hiding this comment

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

I think a better name here would be confirm unsubscribing or something like it. Just by the name is a little hard to differ from the one above (unsubscribe).

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, i felt the same thing about the naming

subscriber = Subscriber.find_by(delete_token: params[:delete_token])
subscriber.destroy

redirect_to new_post_path, notice: 'Subscription canceled!'
end

private

def subscriber_params
Expand Down
9 changes: 9 additions & 0 deletions app/mailers/unsubscribe_mailer.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
class UnsubscribeMailer < ApplicationMailer
default from: '[email protected]'

def unsubscribe_email(subscriber)
@subscriber = subscriber

mail(subject: "Your weekly unsubscribe url", to: @subscriber.email)
end
end
8 changes: 8 additions & 0 deletions app/services/unsubscribe_mail_sender.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
class UnsubscribeMailSender
include Service

def call(subscriber)
subscriber.update_attribute(:delete_token, SecureRandom.hex)
UnsubscribeMailer.unsubscribe_email(subscriber).deliver_now
end
end
9 changes: 9 additions & 0 deletions app/views/posts/new.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,13 @@
<%= link_to new_subscriber_path do %>
<h2>Not subscribed yet?</h2>
<% end %>
<hr>
<hr>
<hr>

<h3>Unsubscribe : (</h2>
Copy link
Owner

Choose a reason for hiding this comment

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

Unsubscribe :

could you please remove the blank space before the :?

<%= form_for :unsubscribe, url: unsubscribe_path, method: :delete do |f| %>
<%= f.text_field :email, placeholder: 'Your email' %>
<%= f.submit 'Unsubscribe' %>
<% end %>
</div>
20 changes: 20 additions & 0 deletions app/views/unsubscribe_mailer/unsubscribe_email.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<!DOCTYPE html>
<html>
<head>
<meta content='text/html; charset=UTF-8' http-equiv='Content-Type' />
</head>
<body style="font-family: open sans, helvetica neue, helvetica, arial; color: #414040">
<div style="text-align: center">
<div style="margin-bottom: 2em;
border: 1px solid #dedbdb;
width: 60%;
margin-left: 20%;
padding: .5em 1.5em;
text-align: left;">

Unsubscribe url:
<%= link_to 'Unsubscribe', cancel_subscription_url(@subscriber.delete_token) %>
</div>
</div>
</body>
</html>
1 change: 1 addition & 0 deletions app/views/unsubscribe_mailer/unsubscribe_email.text.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<%= cancel_subscription_url(@subscriber.delete_token) %>
1 change: 1 addition & 0 deletions config/environments/development.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
Bullet.rails_logger = true
end

config.action_mailer.default_url_options = { :host => "http://localhost:3000" }
config.action_mailer.delivery_method = :smtp
config.action_mailer.smtp_settings = { address: 'localhost',
port: 1025 }
Expand Down
2 changes: 2 additions & 0 deletions config/environments/production.rb
Original file line number Diff line number Diff line change
Expand Up @@ -92,4 +92,6 @@
:domain => 'heroku.com',
:enable_starttls_auto => true
}

config.action_mailer.default_url_options = { :host => "your-weekly-production.herokuapp.com" }
Copy link
Owner

Choose a reason for hiding this comment

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

Why do we need this? The app already works fine today.

Copy link
Author

Choose a reason for hiding this comment

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

It's for the link_to, it needs the host parameter to build the url

end
3 changes: 3 additions & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,8 @@
resources :posts, only: [:new, :create, :index]
resources :subscribers, only: [:new, :create, :index]

delete '/subscribers', to: 'subscribers#unsubscribe', as: :unsubscribe
get '/cancel_subscription/:delete_token', to: 'subscribers#cancel_subscription', as: :cancel_subscription

root to: 'posts#new'
end
5 changes: 5 additions & 0 deletions db/migrate/20161027185334_add_delete_token_to_subscriber.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddDeleteTokenToSubscriber < ActiveRecord::Migration[5.0]
def change
add_column :subscribers, :delete_token, :string, index: true
end
end
9 changes: 5 additions & 4 deletions db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema.define(version: 20160804191849) do
ActiveRecord::Schema.define(version: 20161027185334) do

# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
Expand All @@ -26,9 +26,10 @@
end

create_table "subscribers", force: :cascade do |t|
t.string "email", null: false
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
t.string "email", null: false
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
t.string "delete_token"
end

end