Merge pull request #105 from tadyjp/feat/fix_notify

通知系のリファクタリング

app/assets/javascripts/application.js

@@ -21,3 +21,5 @@
 
 _.mixin(_.string.exports());
 
+// $('[data-toggle="tooltip"]').tooltip();
+// $('[data-toggle="popover"]').popover();

app/assets/stylesheets/application.css

@@ -26,3 +26,7 @@ a .text-link {
 a:visited .text-link {
   color: #609;
 }
+
+.popover {
+  max-width: 400px;
+}

app/controllers/posts_controller.rb

@@ -8,7 +8,6 @@ class PostsController < ApplicationController
   # GET /posts/1
   # GET /posts/1.json
   def show
-
     current_user.visit_post!(@post)
 
     @post.tags.each do |_tag|

app/decorators/user_decorator.rb

@@ -4,5 +4,4 @@ class UserDecorator < Draper::Decorator
   def draft_count
     model.posts.where(is_draft: true).count
   end
-
 end

app/models/comment.rb

@@ -11,11 +11,14 @@
 #
 
 class Comment < ActiveRecord::Base
+  ######################################################################
+  # Associations
+  ######################################################################
   belongs_to :author, class_name: 'User'
   belongs_to :post
 
   ######################################################################
-  # validations
+  # Validations
   ######################################################################
   validates :author_id, presence: true
   validates :post_id, presence: true
@@ -24,14 +27,23 @@ class Comment < ActiveRecord::Base
   ######################################################################
   # Callback
   ######################################################################
-  after_save :notify_author
+  after_save :set_watcher!
+  after_save :notify_watchers!
 
   ######################################################################
   # Instance method
   ######################################################################
   private
 
-  def notify_author
-    post.author.push_notification(post.decorate.show_path, "#{author.name}さんがあなたの投稿にコメントしました")
+  def notify_watchers!
+    post.watchers.each do |watcher|
+      next if watcher == author
+
+      watcher.push_notification(post.decorate.show_path, "#{author.name}さんが「#{post.title}」にコメントしました。")
+    end
+  end
+
+  def set_watcher!
+    author.watch!(post: post)
   end
 end

app/models/post.rb

@@ -16,6 +16,9 @@
 require 'date'
 
 class Post < ActiveRecord::Base
+  ######################################################################
+  # Associations
+  ######################################################################
   has_many :post_tags
   has_many :tags, through: :post_tags
   belongs_to :author, class_name: 'User'
@@ -34,7 +37,8 @@ class Post < ActiveRecord::Base
   ######################################################################
   # Callback
   ######################################################################
-  after_save :notify_watchers
+  after_save :set_watcher!
+  after_save :notify_watchers!
 
   ######################################################################
   # Named scope
@@ -107,11 +111,24 @@ class Post < ActiveRecord::Base
     footprints.select(:user_id).uniq.count
   end
 
+  # FIXME:
+  #   has_many :watchers, :through => :watches
+  #   正常に動作しないため動作しないため一時的にメソッドを作成
+  # def watchers
+  #   watches.map { |watch| watch.watcher }
+  # end
+
   private
 
-  def notify_watchers
+  def notify_watchers!
     watchers.each do |watcher|
+      next if watcher == author
+
       watcher.push_notification(decorate.show_path, "#{author.name}さんが「#{title}」を編集しました")
     end
   end
+
+  def set_watcher!
+    author.watch!(post: self)
+  end
 end

app/models/tag.rb

@@ -15,9 +15,6 @@ class Tag < ActiveRecord::Base
   has_many :post_tags
   has_many :posts, through: :post_tags
 
-  has_many :watches, :as => :watchable, :dependent => :destroy
-  has_many :watchers, :through => :watches
-
   # for tree structure
   has_ancestry
 

app/models/user.rb

@@ -40,12 +40,8 @@ class User < ActiveRecord::Base
   has_many :notifications
   has_many :footprints
 
-  has_many :watches, :as => :watchable, :dependent => :destroy
-  has_many :watchers, :through => :watches
-
   has_many :watchings, class_name: 'Watch', foreign_key: 'watcher_id'
   has_many :watching_posts, :through => :watchings, :source => :watchable, :source_type => "Post"
-  # has_many :watchings, :as => :resource
 
   ######################################################################
   # scope

app/views/partials/_app_header.html.slim

@@ -24,6 +24,11 @@ nav.navbar.navbar-default.navbar-fixed-top role="navigation"
 
       ul.nav.navbar-nav.navbar-right
         li
+          a#notifications data-container="body" data-toggle="popover" data-placement="bottom"
+            span.glyphicon.glyphicon-flag
+            - if current_user.notifications.unread.any?
+              span.badge = current_user.notifications.unread.count
+        li
           form
             a.btn.btn-primary.navbar-btn href=new_post_path
               | New Post  
@@ -46,21 +51,24 @@ nav.navbar.navbar-default.navbar-fixed-top role="navigation"
             li.divider
             li
               a href=destroy_user_session_path data-method="delete" rel="nofollow" SignOut
-        li.dropdown
-          a.dropdown-toggle data-toggle="dropdown"
-            | 通知
-            - if current_user.notifications.unread.any?
-              span.badge = current_user.notifications.unread.count
-            b.caret
-          ul.dropdown-menu
-            - if current_user.notifications.unread.any?
-              - current_user.notifications.unread.each do |notification|
-                li
-                  a href=notification_bridge_path(notification.id)
-                    = notification.body
-            - else
-              li
-                a 通知はありません
 
+script#notification-content type="text/template"
+  - if current_user.notifications.unread.any?
+    h4 通知一覧
+    .list-group
+    - current_user.notifications.unread.each do |notification|
+        a.list-group-item href=notification_bridge_path(notification.id)
+          spen.small
+            | [#{notification.created_at.strftime('%m/%d %H:%M')}] 
+            = notification.body
+  - else
+    h4 通知はありません
+
+- content_for :footer_js do
+  javascript:
+    $('#notifications').popover({
+      html: true,
+      content: $('#notification-content').html()
+    });
 
 

spec/controllers/concerns/rv/mailer_spec.rb

@@ -6,7 +6,7 @@ describe RV::Mailer do
 
   let(:klass) { DummyClass.new.extend(RV::Mailer) }
   let(:alice) { FactoryGirl.create(:alice) }
-  let(:post) { Post.create title: 'ruby rspec', body: 'This is first espec test: ruby' }
+  let(:post) { Post.create title: 'ruby rspec', body: 'This is first espec test: ruby', author: create(:author) }
 
   it 'valid' do
     expect { klass.compose_mail(post, user: alice, to: 'dummy@example.com') }.not_to raise_error

spec/factories/posts.rb

@@ -17,6 +17,7 @@
 FactoryGirl.define do
 
   factory :post do
+    association :author, factory: :author
     title 'sample title'
     body 'sample body'
     specified_date Date.new(2014, 4, 1)

spec/factories/users.rb

@@ -40,6 +40,14 @@ FactoryGirl.define do
     google_token_expires_at Time.now - 1.hour
   end
 
+  factory :author, class: User do
+    name 'Author'
+    email 'author@mail.com'
+    nickname 'author'
+    password Devise.friendly_token[0, 20]
+    google_token_expires_at Time.now - 30.hour
+  end
+
   factory :login_user_1, class: User do
     name 'Test User'
     email 'example@example.com'

spec/models/notification_spec.rb

@@ -15,5 +15,59 @@
 require 'rails_helper'
 
 describe Notification do
-  pending "add some examples to (or delete) #{__FILE__}"
+  describe 'Instance method' do
+
+    before :each do
+      @alice = create(:alice)
+      @bob = create(:bob)
+      @post = create(:post)
+    end
+
+    it "notifies on post edited" do
+      @bob.watch!(post: @post)
+      expect(@bob.watching?(post: @post)).to be_truthy
+      @post.reload
+      expect(@post.watchers).to include(@bob)
+      @post.update!(title: @post.title + ' [New!]')
+      expect(@bob.notifications.size).to eq(1)
+    end
+
+    it "not notifies on post edited by him" do
+      @bob.watch!(post: @post)
+      @post.reload
+      @post.update!(title: @post.title + ' [New!]', author: @bob)
+      expect(@bob.notifications.size).to eq(0)
+    end
+
+    it "notifies on post commented" do
+      @bob.watch!(post: @post)
+      expect(@bob.watching?(post: @post)).to be_truthy
+      @post.reload
+      expect(@post.watchers).to include(@bob)
+      @post.comments.create!(author: @alice, body: 'new comment')
+      expect(@bob.notifications.size).to eq(1)
+    end
+
+    it "not notifies on post commented by him" do
+      @bob.watch!(post: @post)
+      @post.reload
+      @post.comments.create!(author: @bob, body: 'new comment')
+      expect(@bob.notifications.size).to eq(0)
+    end
+
+    it "set watch on user create a new post" do
+      new_post = Post.create!(author: @bob, title: 'title', body: 'body')
+      expect(@bob.watching?(post: new_post)).to be_truthy
+    end
+
+    it "set watch on user edit a post" do
+      @post.update!(author: @bob, title: 'new title')
+      expect(@bob.watching?(post: @post)).to be_truthy
+    end
+
+    it "set watch on user comment a post" do
+      @post.comments.create!(author: @bob, body: 'new comment')
+      expect(@bob.watching?(post: @post)).to be_truthy
+    end
+  end
 end

spec/models/post_spec.rb

@@ -20,17 +20,13 @@ describe Post do
   describe 'Instance method' do
     before :each do
       @post = create(:post)
-      @alice = create(:alice)
-      @new_post = @post.generate_fork(@alice)
+      @bob = create(:bob)
+      @new_post = @post.generate_fork(@bob)
       @new_post.save
     end
 
     describe 'Fork' do
 
-      # subject {
-      #   @post.generate_fork(@alice).save
-      # }
-
       it 'duplicated' do
         expect(@new_post.id).not_to eq(@post.id)
       end
@@ -44,7 +40,7 @@ describe Post do
       end
 
       it 'valid user' do
-        expect(@new_post.author).to eq(@alice)
+        expect(@new_post.author).to eq(@bob)
       end
 
       it 'valid user' do
@@ -61,10 +57,11 @@ describe Post do
   describe 'scope :search' do
     before :each do
       @alice = create(:alice)
-      @post1 = Post.create id: 1001, title: 'ruby rspec', body: 'This is first espec test: ruby'
-      @post2 = Post.create id: 1002, title: 'php test', body: 'PHP is very easy', author_id: @alice.id
-      @post3 = Post.create id: 1003, title: 'java java...', body: 'Java is not ruby...', updated_at: Time.new(1989, 2, 25, 5, 30, 0)
-      @post4 = Post.create id: 1004, title: 'about ruby TDD', body: 'test is the best ....', is_draft: true
+      @author = create(:author)
+      @post1 = Post.create id: 1001, author: @alice, title: 'ruby rspec', body: 'This is first espec test: ruby'
+      @post2 = Post.create id: 1002, author: @alice, title: 'php test', body: 'PHP is very easy'
+      @post3 = Post.create id: 1003, author: @author, title: 'java java...', body: 'Java is not ruby...', updated_at: Time.new(1989, 2, 25, 5, 30, 0)
+      @post4 = Post.create id: 1004, author: @author, title: 'about ruby TDD', body: 'test is the best ....', is_draft: true
       @tag_java = Tag.create(name: 'java')
       @post3.tags << @tag_java
     end
@@ -85,7 +82,7 @@ describe Post do
     end
 
     it 'by @<author_name>' do
-      expect(Post.search('@Alice').size).to eq(1)
+      expect(Post.search('@Alice').size).to eq(2)
       expect(Post.search('@Alice')).to include(@post2)
     end
 
@@ -120,10 +117,11 @@ describe Post do
 
     it do
       @post.watchers << @bob
-
+      @post.reload
       expect(@bob.watching_posts.size).to eq(1)
       expect(@bob.notifications.size).to eq(0)
       @post.update!(title: @post.title + '+')
+      @bob.reload
       expect(@bob.notifications.size).to eq(1)
     end
   end

spec/models/tag_spec.rb

@@ -18,9 +18,10 @@ describe Tag do
     before :each do
       @tag_ruby = Tag.create(name: 'ruby')
       @tag_java = Tag.create(name: 'java')
-      @post1 = Post.create id: 1001, title: 'ruby rspec', body: 'hoge', tags: [@tag_ruby]
-      @post2 = Post.create id: 1002, title: 'ruby is better than java', body: 'hoge', tags: [@tag_ruby, @tag_java]
-      @post3 = Post.create id: 1003, title: 'java java...', body: 'hoge', tags: [@tag_java]
+      @author = create(:author)
+      @post1 = Post.create id: 1001, author: @author, title: 'ruby rspec', body: 'hoge', tags: [@tag_ruby]
+      @post2 = Post.create id: 1002, author: @author, title: 'ruby is better than java', body: 'hoge', tags: [@tag_ruby, @tag_java]
+      @post3 = Post.create id: 1003, author: @author, title: 'java java...', body: 'hoge', tags: [@tag_java]
     end
 
     it 'successfully moved' do

spec/models/user_spec.rb

@@ -29,8 +29,8 @@ describe User do
 
   describe 'Instance method' do
 
-    let(:alice) { create(:alice) }
     let(:bob) { create(:bob) }
+    let(:alice) { create(:alice) }
     let(:post) { create(:post) }
 
     describe '#google_oauth_token_expired?' do