Skip to content

Commit

Permalink
Subscriptions now have a token, which is used during the unsubscripti…
Browse files Browse the repository at this point in the history
…on process
  • Loading branch information
radar committed Jul 13, 2015
1 parent 7acc830 commit 1a5f628
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 1 deletion.
8 changes: 8 additions & 0 deletions app/controllers/forem/topics_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@ def subscribe

def unsubscribe
if find_topic
subscription = @topic.subscriptions.find_by(subscriber_id: forem_user.id, token: params[:token])
unsubscribe_unsuccessful and return unless subscription

@topic.unsubscribe_user(forem_user.id)
unsubscribe_successful
end
Expand Down Expand Up @@ -93,6 +96,11 @@ def unsubscribe_successful
redirect_to forum_topic_url(@topic.forum, @topic)
end

def unsubscribe_unsuccessful
flash.alert = t("forem.topic.unsubscription_failed")
redirect_to forum_topic_url(@topic.forum, @topic)
end

private
def find_forum
@forum = Forem::Forum.friendly.find(params[:forum_id])
Expand Down
8 changes: 8 additions & 0 deletions app/models/forem/subscription.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,20 @@ class Subscription < ActiveRecord::Base

validates :subscriber_id, :presence => true

before_create :set_token

def send_notification(post_id)
# If a user cannot be found, then no-op
# This will happen if the user record has been deleted.
if subscriber.present?
SubscriptionMailer.topic_reply(post_id, subscriber.id).deliver
end
end

private

def set_token
self.token = SecureRandom.hex(24)
end
end
end
1 change: 1 addition & 0 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ en:
not_updated: This topic could not be updated.
subscribed: You have subscribed to this topic.
unsubscribed: You have unsubscribed from this topic.
unsubscription_failed: Failed to unsubscribe you from this topic.
none: There are no topics in this forum currently.
links:
new: New topic
Expand Down
2 changes: 1 addition & 1 deletion config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
resources :posts, :except => :index
member do
post :subscribe
post :unsubscribe
get :unsubscribe
end
end
end
Expand Down
30 changes: 30 additions & 0 deletions spec/controllers/topics_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -68,4 +68,34 @@
flash.alert.should == "You must sign in first."
end
end

context "subscribing & unsubscribing" do
let!(:forum) { create(:forum) }
let!(:user) { create(:user) }
let!(:topic) { create(:approved_topic, forum: forum, forem_user: user) }
let(:subscription) { topic.subscriptions.first }

before do
sign_in(user)
controller.current_user.stub :can_read_topic? => true
end

it "can subscribe to a topic" do
post :subscribe, forum_id: forum.to_param, id: topic.to_param
expect(flash[:notice]).to eq(I18n.t("forem.topic.subscribed"))
expect(response).to redirect_to(forum_topic_path(forum, topic))
end

it "can unsubscribe with a valid token" do
get :unsubscribe, forum_id: forum.to_param, id: topic.to_param, token: subscription.token
expect(flash[:notice]).to eq(I18n.t("forem.topic.unsubscribed"))
expect(response).to redirect_to(forum_topic_path(forum, topic))
end

it "cannot unsubscribe without a token" do
get :unsubscribe, forum_id: forum.to_param, id: topic.to_param, token: "fake"
expect(flash[:alert]).to eq(I18n.t("forem.topic.unsubscription_failed"))
expect(response).to redirect_to(forum_topic_path(forum, topic))
end
end
end
7 changes: 7 additions & 0 deletions spec/models/subscription_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,13 @@
FactoryGirl.build(:subscription).should be_valid
end

context "set_token" do
it "sets a unique token for the subscription" do
subscription = Forem::Subscription.create!(subscriber_id: 1)
expect(subscription.token).to be_present
end
end

describe "topic subscriptions" do
before(:each) do
Forem::Topic.any_instance.stub(:set_first_post_user)
Expand Down

1 comment on commit 1a5f628

@pdcoutinho
Copy link

Choose a reason for hiding this comment

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

Yes, it looks good, any chance you can merge it?

Please sign in to comment.