diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 312617c4e..5150f97ec 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -109,21 +109,4 @@ class ApplicationController < ActionController::Base end end - def track_activity(trackable) - if trackable.is_a? Comment - action = trackable.root? ? "debate_comment" : "comment_reply" - activity = current_user.activities.create! action: action, trackable: trackable - add_notifications_for activity - end - end - - def add_notifications_for(activity) - case activity.action - when "debate_comment" - author = activity.trackable.debate.author - when "comment_reply" - author = activity.trackable.parent.author - end - author.notifications.create!(activity: activity) unless activity.made_by? author - end end diff --git a/app/controllers/comments_controller.rb b/app/controllers/comments_controller.rb index e05be448a..c30ddbaf2 100644 --- a/app/controllers/comments_controller.rb +++ b/app/controllers/comments_controller.rb @@ -9,7 +9,7 @@ class CommentsController < ApplicationController def create if @comment.save CommentNotifier.new(comment: @comment).process - track_activity @comment + add_notification @comment else render :new end @@ -63,4 +63,13 @@ class CommentsController < ApplicationController ["1", true].include?(comment_params[:as_moderator]) && can?(:comment_as_moderator, @commentable) end + def add_notification(comment) + if comment.reply? + author = comment.parent.author + else + author = comment.commentable.author + end + author.notifications.create!(notifiable: comment) unless comment.made_by? author + end + end diff --git a/app/controllers/notifications_controller.rb b/app/controllers/notifications_controller.rb index 199893bd7..8ecb16104 100644 --- a/app/controllers/notifications_controller.rb +++ b/app/controllers/notifications_controller.rb @@ -3,8 +3,7 @@ class NotificationsController < ApplicationController load_and_authorize_resource class: "User" def index - @notifications = current_user.notifications.unread.recent.for_render - @notifications.each { |notification| notification.mark_as_read! } + @notifications = current_user.notifications.recent.for_render end end diff --git a/app/helpers/notifications_helper.rb b/app/helpers/notifications_helper.rb index d0e7e71a9..45ef6422f 100644 --- a/app/helpers/notifications_helper.rb +++ b/app/helpers/notifications_helper.rb @@ -1,16 +1,15 @@ module NotificationsHelper def notification_text_for(notification) - case notification.activity.action - when "debate_comment" - t("comments.notifications.commented_on_your_debate") - when "comment_reply" - t("comments.notifications.replied_to_your_comment") + if notification.notifiable.reply? + t("comments.notifications.replied_to_your_comment") + else + t("comments.notifications.commented_on_your_debate") end end def notifications_class_for(user) - user.notifications.unread.count > 0 ? "with_notifications" : "without_notifications" + user.notifications.count > 0 ? "with_notifications" : "without_notifications" end end diff --git a/app/models/comment.rb b/app/models/comment.rb index 3771af84a..44e16b729 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -90,6 +90,10 @@ class Comment < ActiveRecord::Base !root? end + def made_by?(user) + self.user == user + end + def call_after_commented self.commentable.try(:after_commented) end diff --git a/app/models/notification.rb b/app/models/notification.rb index 0f387f61f..63f44dedc 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -1,16 +1,19 @@ class Notification < ActiveRecord::Base belongs_to :user - belongs_to :activity + belongs_to :notifiable, polymorphic: true - scope :unread, -> { where(read: false) } + scope :unread, -> { all } scope :recent, -> { order(id: :desc) } - scope :for_render, -> { includes(activity: [:user, :trackable]) } + scope :for_render, -> { includes(notifiable: [:user]) } + + def username + notifiable.user.username + end def timestamp - activity.trackable.created_at + notifiable.created_at end def mark_as_read! - update_attribute :read, true end -end +end \ No newline at end of file diff --git a/app/views/notifications/index.html.erb b/app/views/notifications/index.html.erb index ab4351c50..0155f0521 100644 --- a/app/views/notifications/index.html.erb +++ b/app/views/notifications/index.html.erb @@ -3,8 +3,8 @@ <% @notifications.each do |notification| %>
  •  •  - <%= notification.activity.username %> <%= notification_text_for(notification) %> - <%= link_to notification.activity.trackable.debate.title, notification.activity.trackable.debate %> + <%= notification.username %> <%= notification_text_for(notification) %> + <%= link_to notification.notifiable.commentable.title, notification.notifiable.commentable %>
  • <% end %> diff --git a/db/migrate/20150928075646_create_activities.rb b/db/migrate/20150928075646_create_activities.rb deleted file mode 100644 index 89b0edcbc..000000000 --- a/db/migrate/20150928075646_create_activities.rb +++ /dev/null @@ -1,9 +0,0 @@ -class CreateActivities < ActiveRecord::Migration - def change - create_table :activities do |t| - t.belongs_to :user, index: true, foreign_key: true - t.string :action - t.belongs_to :trackable, polymorphic: true - end - end -end diff --git a/db/migrate/20160105170113_merge_activities_and_notifications.rb b/db/migrate/20160105170113_merge_activities_and_notifications.rb new file mode 100644 index 000000000..fac5b213d --- /dev/null +++ b/db/migrate/20160105170113_merge_activities_and_notifications.rb @@ -0,0 +1,10 @@ +class MergeActivitiesAndNotifications < ActiveRecord::Migration + def change + change_table :notifications do |t| + t.remove :read + t.remove :activity_id + t.references :notifiable, polymorphic: true + end + end + +end diff --git a/db/schema.rb b/db/schema.rb index cb1c5aa65..35aa4b68c 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20151215165824) do +ActiveRecord::Schema.define(version: 20160105170113) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -205,14 +205,11 @@ ActiveRecord::Schema.define(version: 20151215165824) do add_index "moderators", ["user_id"], name: "index_moderators_on_user_id", using: :btree create_table "notifications", force: :cascade do |t| - t.integer "user_id" - t.integer "activity_id" - t.boolean "read", default: false - t.datetime "created_at", null: false - t.datetime "updated_at", null: false + t.integer "user_id" + t.integer "notifiable_id" + t.string "notifiable_type" end - add_index "notifications", ["activity_id"], name: "index_notifications_on_activity_id", using: :btree add_index "notifications", ["user_id"], name: "index_notifications_on_user_id", using: :btree create_table "organizations", force: :cascade do |t| @@ -407,7 +404,6 @@ ActiveRecord::Schema.define(version: 20151215165824) do add_index "votes", ["votable_id", "votable_type", "vote_scope"], name: "index_votes_on_votable_id_and_votable_type_and_vote_scope", using: :btree add_index "votes", ["voter_id", "voter_type", "vote_scope"], name: "index_votes_on_voter_id_and_voter_type_and_vote_scope", using: :btree - add_foreign_key "activities", "users" add_foreign_key "administrators", "users" add_foreign_key "annotations", "legislations" add_foreign_key "annotations", "users" @@ -416,7 +412,6 @@ ActiveRecord::Schema.define(version: 20151215165824) do add_foreign_key "identities", "users" add_foreign_key "locks", "users" add_foreign_key "moderators", "users" - add_foreign_key "notifications", "activities" add_foreign_key "notifications", "users" add_foreign_key "organizations", "users" end diff --git a/spec/factories.rb b/spec/factories.rb index 8cdd3a3e4..d48aba942 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -291,9 +291,8 @@ FactoryGirl.define do end factory :notification do - association :user, factory: :user - association :activity, factory: :activity - read false + user + association :notifiable, factory: :comment end end diff --git a/spec/helpers/notifications_helper_spec.rb b/spec/helpers/notifications_helper_spec.rb index 120369bf4..dd5cd1b78 100644 --- a/spec/helpers/notifications_helper_spec.rb +++ b/spec/helpers/notifications_helper_spec.rb @@ -3,20 +3,21 @@ require 'rails_helper' describe NotificationsHelper do describe "#notification_text_for" do - let(:comment_activity) { create :activity, action: "debate_comment" } - let(:reply_activity) { create :activity, action: "comment_reply" } + let(:debate) { create :debate } + let(:debate_comment) { create :comment, commentable: debate } + let(:comment_reply) { create :comment, commentable: debate, parent: debate_comment } context "when action was comment on a debate" do - it "returns 'commented_on_your_debate' locale text" do - notification = create :notification, activity: comment_activity - expect(notification_text_for(notification)).to eq t("comments.notifications.commented_on_your_debate") + it "returns correct text when someone comments on your debate" do + notification = create :notification, notifiable: debate_comment + expect(notification_text_for(notification)).to eq "commented on your debate" end end context "when action was comment on a debate" do - it "returns 'replied_to_your_comment' locale text" do - notification = create :notification, activity: reply_activity - expect(notification_text_for(notification)).to eq t("comments.notifications.replied_to_your_comment") + it "returns correct text when someone replies to your comment" do + notification = create :notification, notifiable: comment_reply + expect(notification_text_for(notification)).to eq "replied to your comment on" end end end diff --git a/spec/models/comment_spec.rb b/spec/models/comment_spec.rb index 11a02400a..684de6438 100644 --- a/spec/models/comment_spec.rb +++ b/spec/models/comment_spec.rb @@ -128,4 +128,19 @@ describe Comment do expect(Comment.not_as_admin_or_moderator.first).to eq(comment1) end end + + describe "#made_by?" do + let(:author) { create :user } + let(:comment) { create :comment, user: author } + + it "returns true if comment was made by user" do + expect(comment.made_by?(author)).to be true + end + + it "returns false if comment was not made by user" do + not_author = create :user + expect(comment.made_by?(not_author)).to be false + end + end + end diff --git a/spec/models/notification_spec.rb b/spec/models/notification_spec.rb index 8d5e3dd97..d93dcf6f3 100644 --- a/spec/models/notification_spec.rb +++ b/spec/models/notification_spec.rb @@ -26,8 +26,8 @@ describe Notification do end describe "#for_render (scope)" do - it "returns notifications with including activity, user and trackable info" do - expect(Notification).to receive(:includes).with(activity: [:user, :trackable]).exactly(:once) + it "returns notifications including notifiable and user" do + expect(Notification).to receive(:includes).with(notifiable: [:user]).exactly(:once) Notification.for_render end end @@ -35,8 +35,7 @@ describe Notification do describe "#timestamp" do it "returns the timestamp of the trackable object" do comment = create :comment - activity = create :activity, trackable: comment - notification = create :notification, activity: activity + notification = create :notification, notifiable: comment expect(notification.timestamp).to eq comment.created_at end @@ -51,4 +50,13 @@ describe Notification do expect(notification.read).to be true end end + + describe "#username" do + it "returns the username of the activity's author" do + comment = create :comment + notification = create :notification, notifiable: comment + expect(notification.username).to eq comment.author.username + end + end + end