From 71447a29e3bef3b6aae013da1a96b6bf6e8a3e4b Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Wed, 18 Jan 2012 23:48:31 +0600 Subject: [PATCH 01/35] [refs #114] add settings for commits notifications --- app/views/settings/notifiers/_form.html.haml | 14 +++++++++++++- config/locales/ru.yml | 19 +++++++++++-------- ...3141_add_settings_to_settings_notifiers.rb | 13 +++++++++++++ db/schema.rb | 17 ++++++++++------- 4 files changed, 47 insertions(+), 16 deletions(-) create mode 100644 db/migrate/20120118173141_add_settings_to_settings_notifiers.rb diff --git a/app/views/settings/notifiers/_form.html.haml b/app/views/settings/notifiers/_form.html.haml index 2631874bc..5cd3e4e3d 100644 --- a/app/views/settings/notifiers/_form.html.haml +++ b/app/views/settings/notifiers/_form.html.haml @@ -17,7 +17,19 @@ .group = f.label :issue_assign, t('activerecord.attributes.settings.notifier.issue_assign'), :class => :label = f.check_box :issue_assign, :class => 'notify_cbx' - + +.group + = f.label :new_comment_commit_owner, t('activerecord.attributes.settings.notifier.new_comment_commit_owner'), :class => :label + = f.check_box :new_comment_commit_owner, :class => 'notify_cbx' + +.group + = f.label :new_comment_commit_repo_owner, t('activerecord.attributes.settings.notifier.new_comment_commit_repo_owner'), :class => :label + = f.check_box :new_comment_commit_repo_owner, :class => 'notify_cbx' + +.group + = f.label :new_comment_commit_commentor, t('activerecord.attributes.settings.notifier.new_comment_commit_commentor'), :class => :label + = f.check_box :new_comment_commit_commentor, :class => 'notify_cbx' + .group.navform.wat-cf %button.button{:type => "submit"} = image_tag("web-app-theme/icons/tick.png", :alt => t("layout.save")) diff --git a/config/locales/ru.yml b/config/locales/ru.yml index 9086b8375..0bdcc1d53 100644 --- a/config/locales/ru.yml +++ b/config/locales/ru.yml @@ -1,12 +1,12 @@ ru: will_paginate: previous_label: ‹ Предыдущая - next_label: Следующая › + next_label: Следующая › page_gap: ... - + datatables: previous_label: ‹ Пред. - next_label: След. › + next_label: След. › first_label: « Первая last_label: Последняя » empty_label: Нет доступных данных @@ -50,7 +50,7 @@ ru: title: Статистика закачек пакетов message: Обновляется автоматически каждые 5 минут refresh_btn: Обновить - + auto_build_lists: header: Проекты с автоматической сборкой message: Все проекты собираются под пользовательский репозиторий и архитектуру i586 @@ -182,7 +182,7 @@ ru: back_to_the_list: ⇐ К списку репозиториев confirm_delete: Вы уверены, что хотите удалить этот репозиторий? current_repository_header: Текущий репозиторий - + personal_repositories: settings_header: Настройки change_visibility_from_hidden: Сменить статус на "Открытый" @@ -324,13 +324,13 @@ ru: publish_success: 'Сборка поставлена в очередь на публикацию.' publish_fail: 'При публикации сборки произошла ошибка!' container_published: 'Контейнер размещен в репозитории' - + build_server_status: header: Статус сборочного сервера client_count: Число клиентов count_new_task: Число заданий в очереди count_build_task: Число выполняемых заданий - + items: statuses: build_started: собирается @@ -361,7 +361,7 @@ ru: settings: saved: Настройки успешно сохранены save_error: При обновлении настроек произошла ошибка - + subscribe: saved: Вы подписаны на оповещения для этой задачи @@ -499,6 +499,9 @@ ru: new_comment_reply: Оповещать о новом ответе на мой комментарий new_issue: Оповещать о новых задачах в моих проектах issue_assign: Оповещать, когда на меня выставляют задачу + new_comment_commit_owner: Оповещать о комментариях к моему коммиту + new_comment_commit_repo_owner: Оповещать о комментариях к коммитам в моем репозитории + new_comment_commit_commentor: Оповещать о комментариях к коммиту после моего auto_build_list: project_id: Проект diff --git a/db/migrate/20120118173141_add_settings_to_settings_notifiers.rb b/db/migrate/20120118173141_add_settings_to_settings_notifiers.rb new file mode 100644 index 000000000..89877b871 --- /dev/null +++ b/db/migrate/20120118173141_add_settings_to_settings_notifiers.rb @@ -0,0 +1,13 @@ +class AddSettingsToSettingsNotifiers < ActiveRecord::Migration + def self.up + add_column :settings_notifiers, :new_comment_commit_owner, :boolean, :default => true + add_column :settings_notifiers, :new_comment_commit_repo_owner, :boolean, :default => true + add_column :settings_notifiers, :new_comment_commit_commentor, :boolean, :default => true + end + + def self.down + remove_column :settings_notifiers, :new_comment_commit_owner + remove_column :settings_notifiers, :new_comment_commit_repo_owner + remove_column :settings_notifiers, :new_comment_commit_commentor + end +end diff --git a/db/schema.rb b/db/schema.rb index 825f237ba..86e064a69 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended to check this file into your version control system. -ActiveRecord::Schema.define(:version => 20120117110723) do +ActiveRecord::Schema.define(:version => 20120118173141) do create_table "arches", :force => true do |t| t.string "name", :null => false @@ -290,14 +290,17 @@ ActiveRecord::Schema.define(:version => 20120117110723) do add_index "rpms", ["project_id"], :name => "index_rpms_on_project_id" create_table "settings_notifiers", :force => true do |t| - t.integer "user_id", :null => false - t.boolean "can_notify", :default => true - t.boolean "new_comment", :default => true - t.boolean "new_comment_reply", :default => true - t.boolean "new_issue", :default => true - t.boolean "issue_assign", :default => true + t.integer "user_id", :null => false + t.boolean "can_notify", :default => true + t.boolean "new_comment", :default => true + t.boolean "new_comment_reply", :default => true + t.boolean "new_issue", :default => true + t.boolean "issue_assign", :default => true t.datetime "created_at" t.datetime "updated_at" + t.boolean "new_comment_commit_owner", :default => true + t.boolean "new_comment_commit_repo_owner", :default => true + t.boolean "new_comment_commit_commentor", :default => true end create_table "subscribes", :force => true do |t| From dba11f4662a3ae6d12f34dd814bfd08dabc31efb Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Thu, 19 Jan 2012 01:12:52 +0600 Subject: [PATCH 02/35] [refs #114] FIXME subscribers --- app/controllers/settings/notifiers_controller.rb | 4 ++-- app/models/comment.rb | 2 +- app/models/project.rb | 1 + spec/models/comment_for_commit_spec.rb | 10 +++++++--- 4 files changed, 11 insertions(+), 6 deletions(-) diff --git a/app/controllers/settings/notifiers_controller.rb b/app/controllers/settings/notifiers_controller.rb index 03df52237..d7254c7fd 100644 --- a/app/controllers/settings/notifiers_controller.rb +++ b/app/controllers/settings/notifiers_controller.rb @@ -12,10 +12,10 @@ class Settings::NotifiersController < ApplicationController def update if @notifier.update_attributes(params[:settings_notifier]) flash[:notice] = I18n.t("flash.settings.saved") - redirect_to [@user, @notifier] + redirect_to user_settings_notifier_path(@user) else flash[:notice] = I18n.t("flash.settings.save_error") - redirect_to [@user, @notifier] + redirect_to user_settings_notifier_path(@user) end end diff --git a/app/models/comment.rb b/app/models/comment.rb index 8ba9c05d0..f1efea329 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -7,7 +7,7 @@ class Comment < ActiveRecord::Base # FIXME after_create :subscribe_on_reply, :unless => "commentable_type == 'Grit::Commit'" - after_create :deliver_new_comment_notification, :unless => "commentable_type == 'Grit::Commit'" + after_create :deliver_new_comment_notification#, :unless => "commentable_type == 'Grit::Commit'" protected diff --git a/app/models/project.rb b/app/models/project.rb index 5763fc071..1b1478aeb 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -14,6 +14,7 @@ class Project < ActiveRecord::Base has_many :relations, :as => :target, :dependent => :destroy has_many :collaborators, :through => :relations, :source => :object, :source_type => 'User' has_many :groups, :through => :relations, :source => :object, :source_type => 'Group' + has_many :commit_comments_subscribers, :as => :subscribeable, :class_name => 'Subscribe' validates :name, :uniqueness => {:scope => [:owner_id, :owner_type]}, :presence => true, :format => { :with => /^[a-zA-Z0-9_\-\+\.]+$/ } validates :owner, :presence => true diff --git a/spec/models/comment_for_commit_spec.rb b/spec/models/comment_for_commit_spec.rb index 938c4db7e..e7c6941b4 100644 --- a/spec/models/comment_for_commit_spec.rb +++ b/spec/models/comment_for_commit_spec.rb @@ -32,8 +32,12 @@ describe Comment do @ability.should be_able_to(:create, Comment.new(@create_params)) end - pending "sends an e-mail" do - ActionMailer::Base.deliveries.last.to.include?(@stranger.email).should == true + it 'should send an e-mail' do + ActionMailer::Base.deliveries = [] + comment = Comment.new(:user => @stranger, :body => 'hello!', :project => @project, + :commentable_type => @commit.class.name, :commentable_id => @commit.id) + comment.save + ActionMailer::Base.deliveries.last.to.include?(@user.email).should == true end it 'should update comment' do @@ -132,4 +136,4 @@ describe Comment do @ability.should_not be_able_to(:destroy, @comment) end end -end +end \ No newline at end of file From 800b02f5449e400af150c85cbf018675f130c14c Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Fri, 20 Jan 2012 00:20:03 +0600 Subject: [PATCH 03/35] [refs #18] Add project subsribe, tests --- .../project_subscribes_controller.rb | 24 +++ app/models/comment.rb | 12 +- app/models/project.rb | 7 +- app/models/subscribe.rb | 22 +++ app/views/projects/show.html.haml | 9 +- .../user_mailer/new_comment_notification.haml | 4 +- config/routes.rb | 1 + spec/models/comment_for_commit_spec.rb | 142 ++++++++++++++++-- 8 files changed, 204 insertions(+), 17 deletions(-) create mode 100644 app/controllers/project_subscribes_controller.rb diff --git a/app/controllers/project_subscribes_controller.rb b/app/controllers/project_subscribes_controller.rb new file mode 100644 index 000000000..665a47086 --- /dev/null +++ b/app/controllers/project_subscribes_controller.rb @@ -0,0 +1,24 @@ +class ProjectSubscribesController < ApplicationController + before_filter :authenticate_user! + + load_and_authorize_resource :project + load_resource :subscribe + #load_and_authorize_resource :subscribe, :find_by => :user_id + + def create + @subscribe = @project.commit_comments_subscribes.build(:user_id => current_user.id) + if @subscribe.save + flash[:notice] = I18n.t("flash.subscribe.saved") + redirect_to @project + else + flash[:error] = I18n.t("flash.subscribe.saved_error") + redirect_to @project + end + end + + def destroy + @project.commit_comments_subscribes.where(:user_id => current_user.id).first.destroy # FIXME + flash[:notice] = t("flash.subscribe.destroyed") + redirect_to @project + end +end diff --git a/app/models/comment.rb b/app/models/comment.rb index f1efea329..7e3a8749d 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -6,20 +6,28 @@ class Comment < ActiveRecord::Base validates :body, :user_id, :commentable_id, :commentable_type, :presence => true # FIXME + #after_create :subscribe_users, :if => "commentable_type == 'Grit::Commit'" after_create :subscribe_on_reply, :unless => "commentable_type == 'Grit::Commit'" after_create :deliver_new_comment_notification#, :unless => "commentable_type == 'Grit::Commit'" protected + #def subscribe_users + # Subscribe.subscribe_users(self.project) + #end + def deliver_new_comment_notification - subscribes = self.commentable.subscribes + subscribes = self.commentable.subscribes if self.commentable_type == 'Issue' + subscribes = self.project.commit_comments_subscribes(true) if self.commentable_type == 'Grit::Commit' subscribes.each do |subscribe| - if self.user_id != subscribe.user_id && User.find(subscribe.user).notifier.new_comment_reply && User.find(subscribe.user).notifier.can_notify + if self.commentable_type == 'Issue' && self.user_id != subscribe.user_id && User.find(subscribe.user).notifier.new_comment_reply && User.find(subscribe.user).notifier.can_notify if self.commentable.comments.exists?(:user_id => subscribe.user.id) UserMailer.delay.new_comment_reply_notification(self, subscribe.user) else UserMailer.delay.new_comment_notification(self, subscribe.user) end + elsif self.commentable_type == 'Grit::Commit' && self.user_id != subscribe.user_id && User.find(subscribe.user).notifier.new_comment_commit_repo_owner && User.find(subscribe.user).notifier.can_notify + UserMailer.delay.new_comment_notification(self, subscribe.user) end end end diff --git a/app/models/project.rb b/app/models/project.rb index 1b1478aeb..bee1b81f6 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -14,7 +14,7 @@ class Project < ActiveRecord::Base has_many :relations, :as => :target, :dependent => :destroy has_many :collaborators, :through => :relations, :source => :object, :source_type => 'User' has_many :groups, :through => :relations, :source => :object, :source_type => 'Group' - has_many :commit_comments_subscribers, :as => :subscribeable, :class_name => 'Subscribe' + has_many :commit_comments_subscribes, :as => :subscribeable, :class_name => 'Subscribe' validates :name, :uniqueness => {:scope => [:owner_id, :owner_type]}, :presence => true, :format => { :with => /^[a-zA-Z0-9_\-\+\.]+$/ } validates :owner, :presence => true @@ -31,6 +31,7 @@ class Project < ActiveRecord::Base after_create :attach_to_personal_repository after_create :create_git_repo + after_create :subscribe_users after_destroy :destroy_git_repo # after_rollback lambda { destroy_git_repo rescue true if new_record? } @@ -159,4 +160,8 @@ class Project < ActiveRecord::Base def destroy_git_repo FileUtils.rm_rf path end + + def subscribe_users + self.commit_comments_subscribes.create(:user_id => self.owner.id) + end end diff --git a/app/models/subscribe.rb b/app/models/subscribe.rb index 0c3a52698..caffd9459 100644 --- a/app/models/subscribe.rb +++ b/app/models/subscribe.rb @@ -1,4 +1,26 @@ class Subscribe < ActiveRecord::Base belongs_to :subscribeable, :polymorphic => true belongs_to :user + + def self.subscribe_users(project) + recipients = Subscribe.collect_recipient_ids(project) + recipients.each do |recipient_id| + ss = project.commit_comments_subscribes.build(:user_id => recipient_id) + ss.save! + end + end + + def self.collect_recipient_ids(project) + recipients = project.relations.by_role('admin').where(:object_type => 'User').map { |rel| rel.read_attribute(:object_id) } +# recipients = recipients | [commentable.user_id] if commentable.user_id +# recipients = recipients | [commentable.project.owner_id] if commentable.project.owner_type == 'User' + + # filter by notification settings + recipients = recipients.select do |recipient| + User.find(recipient).notifier.new_issue && User.find(recipient).notifier.can_notify + end + + recipients + end + end diff --git a/app/views/projects/show.html.haml b/app/views/projects/show.html.haml index e302150d9..689a5272f 100644 --- a/app/views/projects/show.html.haml +++ b/app/views/projects/show.html.haml @@ -30,7 +30,14 @@ = t("activerecord.attributes.project.repository") \: = git_repo_url @project.git_repo_name - + %p + %b + = t('layout.issues.subscribe') + \: + - if @project.commit_comments_subscribes.exists? :user_id => current_user.id + = link_to t('layout.issues.unsubscribe_btn'), project_subscribe_path(@project, current_user.id), :method => :delete + - else + = link_to t('layout.issues.subscribe_btn'), project_subscribes_path(@project), :method => :post .wat-cf = link_to image_tag("web-app-theme/icons/application_edit.png", :alt => t("layout.edit")) + " " + t("layout.edit"), edit_project_path(@project), :class => "button" if can? :update, @project = link_to image_tag("web-app-theme/icons/cross.png", :alt => t("layout.delete")) + " " + t("layout.delete"), project_path(@project), :method => "delete", :class => "button", :confirm => t("layout.projects.confirm_delete") if can? :destroy, @project diff --git a/app/views/user_mailer/new_comment_notification.haml b/app/views/user_mailer/new_comment_notification.haml index 315a56b88..15b4e6a6f 100644 --- a/app/views/user_mailer/new_comment_notification.haml +++ b/app/views/user_mailer/new_comment_notification.haml @@ -1,8 +1,6 @@ %p== Здравствуйте, #{@user.name}. - -%p К задаче #{ link_to @comment.commentable.title, [@comment.commentable.project, @comment.commentable] } был добавлен новый комментарий. - +%p К коммиту был добавлен новый комментарий. %p "#{ @comment.body }" diff --git a/config/routes.rb b/config/routes.rb index bda7c96eb..d36e2b7e2 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -88,6 +88,7 @@ Rosa::Application.routes.draw do end resource :repo, :controller => "git/repositories", :only => [:show] resources :build_lists, :only => [:index, :new, :create] + resources :subscribes, :only => [:create, :destroy], :controller => 'project_subscribes' resources :collaborators, :only => [:index, :edit, :update, :add] do collection do diff --git a/spec/models/comment_for_commit_spec.rb b/spec/models/comment_for_commit_spec.rb index e7c6941b4..fe168ef4d 100644 --- a/spec/models/comment_for_commit_spec.rb +++ b/spec/models/comment_for_commit_spec.rb @@ -4,7 +4,7 @@ require "cancan/matchers" def set_comments_data_for_commit @ability = Ability.new(@user) - @project = Factory(:project) + @project = Factory(:project, :owner => @user) %x(cp -Rf #{Rails.root}/spec/tests.git/* #{@project.git_repository.path}) # maybe FIXME ? @commit = @project.git_repository.commits.first @@ -32,14 +32,6 @@ describe Comment do @ability.should be_able_to(:create, Comment.new(@create_params)) end - it 'should send an e-mail' do - ActionMailer::Base.deliveries = [] - comment = Comment.new(:user => @stranger, :body => 'hello!', :project => @project, - :commentable_type => @commit.class.name, :commentable_id => @commit.id) - comment.save - ActionMailer::Base.deliveries.last.to.include?(@user.email).should == true - end - it 'should update comment' do @ability.should be_able_to(:update, @comment) end @@ -82,6 +74,52 @@ describe Comment do it 'should not destroy comment' do @ability.should_not be_able_to(:destroy, @comment) end + + context 'for default enabled settings' do + it 'should send an e-mail by default settings' do + ActionMailer::Base.deliveries = [] + comment = Comment.new(:user => @stranger, :body => 'hello!', :project => @project, + :commentable_type => @commit.class.name, :commentable_id => @commit.id) + comment.save + puts "deliveries is #{ActionMailer::Base.deliveries.inspect}" + ActionMailer::Base.deliveries.count.should == 1 + ActionMailer::Base.deliveries.last.to.include?(@user.email).should == true + end + end + + context 'for disabled notify setting in project' do + it 'should not send an e-mail' do + ActionMailer::Base.deliveries = [] + @project.commit_comments_subscribes.where(:user_id => @user).first.destroy # FIXME + comment = Comment.new(:user => @stranger, :body => 'hello!', :project => @project, + :commentable_type => @commit.class.name, :commentable_id => @commit.id) + comment.save + ActionMailer::Base.deliveries.count.should == 0 # cache project.commit_comments_subscribes ... + end + end + + context 'for disabled notify setting' do + it 'should not send an e-mail' do + ActionMailer::Base.deliveries = [] + @user.notifier.update_attribute :new_comment_commit_repo_owner, false + comment = Comment.new(:user => @stranger, :body => 'hello!', :project => @project, + :commentable_type => @commit.class.name, :commentable_id => @commit.id) + comment.save + ActionMailer::Base.deliveries.count.should == 0 + end + end + + context 'for disabled global notify setting' do + it 'should not send an e-mail' do + ActionMailer::Base.deliveries = [] + @user.notifier.update_attribute :can_notify, false + comment = Comment.new(:user => @stranger, :body => 'hello!', :project => @project, + :commentable_type => @commit.class.name, :commentable_id => @commit.id) + comment.save + ActionMailer::Base.deliveries.count.should == 0 + end + end + end context 'for project owner user' do @@ -110,6 +148,53 @@ describe Comment do it 'should not destroy comment' do @ability.should_not be_able_to(:destroy, @comment) end + + context 'for default enabled settings' do + it 'should send an e-mail by default settings' do + ActionMailer::Base.deliveries = [] + comment = Comment.new(:user => @stranger, :body => 'hello!', :project => @project, + :commentable_type => @commit.class.name, :commentable_id => @commit.id) + comment.save + puts "owner email is #{@project.owner}" + puts "deliveries is #{ActionMailer::Base.deliveries.last.to}" + ActionMailer::Base.deliveries.count.should == 1 + ActionMailer::Base.deliveries.last.to.include?(@project.owner.email).should == true + end + end + + context 'for disabled notify setting in project' do + it 'should not send an e-mail' do + ActionMailer::Base.deliveries = [] + @project.commit_comments_subscribes.where(:user_id => @project.owner).first.destroy # FIXME + comment = Comment.new(:user => @stranger, :body => 'hello!', :project => @project, + :commentable_type => @commit.class.name, :commentable_id => @commit.id) + comment.save + ActionMailer::Base.deliveries.count.should == 0 # cache project.commit_comments_subscribes ... + end + end + + context 'for disabled notify setting' do + it 'should not send an e-mail' do + ActionMailer::Base.deliveries = [] + @project.owner.notifier.update_attribute :new_comment_commit_repo_owner, false + comment = Comment.new(:user => @stranger, :body => 'hello!', :project => @project, + :commentable_type => @commit.class.name, :commentable_id => @commit.id) + comment.save + ActionMailer::Base.deliveries.count.should == 0 + end + end + + context 'for disabled global notify setting' do + it 'should not send an e-mail' do + ActionMailer::Base.deliveries = [] + @project.owner.notifier.update_attribute :can_notify, false + comment = Comment.new(:user => @stranger, :body => 'hello!', :project => @project, + :commentable_type => @commit.class.name, :commentable_id => @commit.id) + comment.save + ActionMailer::Base.deliveries.count.should == 0 + end + end + end context 'for simple user' do @@ -135,5 +220,42 @@ describe Comment do it 'should not destroy comment' do @ability.should_not be_able_to(:destroy, @comment) end + + context 'for default enabled settings' do + it 'should not send an e-mail' do + ActionMailer::Base.deliveries = [] + comment = Comment.new(:user => @stranger, :body => 'hello!', :project => @project, + :commentable_type => @commit.class.name, :commentable_id => @commit.id) + comment.save + puts "owner email is #{@project.owner}" + puts "deliveries is #{ActionMailer::Base.deliveries.last.to}" + ActionMailer::Base.deliveries.count.should == 1 + ActionMailer::Base.deliveries.last.to.include?(@stranger.email).should == false + end + end + + context 'for subscribe in project' do + it 'should send an e-mail' do + ActionMailer::Base.deliveries = [] + @project.owner.notifier.update_attribute :can_notify, false + @project.commit_comments_subscribes.create(:user_id => @stranger.id) + comment = Comment.new(:user => @project.owner, :body => 'hello!', :project => @project, + :commentable_type => @commit.class.name, :commentable_id => @commit.id) + comment.save + ActionMailer::Base.deliveries.count.should == 1 + ActionMailer::Base.deliveries.last.to.include?(@stranger.email).should == true + end + + it 'should send an e-mail for own comment' do + ActionMailer::Base.deliveries = [] + @project.owner.notifier.update_attribute :can_notify, false + @project.commit_comments_subscribes.create(:user_id => @stranger.id) + comment = Comment.new(:user => @owner, :body => 'hello!', :project => @project, + :commentable_type => @commit.class.name, :commentable_id => @commit.id) + comment.save + ActionMailer::Base.deliveries.count.should == 0 + end + end + end -end \ No newline at end of file +end From 876351edb5116c12b3d24ae1f9b64a9257c23f8b Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Fri, 20 Jan 2012 12:24:55 +0600 Subject: [PATCH 04/35] [refs #114] Fixed tests --- spec/models/comment_for_commit_spec.rb | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/spec/models/comment_for_commit_spec.rb b/spec/models/comment_for_commit_spec.rb index fe168ef4d..4a6b71a57 100644 --- a/spec/models/comment_for_commit_spec.rb +++ b/spec/models/comment_for_commit_spec.rb @@ -81,7 +81,6 @@ describe Comment do comment = Comment.new(:user => @stranger, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) comment.save - puts "deliveries is #{ActionMailer::Base.deliveries.inspect}" ActionMailer::Base.deliveries.count.should == 1 ActionMailer::Base.deliveries.last.to.include?(@user.email).should == true end @@ -130,7 +129,7 @@ describe Comment do set_comments_data_for_commit @project.update_attribute(:owner, @user) - @project.relations.create!(:object_type => 'User', :object_id => @user.id, :role => 'admin') + #@project.relations.create!(:object_type => 'User', :object_id => @user.id, :role => 'admin') end it 'should create comment' do @@ -155,8 +154,6 @@ describe Comment do comment = Comment.new(:user => @stranger, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) comment.save - puts "owner email is #{@project.owner}" - puts "deliveries is #{ActionMailer::Base.deliveries.last.to}" ActionMailer::Base.deliveries.count.should == 1 ActionMailer::Base.deliveries.last.to.include?(@project.owner.email).should == true end @@ -200,9 +197,14 @@ describe Comment do context 'for simple user' do before(:each) do @user = Factory(:user) + @simple = Factory(:user) @stranger = Factory(:user) - set_comments_data_for_commit + @create_params = {:commentable_type => @commit.class.name, :commentable_id => @commit.id, + :user => @simple, :project => @project} + @comment = Factory(:comment, :user => @simple) + @comment.update_attributes(:commentable_type => @commit.class.name, :commentable_id => @commit.id) + @ability = Ability.new(@simple) end it 'should create comment' do @@ -227,8 +229,6 @@ describe Comment do comment = Comment.new(:user => @stranger, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) comment.save - puts "owner email is #{@project.owner}" - puts "deliveries is #{ActionMailer::Base.deliveries.last.to}" ActionMailer::Base.deliveries.count.should == 1 ActionMailer::Base.deliveries.last.to.include?(@stranger.email).should == false end @@ -246,7 +246,7 @@ describe Comment do ActionMailer::Base.deliveries.last.to.include?(@stranger.email).should == true end - it 'should send an e-mail for own comment' do + it 'should not send an e-mail for own comment' do ActionMailer::Base.deliveries = [] @project.owner.notifier.update_attribute :can_notify, false @project.commit_comments_subscribes.create(:user_id => @stranger.id) From 371abac031a2875c6b48e1755f367d1e0c970440 Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Fri, 20 Jan 2012 21:17:05 +0600 Subject: [PATCH 05/35] [refs #114] fix controller tests --- app/controllers/comments_controller.rb | 7 +++-- app/mailers/user_mailer.rb | 6 ++-- app/models/comment.rb | 30 ++++++++++++------- app/models/project.rb | 2 +- .../new_commit_comment_notification.haml | 9 ++++++ .../comments_controller_for_commit_spec.rb | 5 ++-- spec/factories/comments.rb | 8 +++++ spec/models/comment_for_commit_spec.rb | 25 ++++++++++++++++ 8 files changed, 73 insertions(+), 19 deletions(-) create mode 100644 app/views/user_mailer/new_commit_comment_notification.haml diff --git a/app/controllers/comments_controller.rb b/app/controllers/comments_controller.rb index 6f699a602..6f471f08f 100644 --- a/app/controllers/comments_controller.rb +++ b/app/controllers/comments_controller.rb @@ -13,7 +13,7 @@ class CommentsController < ApplicationController def create @comment = @commentable.comments.build(params[:comment]) if @commentable.class == Issue - @comment = Comment.new(params[:comment].merge(:commentable_id => @commentable.id, :commentable_type => @commentable.class.name)) if @commentable.class == Grit::Commit + @comment = Comment.new(params[:comment].merge(:commentable_id => @commentable.id, :commentable_type => @commentable.class.name, :project => @project)) if @commentable.class == Grit::Commit @comment.user = current_user if @comment.save flash[:notice] = I18n.t("flash.comment.saved") @@ -74,7 +74,10 @@ class CommentsController < ApplicationController def find_comment @comment = Comment.find(params[:id]) - @comment.project = @project if @comment.commentable_type == 'Grit::Commit' + if @comment.commentable_type == 'Grit::Commit' + @comment.project = @project + @comment.helper + end end def find_project diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb index 39ceffa17..5680dc408 100644 --- a/app/mailers/user_mailer.rb +++ b/app/mailers/user_mailer.rb @@ -21,10 +21,8 @@ class UserMailer < ActionMailer::Base def new_comment_reply_notification(comment, user) @user = user @comment = comment - mail(:to => user.email, :subject => I18n.t("notifications.subjects.new_comment_reply_notification")) do |format| - format.html - end - end + template = 'new_commit_comment_reply_notification' if @comment.commentable_type == 'Grit::Commit' + mail(:to => user.email, :subject => I18n.t("notifications.subjects.new_comment_reply_notification"), :template_name => template) end def new_issue_notification(issue, user) @user = user diff --git a/app/models/comment.rb b/app/models/comment.rb index 7e3a8749d..9e4dc8f5d 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -5,19 +5,19 @@ class Comment < ActiveRecord::Base validates :body, :user_id, :commentable_id, :commentable_type, :presence => true - # FIXME - #after_create :subscribe_users, :if => "commentable_type == 'Grit::Commit'" - after_create :subscribe_on_reply, :unless => "commentable_type == 'Grit::Commit'" - after_create :deliver_new_comment_notification#, :unless => "commentable_type == 'Grit::Commit'" + after_create :invoke_helper, :if => "commentable_type == 'Grit::Commit'" + after_create :subscribe_on_reply + after_create :deliver_new_comment_notification + + def helper + class_eval "def commentable; project.git_repository.commit('#{commentable_id}'); end" if commentable_type == 'Grit::Commit' + end protected - #def subscribe_users - # Subscribe.subscribe_users(self.project) - #end - def deliver_new_comment_notification subscribes = self.commentable.subscribes if self.commentable_type == 'Issue' + # FIXME (true) for rspec subscribes = self.project.commit_comments_subscribes(true) if self.commentable_type == 'Grit::Commit' subscribes.each do |subscribe| if self.commentable_type == 'Issue' && self.user_id != subscribe.user_id && User.find(subscribe.user).notifier.new_comment_reply && User.find(subscribe.user).notifier.can_notify @@ -27,12 +27,22 @@ class Comment < ActiveRecord::Base UserMailer.delay.new_comment_notification(self, subscribe.user) end elsif self.commentable_type == 'Grit::Commit' && self.user_id != subscribe.user_id && User.find(subscribe.user).notifier.new_comment_commit_repo_owner && User.find(subscribe.user).notifier.can_notify - UserMailer.delay.new_comment_notification(self, subscribe.user) + if Comment.where(:commentable_type => 'Grit::Commit', :commentable_id => self.commentable.id, :user_id => subscribe.user.id).exists? + UserMailer.delay.new_comment_reply_notification(self, subscribe.user) + else + UserMailer.delay.new_comment_notification(self, subscribe.user) + end end end end def subscribe_on_reply - self.commentable.subscribes.create(:user_id => self.user_id) if !self.commentable.subscribes.exists?(:user_id => self.user_id) + self.commentable.subscribes.create(:user_id => self.user_id) if self.commentable_type == 'Issue' && !self.commentable.subscribes.exists?(:user_id => self.user_id) + self.project.commit_comments_subscribes.create(:user_id => self.user_id) if self.commentable_type == 'Grit::Commit' && !self.project.commit_comments_subscribes.exists?(:user_id => self.user_id) end + + def invoke_helper + self.helper + end + end diff --git a/app/models/project.rb b/app/models/project.rb index bee1b81f6..8ae339915 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -139,7 +139,7 @@ class Project < ActiveRecord::Base class << self def commit_comments(commit, project) comments = Comment.where(:commentable_id => commit.id, :commentable_type => 'Grit::Commit').order(:created_at) - comments.each {|x| x.project = project} + comments.each {|x| x.project = project; x.helper} end end diff --git a/app/views/user_mailer/new_commit_comment_notification.haml b/app/views/user_mailer/new_commit_comment_notification.haml new file mode 100644 index 000000000..48885d8c9 --- /dev/null +++ b/app/views/user_mailer/new_commit_comment_notification.haml @@ -0,0 +1,9 @@ +%p== Здравствуйте, #{@user.name}. + + +%p На Ваш комментарий в коммите #{ link_to @comment.commentable.message, commit_path(@comment.project, @comment.commentable_id]) } был дан ответ. + +%p "#{ @comment.body }" + + +%p== Команда поддержки «ROSA Build System» diff --git a/spec/controllers/comments_controller_for_commit_spec.rb b/spec/controllers/comments_controller_for_commit_spec.rb index 70daadb17..e6bb52981 100644 --- a/spec/controllers/comments_controller_for_commit_spec.rb +++ b/spec/controllers/comments_controller_for_commit_spec.rb @@ -77,8 +77,9 @@ describe CommentsController do %x(cp -Rf #{Rails.root}/spec/tests.git/* #{@project.git_repository.path}) # maybe FIXME ? @commit = @project.git_repository.commits.first - @comment = Factory(:comment) - @comment.update_attributes(:commentable_type => @commit.class.name, :commentable_id => @commit.id) + @comment = Factory(:commit_comment, :project => @project, + :commentable_type => @commit.class.name, :commentable_id => @commit.id) + @comment.helper @create_params = {:comment => {:body => 'I am a comment!'}, :project_id => @project.id, :commit_id => @commit.id} @update_params = {:comment => {:body => 'updated'}, :project_id => @project.id, :commit_id => @commit.id} diff --git a/spec/factories/comments.rb b/spec/factories/comments.rb index f036eee06..0a0a6634e 100644 --- a/spec/factories/comments.rb +++ b/spec/factories/comments.rb @@ -3,3 +3,11 @@ Factory.define(:comment) do |p| p.association :user, :factory => :user p.association :commentable, :factory => :issue end + +Factory.define(:commit_comment, :class => 'Comment') do |p| + p.body { Factory.next(:string) } + p.association :user, :factory => :user + p.commentable_type 'Grit::Commit' + p.commentable_id 'asdf' + p.project nil +end \ No newline at end of file diff --git a/spec/models/comment_for_commit_spec.rb b/spec/models/comment_for_commit_spec.rb index 4a6b71a57..3ca26d542 100644 --- a/spec/models/comment_for_commit_spec.rb +++ b/spec/models/comment_for_commit_spec.rb @@ -80,6 +80,7 @@ describe Comment do ActionMailer::Base.deliveries = [] comment = Comment.new(:user => @stranger, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) + comment.helper comment.save ActionMailer::Base.deliveries.count.should == 1 ActionMailer::Base.deliveries.last.to.include?(@user.email).should == true @@ -92,6 +93,7 @@ describe Comment do @project.commit_comments_subscribes.where(:user_id => @user).first.destroy # FIXME comment = Comment.new(:user => @stranger, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) + comment.helper comment.save ActionMailer::Base.deliveries.count.should == 0 # cache project.commit_comments_subscribes ... end @@ -103,6 +105,7 @@ describe Comment do @user.notifier.update_attribute :new_comment_commit_repo_owner, false comment = Comment.new(:user => @stranger, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) + comment.helper comment.save ActionMailer::Base.deliveries.count.should == 0 end @@ -114,6 +117,7 @@ describe Comment do @user.notifier.update_attribute :can_notify, false comment = Comment.new(:user => @stranger, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) + comment.helper comment.save ActionMailer::Base.deliveries.count.should == 0 end @@ -153,6 +157,7 @@ describe Comment do ActionMailer::Base.deliveries = [] comment = Comment.new(:user => @stranger, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) + comment.helper comment.save ActionMailer::Base.deliveries.count.should == 1 ActionMailer::Base.deliveries.last.to.include?(@project.owner.email).should == true @@ -165,6 +170,7 @@ describe Comment do @project.commit_comments_subscribes.where(:user_id => @project.owner).first.destroy # FIXME comment = Comment.new(:user => @stranger, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) + comment.helper comment.save ActionMailer::Base.deliveries.count.should == 0 # cache project.commit_comments_subscribes ... end @@ -176,6 +182,7 @@ describe Comment do @project.owner.notifier.update_attribute :new_comment_commit_repo_owner, false comment = Comment.new(:user => @stranger, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) + comment.helper comment.save ActionMailer::Base.deliveries.count.should == 0 end @@ -187,6 +194,7 @@ describe Comment do @project.owner.notifier.update_attribute :can_notify, false comment = Comment.new(:user => @stranger, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) + comment.helper comment.save ActionMailer::Base.deliveries.count.should == 0 end @@ -228,10 +236,25 @@ describe Comment do ActionMailer::Base.deliveries = [] comment = Comment.new(:user => @stranger, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) + comment.helper comment.save ActionMailer::Base.deliveries.count.should == 1 ActionMailer::Base.deliveries.last.to.include?(@stranger.email).should == false end + + it 'should send an e-mail for comments after his comment' do + comment = Comment.new(:user => @simple, :body => 'hello!', :project => @project, + :commentable_type => @commit.class.name, :commentable_id => @commit.id) + comment.helper + comment.save + ActionMailer::Base.deliveries = [] + comment = Comment.new(:user => @user, :body => 'owner comment', :project => @project, + :commentable_type => @commit.class.name, :commentable_id => @commit.id) + comment.helper + comment.save + ActionMailer::Base.deliveries.count.should == 1 + ActionMailer::Base.deliveries.last.to.include?(@simple.email).should == true + end end context 'for subscribe in project' do @@ -241,6 +264,7 @@ describe Comment do @project.commit_comments_subscribes.create(:user_id => @stranger.id) comment = Comment.new(:user => @project.owner, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) + comment.helper comment.save ActionMailer::Base.deliveries.count.should == 1 ActionMailer::Base.deliveries.last.to.include?(@stranger.email).should == true @@ -252,6 +276,7 @@ describe Comment do @project.commit_comments_subscribes.create(:user_id => @stranger.id) comment = Comment.new(:user => @owner, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) + comment.helper comment.save ActionMailer::Base.deliveries.count.should == 0 end From 3a00a66b86b10590bf808bf6ebc5b8ad5c8945cf Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Fri, 20 Jan 2012 22:38:45 +0600 Subject: [PATCH 06/35] [refs #114] polish controller tests --- spec/models/comment_for_commit_spec.rb | 62 ++++++++------------------ 1 file changed, 18 insertions(+), 44 deletions(-) diff --git a/spec/models/comment_for_commit_spec.rb b/spec/models/comment_for_commit_spec.rb index 3ca26d542..b6bb89d70 100644 --- a/spec/models/comment_for_commit_spec.rb +++ b/spec/models/comment_for_commit_spec.rb @@ -55,7 +55,6 @@ describe Comment do @stranger = Factory(:user) set_comments_data_for_commit - @project.relations.create!(:object_type => 'User', :object_id => @user.id, :role => 'admin') end @@ -78,10 +77,8 @@ describe Comment do context 'for default enabled settings' do it 'should send an e-mail by default settings' do ActionMailer::Base.deliveries = [] - comment = Comment.new(:user => @stranger, :body => 'hello!', :project => @project, + comment = Comment.create(:user => @stranger, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) - comment.helper - comment.save ActionMailer::Base.deliveries.count.should == 1 ActionMailer::Base.deliveries.last.to.include?(@user.email).should == true end @@ -91,10 +88,8 @@ describe Comment do it 'should not send an e-mail' do ActionMailer::Base.deliveries = [] @project.commit_comments_subscribes.where(:user_id => @user).first.destroy # FIXME - comment = Comment.new(:user => @stranger, :body => 'hello!', :project => @project, + comment = Comment.create(:user => @stranger, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) - comment.helper - comment.save ActionMailer::Base.deliveries.count.should == 0 # cache project.commit_comments_subscribes ... end end @@ -103,10 +98,8 @@ describe Comment do it 'should not send an e-mail' do ActionMailer::Base.deliveries = [] @user.notifier.update_attribute :new_comment_commit_repo_owner, false - comment = Comment.new(:user => @stranger, :body => 'hello!', :project => @project, + comment = Comment.create(:user => @stranger, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) - comment.helper - comment.save ActionMailer::Base.deliveries.count.should == 0 end end @@ -115,10 +108,8 @@ describe Comment do it 'should not send an e-mail' do ActionMailer::Base.deliveries = [] @user.notifier.update_attribute :can_notify, false - comment = Comment.new(:user => @stranger, :body => 'hello!', :project => @project, + comment = Comment.create(:user => @stranger, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) - comment.helper - comment.save ActionMailer::Base.deliveries.count.should == 0 end end @@ -137,7 +128,7 @@ describe Comment do end it 'should create comment' do - @ability.should be_able_to(:create, Comment.new(@create_params)) + @ability.should be_able_to(:create, Comment.create(@create_params)) end it 'should update comment' do @@ -155,10 +146,8 @@ describe Comment do context 'for default enabled settings' do it 'should send an e-mail by default settings' do ActionMailer::Base.deliveries = [] - comment = Comment.new(:user => @stranger, :body => 'hello!', :project => @project, + comment = Comment.create(:user => @stranger, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) - comment.helper - comment.save ActionMailer::Base.deliveries.count.should == 1 ActionMailer::Base.deliveries.last.to.include?(@project.owner.email).should == true end @@ -167,11 +156,9 @@ describe Comment do context 'for disabled notify setting in project' do it 'should not send an e-mail' do ActionMailer::Base.deliveries = [] - @project.commit_comments_subscribes.where(:user_id => @project.owner).first.destroy # FIXME - comment = Comment.new(:user => @stranger, :body => 'hello!', :project => @project, + Subscribe.first.destroy # FIXME + comment = Comment.create(:user => @stranger, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) - comment.helper - comment.save ActionMailer::Base.deliveries.count.should == 0 # cache project.commit_comments_subscribes ... end end @@ -180,10 +167,8 @@ describe Comment do it 'should not send an e-mail' do ActionMailer::Base.deliveries = [] @project.owner.notifier.update_attribute :new_comment_commit_repo_owner, false - comment = Comment.new(:user => @stranger, :body => 'hello!', :project => @project, + comment = Comment.create(:user => @stranger, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) - comment.helper - comment.save ActionMailer::Base.deliveries.count.should == 0 end end @@ -192,10 +177,8 @@ describe Comment do it 'should not send an e-mail' do ActionMailer::Base.deliveries = [] @project.owner.notifier.update_attribute :can_notify, false - comment = Comment.new(:user => @stranger, :body => 'hello!', :project => @project, + comment = Comment.create(:user => @stranger, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) - comment.helper - comment.save ActionMailer::Base.deliveries.count.should == 0 end end @@ -216,7 +199,7 @@ describe Comment do end it 'should create comment' do - @ability.should be_able_to(:create, Comment.new(@create_params)) + @ability.should be_able_to(:create, Comment.create(@create_params)) end it 'should update comment' do @@ -234,24 +217,19 @@ describe Comment do context 'for default enabled settings' do it 'should not send an e-mail' do ActionMailer::Base.deliveries = [] - comment = Comment.new(:user => @stranger, :body => 'hello!', :project => @project, + comment = Comment.create(:user => @stranger, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) - comment.helper - comment.save ActionMailer::Base.deliveries.count.should == 1 ActionMailer::Base.deliveries.last.to.include?(@stranger.email).should == false end it 'should send an e-mail for comments after his comment' do - comment = Comment.new(:user => @simple, :body => 'hello!', :project => @project, + comment = Comment.create(:user => @simple, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) - comment.helper - comment.save + ActionMailer::Base.deliveries = [] - comment = Comment.new(:user => @user, :body => 'owner comment', :project => @project, + comment = Comment.create(:user => @user, :body => 'owner comment', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) - comment.helper - comment.save ActionMailer::Base.deliveries.count.should == 1 ActionMailer::Base.deliveries.last.to.include?(@simple.email).should == true end @@ -262,10 +240,8 @@ describe Comment do ActionMailer::Base.deliveries = [] @project.owner.notifier.update_attribute :can_notify, false @project.commit_comments_subscribes.create(:user_id => @stranger.id) - comment = Comment.new(:user => @project.owner, :body => 'hello!', :project => @project, + comment = Comment.create(:user => @project.owner, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) - comment.helper - comment.save ActionMailer::Base.deliveries.count.should == 1 ActionMailer::Base.deliveries.last.to.include?(@stranger.email).should == true end @@ -274,13 +250,11 @@ describe Comment do ActionMailer::Base.deliveries = [] @project.owner.notifier.update_attribute :can_notify, false @project.commit_comments_subscribes.create(:user_id => @stranger.id) - comment = Comment.new(:user => @owner, :body => 'hello!', :project => @project, + comment = Comment.create(:user => @owner, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) - comment.helper - comment.save ActionMailer::Base.deliveries.count.should == 0 end end end -end +end \ No newline at end of file From d8df6c627992929fedd74d878d5957d910587ea2 Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Sat, 21 Jan 2012 00:22:25 +0600 Subject: [PATCH 07/35] [refs #114] committer subscribe + some tests --- app/models/comment.rb | 13 ++++++++-- spec/models/comment_for_commit_spec.rb | 36 +++++++++++++++++++++++++- 2 files changed, 46 insertions(+), 3 deletions(-) diff --git a/app/models/comment.rb b/app/models/comment.rb index 9e4dc8f5d..feb8023f6 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -17,8 +17,11 @@ class Comment < ActiveRecord::Base def deliver_new_comment_notification subscribes = self.commentable.subscribes if self.commentable_type == 'Issue' - # FIXME (true) for rspec - subscribes = self.project.commit_comments_subscribes(true) if self.commentable_type == 'Grit::Commit' + + if self.commentable_type == 'Grit::Commit' + subscribe_committer + subscribes = self.project.commit_comments_subscribes(true) # FIXME (true) for rspec + end subscribes.each do |subscribe| if self.commentable_type == 'Issue' && self.user_id != subscribe.user_id && User.find(subscribe.user).notifier.new_comment_reply && User.find(subscribe.user).notifier.can_notify if self.commentable.comments.exists?(:user_id => subscribe.user.id) @@ -45,4 +48,10 @@ class Comment < ActiveRecord::Base self.helper end + def subscribe_committer + committer = User.where(:email => self.commentable.committer.email).first + if committer && !self.project.commit_comments_subscribes.exists?(:user_id => committer.id) && committer.notifier.new_comment_commit_owner + self.project.commit_comments_subscribes.create(:user_id => committer.id) + end + end end diff --git a/spec/models/comment_for_commit_spec.rb b/spec/models/comment_for_commit_spec.rb index b6bb89d70..dc776d4e3 100644 --- a/spec/models/comment_for_commit_spec.rb +++ b/spec/models/comment_for_commit_spec.rb @@ -156,7 +156,7 @@ describe Comment do context 'for disabled notify setting in project' do it 'should not send an e-mail' do ActionMailer::Base.deliveries = [] - Subscribe.first.destroy # FIXME + @project.commit_comments_subscribes.where(:user_id => @user).first.destroy # FIXME comment = Comment.create(:user => @stranger, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) ActionMailer::Base.deliveries.count.should == 0 # cache project.commit_comments_subscribes ... @@ -183,6 +183,28 @@ describe Comment do end end + context 'for own commit' do + it 'should send a one e-mail' do + ActionMailer::Base.deliveries = [] + @project.owner.update_attribute :email, 'code@tpope.net' + comment = Comment.create(:user => @stranger, :body => 'hello!', :project => @project, + :commentable_type => @commit.class.name, :commentable_id => @commit.id) + ActionMailer::Base.deliveries.count.should == 1 + ActionMailer::Base.deliveries.last.to.include?(@user.email).should == true + end + + it 'should not send an e-mail if disable comments of own commit' do + ActionMailer::Base.deliveries = [] + @project.owner.update_attribute :email, 'code@tpope.net' + # what setting is first-priority? + @project.owner.notifier.update_attribute :new_comment_commit_repo_owner, false + @project.owner.notifier.update_attribute :new_comment_commit_owner, false + comment = Comment.create(:user => @stranger, :body => 'hello!', :project => @project, + :commentable_type => @commit.class.name, :commentable_id => @commit.id) + ActionMailer::Base.deliveries.count.should == 0 + end + end + end context 'for simple user' do @@ -233,6 +255,18 @@ describe Comment do ActionMailer::Base.deliveries.count.should == 1 ActionMailer::Base.deliveries.last.to.include?(@simple.email).should == true end + + context 'for own commit' do + it 'should send an e-mail' do + ActionMailer::Base.deliveries = [] + @stranger.update_attribute :email, 'code@tpope.net' + comment = Comment.create(:user => @user, :body => 'hello!', :project => @project, + :commentable_type => @commit.class.name, :commentable_id => @commit.id) + ActionMailer::Base.deliveries.count.should == 1 + ActionMailer::Base.deliveries.last.to.include?(@stranger.email).should == true + end + end + end context 'for subscribe in project' do From 21d422c3135d55527b8b76262b5d8372e1efb9a2 Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Sat, 21 Jan 2012 19:32:22 +0600 Subject: [PATCH 08/35] [refs #114] refactoring subscribe --- app/models/comment.rb | 30 +++++------------------------- app/models/subscribe.rb | 38 +++++++++++++++++++++++--------------- 2 files changed, 28 insertions(+), 40 deletions(-) diff --git a/app/models/comment.rb b/app/models/comment.rb index feb8023f6..d86dfc2d9 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -7,38 +7,18 @@ class Comment < ActiveRecord::Base after_create :invoke_helper, :if => "commentable_type == 'Grit::Commit'" after_create :subscribe_on_reply - after_create :deliver_new_comment_notification + after_create {|comment| Subscribe.new_comment_notification(comment)} def helper class_eval "def commentable; project.git_repository.commit('#{commentable_id}'); end" if commentable_type == 'Grit::Commit' end - protected - - def deliver_new_comment_notification - subscribes = self.commentable.subscribes if self.commentable_type == 'Issue' - - if self.commentable_type == 'Grit::Commit' - subscribe_committer - subscribes = self.project.commit_comments_subscribes(true) # FIXME (true) for rspec - end - subscribes.each do |subscribe| - if self.commentable_type == 'Issue' && self.user_id != subscribe.user_id && User.find(subscribe.user).notifier.new_comment_reply && User.find(subscribe.user).notifier.can_notify - if self.commentable.comments.exists?(:user_id => subscribe.user.id) - UserMailer.delay.new_comment_reply_notification(self, subscribe.user) - else - UserMailer.delay.new_comment_notification(self, subscribe.user) - end - elsif self.commentable_type == 'Grit::Commit' && self.user_id != subscribe.user_id && User.find(subscribe.user).notifier.new_comment_commit_repo_owner && User.find(subscribe.user).notifier.can_notify - if Comment.where(:commentable_type => 'Grit::Commit', :commentable_id => self.commentable.id, :user_id => subscribe.user.id).exists? - UserMailer.delay.new_comment_reply_notification(self, subscribe.user) - else - UserMailer.delay.new_comment_notification(self, subscribe.user) - end - end - end + def own_comment?(user) + user_id == user.id end + protected + def subscribe_on_reply self.commentable.subscribes.create(:user_id => self.user_id) if self.commentable_type == 'Issue' && !self.commentable.subscribes.exists?(:user_id => self.user_id) self.project.commit_comments_subscribes.create(:user_id => self.user_id) if self.commentable_type == 'Grit::Commit' && !self.project.commit_comments_subscribes.exists?(:user_id => self.user_id) diff --git a/app/models/subscribe.rb b/app/models/subscribe.rb index caffd9459..638947a71 100644 --- a/app/models/subscribe.rb +++ b/app/models/subscribe.rb @@ -2,25 +2,33 @@ class Subscribe < ActiveRecord::Base belongs_to :subscribeable, :polymorphic => true belongs_to :user - def self.subscribe_users(project) - recipients = Subscribe.collect_recipient_ids(project) - recipients.each do |recipient_id| - ss = project.commit_comments_subscribes.build(:user_id => recipient_id) - ss.save! + def self.new_comment_notification(comment) + commentable_class = comment.commentable.class + subscribes = comment.commentable.subscribes if commentable_class == Issue + if commentable_class == Grit::Commit + Subscribe.subscribe_committer(comment) + subscribes = comment.project.commit_comments_subscribes(true) # FIXME (true) for rspec + end + subscribes.each do |subscribe| + user = subscribe.user + next if comment.own_comment?(user) || !user.notifier.can_notify + Subscribe.send_notification(comment, user) if commentable_class == Issue && user.notifier.new_comment_reply + Subscribe.send_notification(comment, user) if commentable_class == Grit::Commit && user.notifier.new_comment_commit_repo_owner end end - def self.collect_recipient_ids(project) - recipients = project.relations.by_role('admin').where(:object_type => 'User').map { |rel| rel.read_attribute(:object_id) } -# recipients = recipients | [commentable.user_id] if commentable.user_id -# recipients = recipients | [commentable.project.owner_id] if commentable.project.owner_type == 'User' - - # filter by notification settings - recipients = recipients.select do |recipient| - User.find(recipient).notifier.new_issue && User.find(recipient).notifier.can_notify + def self.subscribe_committer(comment) + committer = User.where(:email => comment.commentable.committer.email).first + if committer && !comment.project.commit_comments_subscribes.exists?(:user_id => committer.id) && committer.notifier.new_comment_commit_owner + comment.project.commit_comments_subscribes.create(:user_id => committer.id) end - - recipients end + def self.send_notification(comment, user) + if Comment.where(:commentable_type => comment.commentable_type, :commentable_id => comment.commentable.id.to_s, :user_id => user.id).exists? + UserMailer.delay.new_comment_reply_notification(comment, user) + else + UserMailer.delay.new_comment_notification(comment, user) + end + end end From 36a895385c42cce58c45ac711a4cd4eb7982a6e7 Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Sun, 22 Jan 2012 01:13:33 +0600 Subject: [PATCH 09/35] [refs #114] changed rules --- app/models/comment.rb | 8 +- app/models/project.rb | 6 +- app/models/relation.rb | 6 ++ app/models/subscribe.rb | 25 +++-- spec/models/comment_for_commit_spec.rb | 132 ++++++++++++++++++------- 5 files changed, 121 insertions(+), 56 deletions(-) diff --git a/app/models/comment.rb b/app/models/comment.rb index d86dfc2d9..0353fc3f0 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -17,6 +17,7 @@ class Comment < ActiveRecord::Base user_id == user.id end + protected def subscribe_on_reply @@ -27,11 +28,4 @@ class Comment < ActiveRecord::Base def invoke_helper self.helper end - - def subscribe_committer - committer = User.where(:email => self.commentable.committer.email).first - if committer && !self.project.commit_comments_subscribes.exists?(:user_id => committer.id) && committer.notifier.new_comment_commit_owner - self.project.commit_comments_subscribes.create(:user_id => committer.id) - end - end end diff --git a/app/models/project.rb b/app/models/project.rb index 8ae339915..cc66415e6 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -31,7 +31,7 @@ class Project < ActiveRecord::Base after_create :attach_to_personal_repository after_create :create_git_repo - after_create :subscribe_users + after_create :subscribe_owner after_destroy :destroy_git_repo # after_rollback lambda { destroy_git_repo rescue true if new_record? } @@ -161,7 +161,7 @@ class Project < ActiveRecord::Base FileUtils.rm_rf path end - def subscribe_users - self.commit_comments_subscribes.create(:user_id => self.owner.id) + def subscribe_owner + Subscribe.subscribe_user(self, self.owner.id) end end diff --git a/app/models/relation.rb b/app/models/relation.rb index a08ddf6a9..d2e35bc23 100644 --- a/app/models/relation.rb +++ b/app/models/relation.rb @@ -12,6 +12,8 @@ class Relation < ActiveRecord::Base scope :by_target, lambda {|tar| {:conditions => ['target_id = ? AND target_type = ?', tar.id, tar.class.to_s]}} scope :by_role, lambda {|role| {:conditions => ['role = ?', role]}} + after_create :subscribe_project_admin, :if => "role == 'admin' && object_id == 'User' && targer_type == 'Project'" + def self.create_with_role(object, target, role) r = new r.object = object @@ -24,4 +26,8 @@ class Relation < ActiveRecord::Base def add_default_role self.role = ROLES.first if role.nil? || role.empty? end + + def subscribe_project_admin + Subscribe.subscribe_user(self.target_id, self.object_id) + end end diff --git a/app/models/subscribe.rb b/app/models/subscribe.rb index 638947a71..9f26245f6 100644 --- a/app/models/subscribe.rb +++ b/app/models/subscribe.rb @@ -6,22 +6,21 @@ class Subscribe < ActiveRecord::Base commentable_class = comment.commentable.class subscribes = comment.commentable.subscribes if commentable_class == Issue if commentable_class == Grit::Commit - Subscribe.subscribe_committer(comment) subscribes = comment.project.commit_comments_subscribes(true) # FIXME (true) for rspec + committer = User.where(:email => comment.commentable.committer.email).first + Subscribe.send_notification(comment, committer) if committer && committer.notifier.new_comment_commit_owner && subscribes.where(:user_id => committer).empty? end subscribes.each do |subscribe| - user = subscribe.user - next if comment.own_comment?(user) || !user.notifier.can_notify - Subscribe.send_notification(comment, user) if commentable_class == Issue && user.notifier.new_comment_reply - Subscribe.send_notification(comment, user) if commentable_class == Grit::Commit && user.notifier.new_comment_commit_repo_owner + user = subscribe.user + next if comment.own_comment?(user) || !user.notifier.can_notify + Subscribe.send_notification(comment, user) if commentable_class == Issue && user.notifier.new_comment_reply + Subscribe.send_notification(comment, user) if commentable_class == Grit::Commit && Subscribe.send_notification_for_commit_comment?(subscribe.subscribeable, user, comment) end end - def self.subscribe_committer(comment) - committer = User.where(:email => comment.commentable.committer.email).first - if committer && !comment.project.commit_comments_subscribes.exists?(:user_id => committer.id) && committer.notifier.new_comment_commit_owner - comment.project.commit_comments_subscribes.create(:user_id => committer.id) - end + def self.subscribe_user(project_id, user_id) + list = Project.find(project_id).commit_comments_subscribes + list.create(:user_id => user_id) unless list.exists?(:user_id => user_id) end def self.send_notification(comment, user) @@ -31,4 +30,10 @@ class Subscribe < ActiveRecord::Base UserMailer.delay.new_comment_notification(comment, user) end end + + def self.send_notification_for_commit_comment?(project, user, comment) + is_owner = (project.owner_id == user.id) + is_commentor = (project.commit_comments_subscribes.exists?(:user_id => user.id)) + (is_owner && user.notifier.new_comment_commit_repo_owner) or (is_commentor && user.notifier.new_comment_commit_commentor) + end end diff --git a/spec/models/comment_for_commit_spec.rb b/spec/models/comment_for_commit_spec.rb index dc776d4e3..a66dc15b4 100644 --- a/spec/models/comment_for_commit_spec.rb +++ b/spec/models/comment_for_commit_spec.rb @@ -74,8 +74,8 @@ describe Comment do @ability.should_not be_able_to(:destroy, @comment) end - context 'for default enabled settings' do - it 'should send an e-mail by default settings' do + context 'for default settings' do + it 'should send an e-mail' do ActionMailer::Base.deliveries = [] comment = Comment.create(:user => @stranger, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) @@ -84,23 +84,58 @@ describe Comment do end end - context 'for disabled notify setting in project' do + context 'for disabled notify setting new_comment_commit_repo_owner' do + it 'should send an e-mail' do + ActionMailer::Base.deliveries = [] + @user.notifier.update_attribute :new_comment_commit_repo_owner, false + comment = Comment.create(:user => @stranger, :body => 'hello!', :project => @project, + :commentable_type => @commit.class.name, :commentable_id => @commit.id) + ActionMailer::Base.deliveries.count.should == 1 # cache project.commit_comments_subscribes ... + ActionMailer::Base.deliveries.last.to.include?(@user.email).should == true + end + end + + context 'for disabled notify setting new_comment_commit_owner' do + it 'should send an e-mail' do + ActionMailer::Base.deliveries = [] + @user.notifier.update_attribute :new_comment_commit_owner, false + comment = Comment.create(:user => @stranger, :body => 'hello!', :project => @project, + :commentable_type => @commit.class.name, :commentable_id => @commit.id) + ActionMailer::Base.deliveries.count.should == 1 # cache project.commit_comments_subscribes ... + ActionMailer::Base.deliveries.last.to.include?(@user.email).should == true + end + end + + context 'for disabled notify setting new_comment_commit_commentor' do + it 'should send an e-mail' do + ActionMailer::Base.deliveries = [] + @user.notifier.update_attribute :new_comment_commit_commentor, false + comment = Comment.create(:user => @stranger, :body => 'hello!', :project => @project, + :commentable_type => @commit.class.name, :commentable_id => @commit.id) + ActionMailer::Base.deliveries.count.should == 1 # cache project.commit_comments_subscribes ... + ActionMailer::Base.deliveries.last.to.include?(@user.email).should == true + end + end + + context 'for disabled all notify setting expect global' do it 'should not send an e-mail' do ActionMailer::Base.deliveries = [] - @project.commit_comments_subscribes.where(:user_id => @user).first.destroy # FIXME + @user.notifier.update_attribute :new_comment_commit_repo_owner, false + @user.notifier.update_attribute :new_comment_commit_owner, false + @user.notifier.update_attribute :new_comment_commit_commentor, false comment = Comment.create(:user => @stranger, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) ActionMailer::Base.deliveries.count.should == 0 # cache project.commit_comments_subscribes ... end end - context 'for disabled notify setting' do + context 'for unsubscribe project' do it 'should not send an e-mail' do ActionMailer::Base.deliveries = [] - @user.notifier.update_attribute :new_comment_commit_repo_owner, false + @project.commit_comments_subscribes.where(:user_id => @user).first.destroy # FIXME comment = Comment.create(:user => @stranger, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) - ActionMailer::Base.deliveries.count.should == 0 + ActionMailer::Base.deliveries.count.should == 0 # cache project.commit_comments_subscribes ... end end @@ -153,7 +188,52 @@ describe Comment do end end - context 'for disabled notify setting in project' do + context 'for disabled notify setting new_comment_commit_repo_owner' do + it 'should not send an e-mail' do + ActionMailer::Base.deliveries = [] + @user.notifier.update_attribute :new_comment_commit_repo_owner, false + comment = Comment.create(:user => @stranger, :body => 'hello!', :project => @project, + :commentable_type => @commit.class.name, :commentable_id => @commit.id) + ActionMailer::Base.deliveries.count.should == 1 # cache project.commit_comments_subscribes ... + ActionMailer::Base.deliveries.last.to.include?(@user.email).should == true + end + end + + context 'for disabled notify setting new_comment_commit_owner' do + it 'should send an e-mail' do + ActionMailer::Base.deliveries = [] + @user.notifier.update_attribute :new_comment_commit_owner, false + comment = Comment.create(:user => @stranger, :body => 'hello!', :project => @project, + :commentable_type => @commit.class.name, :commentable_id => @commit.id) + ActionMailer::Base.deliveries.count.should == 1 # cache project.commit_comments_subscribes ... + ActionMailer::Base.deliveries.last.to.include?(@user.email).should == true + end + end + + context 'for disabled notify setting new_comment_commit_commentor' do + it 'should send an e-mail' do + ActionMailer::Base.deliveries = [] + @user.notifier.update_attribute :new_comment_commit_commentor, false + comment = Comment.create(:user => @stranger, :body => 'hello!', :project => @project, + :commentable_type => @commit.class.name, :commentable_id => @commit.id) + ActionMailer::Base.deliveries.count.should == 1 # cache project.commit_comments_subscribes ... + ActionMailer::Base.deliveries.last.to.include?(@user.email).should == true + end + end + + context 'for disabled all notify setting expect global' do + it 'should not send an e-mail' do + ActionMailer::Base.deliveries = [] + @user.notifier.update_attribute :new_comment_commit_repo_owner, false + @user.notifier.update_attribute :new_comment_commit_owner, false + @user.notifier.update_attribute :new_comment_commit_commentor, false + comment = Comment.create(:user => @stranger, :body => 'hello!', :project => @project, + :commentable_type => @commit.class.name, :commentable_id => @commit.id) + ActionMailer::Base.deliveries.count.should == 0 # cache project.commit_comments_subscribes ... + end + end + + context 'for unsubscribe project' do it 'should not send an e-mail' do ActionMailer::Base.deliveries = [] @project.commit_comments_subscribes.where(:user_id => @user).first.destroy # FIXME @@ -163,20 +243,10 @@ describe Comment do end end - context 'for disabled notify setting' do - it 'should not send an e-mail' do - ActionMailer::Base.deliveries = [] - @project.owner.notifier.update_attribute :new_comment_commit_repo_owner, false - comment = Comment.create(:user => @stranger, :body => 'hello!', :project => @project, - :commentable_type => @commit.class.name, :commentable_id => @commit.id) - ActionMailer::Base.deliveries.count.should == 0 - end - end - context 'for disabled global notify setting' do it 'should not send an e-mail' do ActionMailer::Base.deliveries = [] - @project.owner.notifier.update_attribute :can_notify, false + @user.notifier.update_attribute :can_notify, false comment = Comment.create(:user => @stranger, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) ActionMailer::Base.deliveries.count.should == 0 @@ -189,19 +259,9 @@ describe Comment do @project.owner.update_attribute :email, 'code@tpope.net' comment = Comment.create(:user => @stranger, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) - ActionMailer::Base.deliveries.count.should == 1 - ActionMailer::Base.deliveries.last.to.include?(@user.email).should == true - end - it 'should not send an e-mail if disable comments of own commit' do - ActionMailer::Base.deliveries = [] - @project.owner.update_attribute :email, 'code@tpope.net' - # what setting is first-priority? - @project.owner.notifier.update_attribute :new_comment_commit_repo_owner, false - @project.owner.notifier.update_attribute :new_comment_commit_owner, false - comment = Comment.create(:user => @stranger, :body => 'hello!', :project => @project, - :commentable_type => @commit.class.name, :commentable_id => @commit.id) - ActionMailer::Base.deliveries.count.should == 0 + ActionMailer::Base.deliveries.count.should == 1 + ActionMailer::Base.deliveries.last.to.include?(@project.owner.email).should == true end end @@ -267,12 +327,13 @@ describe Comment do end end - end - - context 'for subscribe in project' do it 'should send an e-mail' do ActionMailer::Base.deliveries = [] @project.owner.notifier.update_attribute :can_notify, false + @stranger.notifier.update_attribute :new_comment_commit_repo_owner, false + @stranger.notifier.update_attribute :new_comment_commit_owner, false + #@stranger.notifier.update_attribute :new_comment_commit_commentor, false + @project.commit_comments_subscribes.create(:user_id => @stranger.id) comment = Comment.create(:user => @project.owner, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) @@ -282,13 +343,12 @@ describe Comment do it 'should not send an e-mail for own comment' do ActionMailer::Base.deliveries = [] - @project.owner.notifier.update_attribute :can_notify, false + #@project.owner.notifier.update_attribute :can_notify, false @project.commit_comments_subscribes.create(:user_id => @stranger.id) comment = Comment.create(:user => @owner, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) ActionMailer::Base.deliveries.count.should == 0 end end - end end \ No newline at end of file From ab081d3c46b988b96f4edc7ecb56e46a72ebc17e Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Sun, 22 Jan 2012 22:15:25 +0600 Subject: [PATCH 10/35] [refs #114] only new comment notification --- app/mailers/user_mailer.rb | 6 ------ app/models/subscribe.rb | 14 +++----------- .../new_comment_reply_notification.haml | 9 --------- 3 files changed, 3 insertions(+), 26 deletions(-) delete mode 100644 app/views/user_mailer/new_comment_reply_notification.haml diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb index 5680dc408..9be195dd4 100644 --- a/app/mailers/user_mailer.rb +++ b/app/mailers/user_mailer.rb @@ -18,12 +18,6 @@ class UserMailer < ActionMailer::Base end end - def new_comment_reply_notification(comment, user) - @user = user - @comment = comment - template = 'new_commit_comment_reply_notification' if @comment.commentable_type == 'Grit::Commit' - mail(:to => user.email, :subject => I18n.t("notifications.subjects.new_comment_reply_notification"), :template_name => template) end - def new_issue_notification(issue, user) @user = user @issue = issue diff --git a/app/models/subscribe.rb b/app/models/subscribe.rb index 9f26245f6..4a21c1b4f 100644 --- a/app/models/subscribe.rb +++ b/app/models/subscribe.rb @@ -8,13 +8,13 @@ class Subscribe < ActiveRecord::Base if commentable_class == Grit::Commit subscribes = comment.project.commit_comments_subscribes(true) # FIXME (true) for rspec committer = User.where(:email => comment.commentable.committer.email).first - Subscribe.send_notification(comment, committer) if committer && committer.notifier.new_comment_commit_owner && subscribes.where(:user_id => committer).empty? + UserMailer.delay.new_comment_notification(comment, user) if committer && committer.notifier.new_comment_commit_owner && subscribes.where(:user_id => committer).empty? end subscribes.each do |subscribe| user = subscribe.user next if comment.own_comment?(user) || !user.notifier.can_notify - Subscribe.send_notification(comment, user) if commentable_class == Issue && user.notifier.new_comment_reply - Subscribe.send_notification(comment, user) if commentable_class == Grit::Commit && Subscribe.send_notification_for_commit_comment?(subscribe.subscribeable, user, comment) + UserMailer.delay.new_comment_notification(comment, user) if commentable_class == Issue && user.notifier.new_comment_reply + UserMailer.delay.new_comment_notification(comment, user) if commentable_class == Grit::Commit && Subscribe.send_notification_for_commit_comment?(subscribe.subscribeable, user, comment) end end @@ -23,14 +23,6 @@ class Subscribe < ActiveRecord::Base list.create(:user_id => user_id) unless list.exists?(:user_id => user_id) end - def self.send_notification(comment, user) - if Comment.where(:commentable_type => comment.commentable_type, :commentable_id => comment.commentable.id.to_s, :user_id => user.id).exists? - UserMailer.delay.new_comment_reply_notification(comment, user) - else - UserMailer.delay.new_comment_notification(comment, user) - end - end - def self.send_notification_for_commit_comment?(project, user, comment) is_owner = (project.owner_id == user.id) is_commentor = (project.commit_comments_subscribes.exists?(:user_id => user.id)) diff --git a/app/views/user_mailer/new_comment_reply_notification.haml b/app/views/user_mailer/new_comment_reply_notification.haml deleted file mode 100644 index 48ff0ab4b..000000000 --- a/app/views/user_mailer/new_comment_reply_notification.haml +++ /dev/null @@ -1,9 +0,0 @@ -%p== Здравствуйте, #{@user.name}. - - -%p На Ваш комментарий в задаче #{ link_to @comment.commentable.title, [@comment.commentable.project, @comment.commentable] } был дан ответ. - -%p "#{ @comment.body }" - - -%p== Команда поддержки «ROSA Build System» From f0d7ed0d89b166c54a8a44b3dd5e78a03c7ddffd Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Sun, 22 Jan 2012 22:17:08 +0600 Subject: [PATCH 11/35] [refs #114] fix notification for committer --- app/models/subscribe.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/subscribe.rb b/app/models/subscribe.rb index 4a21c1b4f..5ca43bcc0 100644 --- a/app/models/subscribe.rb +++ b/app/models/subscribe.rb @@ -8,7 +8,7 @@ class Subscribe < ActiveRecord::Base if commentable_class == Grit::Commit subscribes = comment.project.commit_comments_subscribes(true) # FIXME (true) for rspec committer = User.where(:email => comment.commentable.committer.email).first - UserMailer.delay.new_comment_notification(comment, user) if committer && committer.notifier.new_comment_commit_owner && subscribes.where(:user_id => committer).empty? + UserMailer.delay.new_comment_notification(comment, committer) if committer && committer.notifier.new_comment_commit_owner && subscribes.where(:user_id => committer).empty? end subscribes.each do |subscribe| user = subscribe.user From f1f6d1b78b4d3aebb9e11bdad9f6c8635f570844 Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Sun, 22 Jan 2012 22:17:55 +0600 Subject: [PATCH 12/35] [refs #114] email i18n --- .../user_mailer/new_comment_notification.en.haml | 14 ++++++++++++++ .../user_mailer/new_comment_notification.haml | 7 ------- .../user_mailer/new_comment_notification.ru.haml | 14 ++++++++++++++ .../new_commit_comment_notification.haml | 9 --------- 4 files changed, 28 insertions(+), 16 deletions(-) create mode 100644 app/views/user_mailer/new_comment_notification.en.haml delete mode 100644 app/views/user_mailer/new_comment_notification.haml create mode 100644 app/views/user_mailer/new_comment_notification.ru.haml delete mode 100644 app/views/user_mailer/new_commit_comment_notification.haml diff --git a/app/views/user_mailer/new_comment_notification.en.haml b/app/views/user_mailer/new_comment_notification.en.haml new file mode 100644 index 000000000..e7e70e53e --- /dev/null +++ b/app/views/user_mailer/new_comment_notification.en.haml @@ -0,0 +1,14 @@ +%p== Hello, #{@user.name}. + +- if @comment.commentable.class == Issue + - link = link_to @comment.commentable.title, [@comment.commentable.project, @comment.commentable] + - object = 'issue' +- elsif @comment.commentable.class == Grit::Commit + - link = link_to @comment.commentable.message, commit_path(@comment.project, @comment.commentable_id) + - object = 'commit' +%p User #{ link_to @comment.user.uname, user_path(@comment.user)} added new comment to #{object} #{link}. + +%p "#{ @comment.body }" + + +%p== Support Team «ROSA Build System» diff --git a/app/views/user_mailer/new_comment_notification.haml b/app/views/user_mailer/new_comment_notification.haml deleted file mode 100644 index 15b4e6a6f..000000000 --- a/app/views/user_mailer/new_comment_notification.haml +++ /dev/null @@ -1,7 +0,0 @@ -%p== Здравствуйте, #{@user.name}. - -%p К коммиту был добавлен новый комментарий. -%p "#{ @comment.body }" - - -%p== Команда поддержки «ROSA Build System» diff --git a/app/views/user_mailer/new_comment_notification.ru.haml b/app/views/user_mailer/new_comment_notification.ru.haml new file mode 100644 index 000000000..c0f4e27e3 --- /dev/null +++ b/app/views/user_mailer/new_comment_notification.ru.haml @@ -0,0 +1,14 @@ +%p== Hello, #{@user.name}. + +- if @comment.commentable.class == Issue + - link = link_to @comment.commentable.title, [@comment.commentable.project, @comment.commentable] + - object = 'issue' +- elsif @comment.commentable.class == Grit::Commit + - link = link_to @comment.commentable.message, commit_path(@comment.project, @comment.commentable_id]) + - object = 'commit' +%p User #{ link_to @comment.user.uname, user_path(@comment.user)} added new comment to #{object} #{link}. + +%p "#{ @comment.body }" + + +%p== Support Team «ROSA Build System» diff --git a/app/views/user_mailer/new_commit_comment_notification.haml b/app/views/user_mailer/new_commit_comment_notification.haml deleted file mode 100644 index 48885d8c9..000000000 --- a/app/views/user_mailer/new_commit_comment_notification.haml +++ /dev/null @@ -1,9 +0,0 @@ -%p== Здравствуйте, #{@user.name}. - - -%p На Ваш комментарий в коммите #{ link_to @comment.commentable.message, commit_path(@comment.project, @comment.commentable_id]) } был дан ответ. - -%p "#{ @comment.body }" - - -%p== Команда поддержки «ROSA Build System» From 651c5637ed66a506b2ceeb41dc2274e270ae4b3f Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Sun, 22 Jan 2012 23:49:41 +0600 Subject: [PATCH 13/35] [refs #114] fix committer notification --- app/models/subscribe.rb | 2 +- spec/models/comment_for_commit_spec.rb | 33 +++++++++++++++++++++++--- 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/app/models/subscribe.rb b/app/models/subscribe.rb index 5ca43bcc0..381feaf6c 100644 --- a/app/models/subscribe.rb +++ b/app/models/subscribe.rb @@ -8,7 +8,7 @@ class Subscribe < ActiveRecord::Base if commentable_class == Grit::Commit subscribes = comment.project.commit_comments_subscribes(true) # FIXME (true) for rspec committer = User.where(:email => comment.commentable.committer.email).first - UserMailer.delay.new_comment_notification(comment, committer) if committer && committer.notifier.new_comment_commit_owner && subscribes.where(:user_id => committer).empty? + UserMailer.delay.new_comment_notification(comment, committer) if committer && !comment.own_comment?(committer) && committer.notifier.new_comment_commit_owner && !committer.notifier.can_notify && subscribes.where(:user_id => committer).empty? end subscribes.each do |subscribe| user = subscribe.user diff --git a/spec/models/comment_for_commit_spec.rb b/spec/models/comment_for_commit_spec.rb index a66dc15b4..5fe0f3799 100644 --- a/spec/models/comment_for_commit_spec.rb +++ b/spec/models/comment_for_commit_spec.rb @@ -316,8 +316,8 @@ describe Comment do ActionMailer::Base.deliveries.last.to.include?(@simple.email).should == true end - context 'for own commit' do - it 'should send an e-mail' do + context 'for committer' do + it 'should send a one e-mail' do ActionMailer::Base.deliveries = [] @stranger.update_attribute :email, 'code@tpope.net' comment = Comment.create(:user => @user, :body => 'hello!', :project => @project, @@ -325,9 +325,36 @@ describe Comment do ActionMailer::Base.deliveries.count.should == 1 ActionMailer::Base.deliveries.last.to.include?(@stranger.email).should == true end + + it 'should not send an e-mail for own comment' do + ActionMailer::Base.deliveries = [] + #@project.owner.notifier.update_attribute :can_notify, false + @stranger.update_attribute :email, 'code@tpope.net' + comment = Comment.create(:user => @stranger, :body => 'hello!', :project => @project, + :commentable_type => @commit.class.name, :commentable_id => @commit.id) + ActionMailer::Base.deliveries.count.should == 0 + end + + it 'should not send an e-mail if global notify off' do + ActionMailer::Base.deliveries = [] + @project.owner.notifier.update_attribute :can_notify, false + @stranger.update_attribute :email, 'code@tpope.net' + comment = Comment.create(:user => @user, :body => 'hello!', :project => @project, + :commentable_type => @commit.class.name, :commentable_id => @commit.id) + ActionMailer::Base.deliveries.count.should == 0 + end + + it 'should not send an e-mail if notify for my commits off' do + ActionMailer::Base.deliveries = [] + @stranger.notifier.update_attribute :new_comment_commit_owner, false + @stranger.update_attribute :email, 'code@tpope.net' + comment = Comment.create(:user => @user, :body => 'hello!', :project => @project, + :commentable_type => @commit.class.name, :commentable_id => @commit.id) + ActionMailer::Base.deliveries.count.should == 0 + end end - it 'should send an e-mail' do + it 'should send an e-mail when subscribed to project' do ActionMailer::Base.deliveries = [] @project.owner.notifier.update_attribute :can_notify, false @stranger.notifier.update_attribute :new_comment_commit_repo_owner, false From 9127b6b689c447435bd4536e9061350d267197ed Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Tue, 24 Jan 2012 01:42:54 +0600 Subject: [PATCH 14/35] [refs #114] changed logics --- .../commit_subscribes_controller.rb | 31 +++++++++++++++ .../project_subscribes_controller.rb | 24 ------------ app/models/comment.rb | 11 +++++- app/models/issue.rb | 4 +- app/models/project.rb | 5 --- app/models/relation.rb | 6 --- app/models/subscribe.rb | 38 ++++++++++++++----- app/views/git/commits/show.html.haml | 9 +++++ app/views/projects/show.html.haml | 8 ---- config/routes.rb | 4 +- .../20120123120400_add_status_to_subscribe.rb | 9 +++++ ...20120123134616_add_project_to_subscribe.rb | 9 +++++ ...23161250_change_subscribeable_to_string.rb | 9 +++++ db/schema.rb | 16 +++++++- 14 files changed, 125 insertions(+), 58 deletions(-) create mode 100644 app/controllers/commit_subscribes_controller.rb delete mode 100644 app/controllers/project_subscribes_controller.rb create mode 100644 db/migrate/20120123120400_add_status_to_subscribe.rb create mode 100644 db/migrate/20120123134616_add_project_to_subscribe.rb create mode 100644 db/migrate/20120123161250_change_subscribeable_to_string.rb diff --git a/app/controllers/commit_subscribes_controller.rb b/app/controllers/commit_subscribes_controller.rb new file mode 100644 index 000000000..bc6596159 --- /dev/null +++ b/app/controllers/commit_subscribes_controller.rb @@ -0,0 +1,31 @@ +class CommitSubscribesController < ApplicationController + before_filter :authenticate_user! + + load_resource :subscribe + load_and_authorize_resource :project + + before_filter :find_commit + + def create + if Subscribe.set_subscribe(@project, @commit, current_user.id, 1) + flash[:notice] = I18n.t("flash.subscribe.saved") + # TODO js + redirect_to commit_path(@project, @commit) + else + flash[:error] = I18n.t("flash.subscribe.saved_error") + redirect_to commit_path(@project, @commit) + end + end + + def destroy + Subscribe.set_subscribe(@project, @commit, current_user.id, 0) + flash[:notice] = t("flash.subscribe.destroyed") + redirect_to commit_path(@project, @commit) + end + + protected + + def find_commit + @commit = @project.git_repository.commit(params[:commit_id]) + end +end diff --git a/app/controllers/project_subscribes_controller.rb b/app/controllers/project_subscribes_controller.rb deleted file mode 100644 index 665a47086..000000000 --- a/app/controllers/project_subscribes_controller.rb +++ /dev/null @@ -1,24 +0,0 @@ -class ProjectSubscribesController < ApplicationController - before_filter :authenticate_user! - - load_and_authorize_resource :project - load_resource :subscribe - #load_and_authorize_resource :subscribe, :find_by => :user_id - - def create - @subscribe = @project.commit_comments_subscribes.build(:user_id => current_user.id) - if @subscribe.save - flash[:notice] = I18n.t("flash.subscribe.saved") - redirect_to @project - else - flash[:error] = I18n.t("flash.subscribe.saved_error") - redirect_to @project - end - end - - def destroy - @project.commit_comments_subscribes.where(:user_id => current_user.id).first.destroy # FIXME - flash[:notice] = t("flash.subscribe.destroyed") - redirect_to @project - end -end diff --git a/app/models/comment.rb b/app/models/comment.rb index 0353fc3f0..518e636f5 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -7,6 +7,7 @@ class Comment < ActiveRecord::Base after_create :invoke_helper, :if => "commentable_type == 'Grit::Commit'" after_create :subscribe_on_reply + after_create :subscribe_users, :if => "commentable_type == 'Grit::Commit'" after_create {|comment| Subscribe.new_comment_notification(comment)} def helper @@ -22,10 +23,18 @@ class Comment < ActiveRecord::Base def subscribe_on_reply self.commentable.subscribes.create(:user_id => self.user_id) if self.commentable_type == 'Issue' && !self.commentable.subscribes.exists?(:user_id => self.user_id) - self.project.commit_comments_subscribes.create(:user_id => self.user_id) if self.commentable_type == 'Grit::Commit' && !self.project.commit_comments_subscribes.exists?(:user_id => self.user_id) + Subscribe.subscribe_user_to_commit(self, self.user_id) if self.commentable_type == 'Grit::Commit' end def invoke_helper self.helper end + + def subscribe_users + recipients = self.project.relations.by_role('admin').where(:object_type => 'User').map { |rel| rel.read_attribute(:object_id) } + committer = User.where(:email => self.commentable.committer.email).first + recipients = recipients | [committer.id] if committer + recipients = recipients | [self.project.owner_id] if self.project.owner_type == 'User' + recipients.each {|user| Subscribe.subscribe_user_to_commit(self, user)} + end end diff --git a/app/models/issue.rb b/app/models/issue.rb index efeffbf13..45d417d20 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -11,7 +11,9 @@ class Issue < ActiveRecord::Base #'WHERE comments.commentable_id = \'#{self.id}\' ' + #' AND comments.commentable_type = \'#{self.class.name}\' ' + #'ORDER BY comments.created_at' - has_many :subscribes, :as => :subscribeable + has_many :subscribes, :as => :subscribeable, + :finder_sql => proc { "subscribes.subscribeable_id = '#{self.id}' " + + " AND subscribes.subscribeable_type = '#{self.class.name}'"} validates :title, :body, :project_id, :presence => true diff --git a/app/models/project.rb b/app/models/project.rb index cc66415e6..c7b827ebe 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -14,7 +14,6 @@ class Project < ActiveRecord::Base has_many :relations, :as => :target, :dependent => :destroy has_many :collaborators, :through => :relations, :source => :object, :source_type => 'User' has_many :groups, :through => :relations, :source => :object, :source_type => 'Group' - has_many :commit_comments_subscribes, :as => :subscribeable, :class_name => 'Subscribe' validates :name, :uniqueness => {:scope => [:owner_id, :owner_type]}, :presence => true, :format => { :with => /^[a-zA-Z0-9_\-\+\.]+$/ } validates :owner, :presence => true @@ -31,7 +30,6 @@ class Project < ActiveRecord::Base after_create :attach_to_personal_repository after_create :create_git_repo - after_create :subscribe_owner after_destroy :destroy_git_repo # after_rollback lambda { destroy_git_repo rescue true if new_record? } @@ -161,7 +159,4 @@ class Project < ActiveRecord::Base FileUtils.rm_rf path end - def subscribe_owner - Subscribe.subscribe_user(self, self.owner.id) - end end diff --git a/app/models/relation.rb b/app/models/relation.rb index d2e35bc23..a08ddf6a9 100644 --- a/app/models/relation.rb +++ b/app/models/relation.rb @@ -12,8 +12,6 @@ class Relation < ActiveRecord::Base scope :by_target, lambda {|tar| {:conditions => ['target_id = ? AND target_type = ?', tar.id, tar.class.to_s]}} scope :by_role, lambda {|role| {:conditions => ['role = ?', role]}} - after_create :subscribe_project_admin, :if => "role == 'admin' && object_id == 'User' && targer_type == 'Project'" - def self.create_with_role(object, target, role) r = new r.object = object @@ -26,8 +24,4 @@ class Relation < ActiveRecord::Base def add_default_role self.role = ROLES.first if role.nil? || role.empty? end - - def subscribe_project_admin - Subscribe.subscribe_user(self.target_id, self.object_id) - end end diff --git a/app/models/subscribe.rb b/app/models/subscribe.rb index 381feaf6c..ca31ce497 100644 --- a/app/models/subscribe.rb +++ b/app/models/subscribe.rb @@ -1,31 +1,49 @@ class Subscribe < ActiveRecord::Base belongs_to :subscribeable, :polymorphic => true belongs_to :user + belongs_to :project + + validates :status, :inclusion => {:in => 0..1} + + scope :subscribed, where(:status => 1) + scope :unsubscribed, where(:status => 0) def self.new_comment_notification(comment) commentable_class = comment.commentable.class subscribes = comment.commentable.subscribes if commentable_class == Issue if commentable_class == Grit::Commit - subscribes = comment.project.commit_comments_subscribes(true) # FIXME (true) for rspec - committer = User.where(:email => comment.commentable.committer.email).first - UserMailer.delay.new_comment_notification(comment, committer) if committer && !comment.own_comment?(committer) && committer.notifier.new_comment_commit_owner && !committer.notifier.can_notify && subscribes.where(:user_id => committer).empty? + subscribes = Subscribe.where(:subscribeable_id => comment.commentable.id, :subscribeable_type => comment.commentable.class.name.to_s, :project_id => comment.project).subscribed(true) # FIXME (true) for rspec end subscribes.each do |subscribe| user = subscribe.user next if comment.own_comment?(user) || !user.notifier.can_notify UserMailer.delay.new_comment_notification(comment, user) if commentable_class == Issue && user.notifier.new_comment_reply - UserMailer.delay.new_comment_notification(comment, user) if commentable_class == Grit::Commit && Subscribe.send_notification_for_commit_comment?(subscribe.subscribeable, user, comment) + UserMailer.delay.new_comment_notification(comment, user) if commentable_class == Grit::Commit end end - def self.subscribe_user(project_id, user_id) - list = Project.find(project_id).commit_comments_subscribes - list.create(:user_id => user_id) unless list.exists?(:user_id => user_id) + def self.subscribe_user_to_commit(comment, user_id) + subscribe = Subscribe.where(:subscribeable_id => comment.commentable.id, :subscribeable_type => comment.commentable.class.name, :project_id => comment.project).unsubscribed.first + subscribe.update_attribute(:status, 1) if subscribe + Subscribe.create(:subscribeable_id => comment.commentable.id, :subscribeable_type => comment.commentable.class.name.to_s, :user_id => user_id, :project_id => comment.project, :status => 1) unless subscribe end - def self.send_notification_for_commit_comment?(project, user, comment) + def self.subscribed_for_commit?(project, user, commentable) is_owner = (project.owner_id == user.id) - is_commentor = (project.commit_comments_subscribes.exists?(:user_id => user.id)) - (is_owner && user.notifier.new_comment_commit_repo_owner) or (is_commentor && user.notifier.new_comment_commit_commentor) + is_commentor = (Comment.where(:commentable_type => commentable.class.name, :commentable_id => commentable.id).exists?(:user_id => user.id)) + is_committer = (user.email == commentable.committer.email) + (is_owner && user.notifier.new_comment_commit_repo_owner) or (is_commentor && user.notifier.new_comment_commit_commentor) or (is_committer && committer.notifier.new_comment_commit_owner) + end + + def self.set_subscribe(project, commit, user, status) + # FIXME maybe? + subscribe = Subscribe.where(:subscribeable_id => commit.id, :subscribeable_type => commit.class.name.to_s, + :user_id => user, :project_id => project).first + if subscribe + subscribe.update_attribute(:status, status) + else + Subscribe.create(:subscribeable_id => commit.id, :subscribeable_type => commit.class.name.to_s, + :user_id => user, :project_id => project, :status => status) + end end end diff --git a/app/views/git/commits/show.html.haml b/app/views/git/commits/show.html.haml index 8f3ec67f8..90131adb8 100644 --- a/app/views/git/commits/show.html.haml +++ b/app/views/git/commits/show.html.haml @@ -28,3 +28,12 @@ - content_for :sidebar, render(:partial => 'git/shared/sidebar') = render :partial => "comments/list", :locals => {:list => Project.commit_comments(@commit, @project), :project => @project, :commentable => @commit} +%p + %b + = t('layout.issues.subscribe') + \: + - subscribe = Subscribe.where(:subscribeable_id => @commit.id, :subscribeable_type => @commit.class.name.to_s, :project_id => @project, :user_id => current_user.id).first + - if subscribe.try(:status) == 1 or (subscribe.nil? && Subscribe.subscribed_for_commit?(@project, current_user, @commit)) + = link_to t('layout.issues.unsubscribe_btn'), unsubscribe_commit_path(@project, @commit), :method => :delete + - else + = link_to t('layout.issues.subscribe_btn'), subscribe_commit_path(@project, @commit), :method => :post diff --git a/app/views/projects/show.html.haml b/app/views/projects/show.html.haml index 689a5272f..6dfab842c 100644 --- a/app/views/projects/show.html.haml +++ b/app/views/projects/show.html.haml @@ -30,14 +30,6 @@ = t("activerecord.attributes.project.repository") \: = git_repo_url @project.git_repo_name - %p - %b - = t('layout.issues.subscribe') - \: - - if @project.commit_comments_subscribes.exists? :user_id => current_user.id - = link_to t('layout.issues.unsubscribe_btn'), project_subscribe_path(@project, current_user.id), :method => :delete - - else - = link_to t('layout.issues.subscribe_btn'), project_subscribes_path(@project), :method => :post .wat-cf = link_to image_tag("web-app-theme/icons/application_edit.png", :alt => t("layout.edit")) + " " + t("layout.edit"), edit_project_path(@project), :class => "button" if can? :update, @project = link_to image_tag("web-app-theme/icons/cross.png", :alt => t("layout.delete")) + " " + t("layout.delete"), project_path(@project), :method => "delete", :class => "button", :confirm => t("layout.projects.confirm_delete") if can? :destroy, @project diff --git a/config/routes.rb b/config/routes.rb index d36e2b7e2..4ee716fe8 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -88,7 +88,6 @@ Rosa::Application.routes.draw do end resource :repo, :controller => "git/repositories", :only => [:show] resources :build_lists, :only => [:index, :new, :create] - resources :subscribes, :only => [:create, :destroy], :controller => 'project_subscribes' resources :collaborators, :only => [:index, :edit, :update, :add] do collection do @@ -156,6 +155,9 @@ Rosa::Application.routes.draw do match '/projects/:project_id/git/commit/:commit_id/comments/:id(.:format)', :controller => "comments", :action => :update, :as => :project_commit_comment, :via => :put match '/projects/:project_id/git/commit/:commit_id/comments/:id(.:format)', :controller => "comments", :action => :destroy, :via => :delete match '/projects/:project_id/git/commit/:commit_id/comments(.:format)', :controller => "comments", :action => :create, :as => :project_commit_comments, :via => :post + # Commits subscribe + match '/projects/:project_id/git/commit/:commit_id/subscribe', :controller => "commit_subscribes", :action => :create, :defaults => { :format => :html }, :as => :subscribe_commit, :via => :post + match '/projects/:project_id/git/commit/:commit_id/unsubscribe', :controller => "commit_subscribes", :action => :destroy, :defaults => { :format => :html }, :as => :unsubscribe_commit, :via => :delete # Blobs match '/projects/:project_id/git/blob/:treeish/*path', :controller => "git/blobs", :action => :show, :treeish => /[0-9a-zA-Z_.\-]*/, :defaults => { :treeish => :master }, :as => :blob match '/projects/:project_id/git/commit/blob/:commit_hash/*path', :controller => "git/blobs", :action => :show, :project_name => /[0-9a-zA-Z_.\-]*/, :as => :blob_commit diff --git a/db/migrate/20120123120400_add_status_to_subscribe.rb b/db/migrate/20120123120400_add_status_to_subscribe.rb new file mode 100644 index 000000000..27fef24af --- /dev/null +++ b/db/migrate/20120123120400_add_status_to_subscribe.rb @@ -0,0 +1,9 @@ +class AddStatusToSubscribe < ActiveRecord::Migration + def self.up + add_column :subscribes, :status, :integer, :default => 1 + end + + def self.down + remove_column :subscribes, :status + end +end diff --git a/db/migrate/20120123134616_add_project_to_subscribe.rb b/db/migrate/20120123134616_add_project_to_subscribe.rb new file mode 100644 index 000000000..f1db12992 --- /dev/null +++ b/db/migrate/20120123134616_add_project_to_subscribe.rb @@ -0,0 +1,9 @@ +class AddProjectToSubscribe < ActiveRecord::Migration + def self.up + add_column :subscribes, :project_id, :integer + end + + def self.down + remove_column :subscribes, :project_id + end +end diff --git a/db/migrate/20120123161250_change_subscribeable_to_string.rb b/db/migrate/20120123161250_change_subscribeable_to_string.rb new file mode 100644 index 000000000..fcee398de --- /dev/null +++ b/db/migrate/20120123161250_change_subscribeable_to_string.rb @@ -0,0 +1,9 @@ +class ChangeSubscribeableToString < ActiveRecord::Migration + def self.up + change_column :subscribes, :subscribeable_id, :string + end + + def self.down + change_column :subscribes, :subscribeable_id, :integer + end +end diff --git a/db/schema.rb b/db/schema.rb index 86e064a69..19d47b0c9 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended to check this file into your version control system. -ActiveRecord::Schema.define(:version => 20120118173141) do +ActiveRecord::Schema.define(:version => 20120123161250) do create_table "arches", :force => true do |t| t.string "name", :null => false @@ -304,13 +304,25 @@ ActiveRecord::Schema.define(:version => 20120118173141) do end create_table "subscribes", :force => true do |t| - t.integer "subscribeable_id" + t.string "subscribeable_id" t.string "subscribeable_type" t.integer "user_id" t.datetime "created_at" t.datetime "updated_at" + t.integer "status", :default => 1 + t.integer "project_id" end + create_table "user_emails", :force => true do |t| + t.integer "user_id", :null => false + t.string "email", :null => false + t.datetime "created_at" + t.datetime "updated_at" + end + + add_index "user_emails", ["email"], :name => "index_user_emails_on_email" + add_index "user_emails", ["user_id"], :name => "index_user_emails_on_user_id" + create_table "users", :force => true do |t| t.string "name" t.string "email", :default => "", :null => false From 6582a9afa5414b5d2852b197c41c6c2447bdf06c Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Tue, 24 Jan 2012 18:15:56 +0600 Subject: [PATCH 15/35] [refs #114] fix tests --- spec/models/comment_for_commit_spec.rb | 118 ++++++++++++++----------- 1 file changed, 64 insertions(+), 54 deletions(-) diff --git a/spec/models/comment_for_commit_spec.rb b/spec/models/comment_for_commit_spec.rb index 5fe0f3799..054a2c74d 100644 --- a/spec/models/comment_for_commit_spec.rb +++ b/spec/models/comment_for_commit_spec.rb @@ -55,7 +55,7 @@ describe Comment do @stranger = Factory(:user) set_comments_data_for_commit - @project.relations.create!(:object_type => 'User', :object_id => @user.id, :role => 'admin') + #~ #@project.relations.create!(:object_type => 'User', :object_id => @user.id, :role => 'admin') end it 'should create comment' do @@ -85,13 +85,12 @@ describe Comment do end context 'for disabled notify setting new_comment_commit_repo_owner' do - it 'should send an e-mail' do + it 'should not send an e-mail' do ActionMailer::Base.deliveries = [] @user.notifier.update_attribute :new_comment_commit_repo_owner, false comment = Comment.create(:user => @stranger, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) - ActionMailer::Base.deliveries.count.should == 1 # cache project.commit_comments_subscribes ... - ActionMailer::Base.deliveries.last.to.include?(@user.email).should == true + ActionMailer::Base.deliveries.count.should == 0 end end @@ -101,7 +100,7 @@ describe Comment do @user.notifier.update_attribute :new_comment_commit_owner, false comment = Comment.create(:user => @stranger, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) - ActionMailer::Base.deliveries.count.should == 1 # cache project.commit_comments_subscribes ... + ActionMailer::Base.deliveries.count.should == 1 ActionMailer::Base.deliveries.last.to.include?(@user.email).should == true end end @@ -112,7 +111,7 @@ describe Comment do @user.notifier.update_attribute :new_comment_commit_commentor, false comment = Comment.create(:user => @stranger, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) - ActionMailer::Base.deliveries.count.should == 1 # cache project.commit_comments_subscribes ... + ActionMailer::Base.deliveries.count.should == 1 ActionMailer::Base.deliveries.last.to.include?(@user.email).should == true end end @@ -125,14 +124,14 @@ describe Comment do @user.notifier.update_attribute :new_comment_commit_commentor, false comment = Comment.create(:user => @stranger, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) - ActionMailer::Base.deliveries.count.should == 0 # cache project.commit_comments_subscribes ... + ActionMailer::Base.deliveries.count.should == 0 end end - context 'for unsubscribe project' do + context 'for unsubscribe commit' do it 'should not send an e-mail' do ActionMailer::Base.deliveries = [] - @project.commit_comments_subscribes.where(:user_id => @user).first.destroy # FIXME + Subscribe.set_subscribe(@project, @commit, @user.id, Subscribe::OFF) comment = Comment.create(:user => @stranger, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) ActionMailer::Base.deliveries.count.should == 0 # cache project.commit_comments_subscribes ... @@ -194,8 +193,7 @@ describe Comment do @user.notifier.update_attribute :new_comment_commit_repo_owner, false comment = Comment.create(:user => @stranger, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) - ActionMailer::Base.deliveries.count.should == 1 # cache project.commit_comments_subscribes ... - ActionMailer::Base.deliveries.last.to.include?(@user.email).should == true + ActionMailer::Base.deliveries.count.should == 0 end end @@ -229,17 +227,17 @@ describe Comment do @user.notifier.update_attribute :new_comment_commit_commentor, false comment = Comment.create(:user => @stranger, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) - ActionMailer::Base.deliveries.count.should == 0 # cache project.commit_comments_subscribes ... + ActionMailer::Base.deliveries.count.should == 0 end end context 'for unsubscribe project' do it 'should not send an e-mail' do ActionMailer::Base.deliveries = [] - @project.commit_comments_subscribes.where(:user_id => @user).first.destroy # FIXME + Subscribe.set_subscribe(@project, @commit, @user.id, Subscribe::OFF) comment = Comment.create(:user => @stranger, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) - ActionMailer::Base.deliveries.count.should == 0 # cache project.commit_comments_subscribes ... + ActionMailer::Base.deliveries.count.should == 0 end end @@ -316,44 +314,6 @@ describe Comment do ActionMailer::Base.deliveries.last.to.include?(@simple.email).should == true end - context 'for committer' do - it 'should send a one e-mail' do - ActionMailer::Base.deliveries = [] - @stranger.update_attribute :email, 'code@tpope.net' - comment = Comment.create(:user => @user, :body => 'hello!', :project => @project, - :commentable_type => @commit.class.name, :commentable_id => @commit.id) - ActionMailer::Base.deliveries.count.should == 1 - ActionMailer::Base.deliveries.last.to.include?(@stranger.email).should == true - end - - it 'should not send an e-mail for own comment' do - ActionMailer::Base.deliveries = [] - #@project.owner.notifier.update_attribute :can_notify, false - @stranger.update_attribute :email, 'code@tpope.net' - comment = Comment.create(:user => @stranger, :body => 'hello!', :project => @project, - :commentable_type => @commit.class.name, :commentable_id => @commit.id) - ActionMailer::Base.deliveries.count.should == 0 - end - - it 'should not send an e-mail if global notify off' do - ActionMailer::Base.deliveries = [] - @project.owner.notifier.update_attribute :can_notify, false - @stranger.update_attribute :email, 'code@tpope.net' - comment = Comment.create(:user => @user, :body => 'hello!', :project => @project, - :commentable_type => @commit.class.name, :commentable_id => @commit.id) - ActionMailer::Base.deliveries.count.should == 0 - end - - it 'should not send an e-mail if notify for my commits off' do - ActionMailer::Base.deliveries = [] - @stranger.notifier.update_attribute :new_comment_commit_owner, false - @stranger.update_attribute :email, 'code@tpope.net' - comment = Comment.create(:user => @user, :body => 'hello!', :project => @project, - :commentable_type => @commit.class.name, :commentable_id => @commit.id) - ActionMailer::Base.deliveries.count.should == 0 - end - end - it 'should send an e-mail when subscribed to project' do ActionMailer::Base.deliveries = [] @project.owner.notifier.update_attribute :can_notify, false @@ -361,7 +321,7 @@ describe Comment do @stranger.notifier.update_attribute :new_comment_commit_owner, false #@stranger.notifier.update_attribute :new_comment_commit_commentor, false - @project.commit_comments_subscribes.create(:user_id => @stranger.id) + Subscribe.set_subscribe(@project, @commit, @stranger.id, Subscribe::ON) comment = Comment.create(:user => @project.owner, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) ActionMailer::Base.deliveries.count.should == 1 @@ -371,11 +331,61 @@ describe Comment do it 'should not send an e-mail for own comment' do ActionMailer::Base.deliveries = [] #@project.owner.notifier.update_attribute :can_notify, false - @project.commit_comments_subscribes.create(:user_id => @stranger.id) + Subscribe.set_subscribe(@project, @commit, @stranger.id, Subscribe::ON) comment = Comment.create(:user => @owner, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) ActionMailer::Base.deliveries.count.should == 0 end end + + context 'for committer' do + it 'should send an e-mail' do + ActionMailer::Base.deliveries = [] + @stranger.update_attribute :email, 'code@tpope.net' + comment = Comment.create(:user => @user, :body => 'hello!', :project => @project, + :commentable_type => @commit.class.name, :commentable_id => @commit.id) + ActionMailer::Base.deliveries.count.should == 1 + ActionMailer::Base.deliveries.last.to.include?(@stranger.email).should == true + end + + it 'should send a one e-mail when subscribed to commit' do + ActionMailer::Base.deliveries = [] + Subscribe.set_subscribe(@project, @commit, @stranger.id, Subscribe::ON) + @stranger.update_attribute :email, 'code@tpope.net' + comment = Comment.create(:user => @user, :body => 'hello!', :project => @project, + :commentable_type => @commit.class.name, :commentable_id => @commit.id) + ActionMailer::Base.deliveries.count.should == 1 + ActionMailer::Base.deliveries.last.to.include?(@stranger.email).should == true + end + + it 'should not send an e-mail for own comment' do + ActionMailer::Base.deliveries = [] + #@project.owner.notifier.update_attribute :can_notify, false + @stranger.update_attribute :email, 'code@tpope.net' + comment = Comment.create(:user => @stranger, :body => 'hello!', :project => @project, + :commentable_type => @commit.class.name, :commentable_id => @commit.id) + ActionMailer::Base.deliveries.count.should == 1 + ActionMailer::Base.deliveries.last.to.include?(@stranger.email).should == false + end + + it 'should not send an e-mail if global notify off' do + ActionMailer::Base.deliveries = [] + @project.owner.notifier.update_attribute :can_notify, false + @stranger.update_attribute :email, 'code@tpope.net' + @stranger.notifier.update_attribute :can_notify, false + comment = Comment.create(:user => @user, :body => 'hello!', :project => @project, + :commentable_type => @commit.class.name, :commentable_id => @commit.id) + ActionMailer::Base.deliveries.count.should == 0 + end + + it 'should not send an e-mail if notify for my commits off' do + ActionMailer::Base.deliveries = [] + @stranger.notifier.update_attribute :new_comment_commit_owner, false + @stranger.update_attribute :email, 'code@tpope.net' + comment = Comment.create(:user => @user, :body => 'hello!', :project => @project, + :commentable_type => @commit.class.name, :commentable_id => @commit.id) + ActionMailer::Base.deliveries.count.should == 0 + end + end end end \ No newline at end of file From 6df16404851d2ad132a860dfc109149d4fff516f Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Tue, 24 Jan 2012 18:18:39 +0600 Subject: [PATCH 16/35] [refs #114] fix logics --- .../commit_subscribes_controller.rb | 5 +- app/models/comment.rb | 3 +- app/models/issue.rb | 10 +--- app/models/subscribe.rb | 54 ++++++++++++------- app/views/git/commits/show.html.haml | 2 +- db/schema.rb | 13 +++++ 6 files changed, 54 insertions(+), 33 deletions(-) diff --git a/app/controllers/commit_subscribes_controller.rb b/app/controllers/commit_subscribes_controller.rb index bc6596159..cd22dfd87 100644 --- a/app/controllers/commit_subscribes_controller.rb +++ b/app/controllers/commit_subscribes_controller.rb @@ -1,13 +1,12 @@ class CommitSubscribesController < ApplicationController before_filter :authenticate_user! - load_resource :subscribe load_and_authorize_resource :project before_filter :find_commit def create - if Subscribe.set_subscribe(@project, @commit, current_user.id, 1) + if Subscribe.set_subscribe(@project, @commit, current_user.id, Subscribe::ON) flash[:notice] = I18n.t("flash.subscribe.saved") # TODO js redirect_to commit_path(@project, @commit) @@ -18,7 +17,7 @@ class CommitSubscribesController < ApplicationController end def destroy - Subscribe.set_subscribe(@project, @commit, current_user.id, 0) + Subscribe.set_subscribe(@project, @commit, current_user.id, Subscribe::OFF) flash[:notice] = t("flash.subscribe.destroyed") redirect_to commit_path(@project, @commit) end diff --git a/app/models/comment.rb b/app/models/comment.rb index 518e636f5..9bb9c4f8b 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -18,12 +18,11 @@ class Comment < ActiveRecord::Base user_id == user.id end - protected def subscribe_on_reply self.commentable.subscribes.create(:user_id => self.user_id) if self.commentable_type == 'Issue' && !self.commentable.subscribes.exists?(:user_id => self.user_id) - Subscribe.subscribe_user_to_commit(self, self.user_id) if self.commentable_type == 'Grit::Commit' + Subscribe.subscribe_user_to_commit(self, self.user.id) if self.commentable_type == 'Grit::Commit' end def invoke_helper diff --git a/app/models/issue.rb b/app/models/issue.rb index 45d417d20..4bfa290ce 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -5,15 +5,9 @@ class Issue < ActiveRecord::Base belongs_to :user has_many :comments, :as => :commentable, - :finder_sql => proc { "comments.commentable_id = '#{self.id}' " + - " AND comments.commentable_type = '#{self.class.name}'"} - #'SELECT comments.* FROM comments ' + - #'WHERE comments.commentable_id = \'#{self.id}\' ' + - #' AND comments.commentable_type = \'#{self.class.name}\' ' + - #'ORDER BY comments.created_at' + :finder_sql => proc { "comments.commentable_id = '#{self.id}' AND comments.commentable_type = '#{self.class.name}'"} has_many :subscribes, :as => :subscribeable, - :finder_sql => proc { "subscribes.subscribeable_id = '#{self.id}' " + - " AND subscribes.subscribeable_type = '#{self.class.name}'"} + :finder_sql => proc { "subscribes.subscribeable_id = '#{self.id}' AND subscribes.subscribeable_type = '#{self.class.name}'"} validates :title, :body, :project_id, :presence => true diff --git a/app/models/subscribe.rb b/app/models/subscribe.rb index ca31ce497..fe3a9b522 100644 --- a/app/models/subscribe.rb +++ b/app/models/subscribe.rb @@ -1,49 +1,65 @@ class Subscribe < ActiveRecord::Base + ON = 1 + OFF = 0 belongs_to :subscribeable, :polymorphic => true belongs_to :user belongs_to :project validates :status, :inclusion => {:in => 0..1} - scope :subscribed, where(:status => 1) - scope :unsubscribed, where(:status => 0) + scope :on, where(:status => ON) + scope :off, where(:status => OFF) + scope :finder_hack, order('') # FIXME .subscribes - error; .subscribes.finder_hack - success Oo + + def self.comment_subscribes(comment) + Subscribe.where(:subscribeable_id => comment.commentable.id, :subscribeable_type => comment.commentable.class.name.to_s, :project_id => comment.project) + end def self.new_comment_notification(comment) commentable_class = comment.commentable.class - subscribes = comment.commentable.subscribes if commentable_class == Issue - if commentable_class == Grit::Commit - subscribes = Subscribe.where(:subscribeable_id => comment.commentable.id, :subscribeable_type => comment.commentable.class.name.to_s, :project_id => comment.project).subscribed(true) # FIXME (true) for rspec - end - subscribes.each do |subscribe| - user = subscribe.user - next if comment.own_comment?(user) || !user.notifier.can_notify - UserMailer.delay.new_comment_notification(comment, user) if commentable_class == Issue && user.notifier.new_comment_reply - UserMailer.delay.new_comment_notification(comment, user) if commentable_class == Grit::Commit + Subscribe.new_comment_issue_notification(comment) if commentable_class == Issue + Subscribe.new_comment_commit_notification(comment) if commentable_class == Grit::Commit + end + + def self.new_comment_issue_notification(comment) + comment.commentable.subscribes.finder_hack.each do |subscribe| + next if comment.own_comment?(subscribe.user) || !subscribe.user.notifier.can_notify + UserMailer.delay.new_comment_notification(comment, subscribe.user) if subscribe.user.notifier.new_comment_reply end end - def self.subscribe_user_to_commit(comment, user_id) - subscribe = Subscribe.where(:subscribeable_id => comment.commentable.id, :subscribeable_type => comment.commentable.class.name, :project_id => comment.project).unsubscribed.first - subscribe.update_attribute(:status, 1) if subscribe - Subscribe.create(:subscribeable_id => comment.commentable.id, :subscribeable_type => comment.commentable.class.name.to_s, :user_id => user_id, :project_id => comment.project, :status => 1) unless subscribe + def self.new_comment_commit_notification(comment) + subscribes = Subscribe.comment_subscribes(comment).on(true) # FIXME (true) for rspec + subscribes.each do |subscribe| + next if comment.own_comment?(subscribe.user) || !subscribe.user.notifier.can_notify + UserMailer.delay.new_comment_notification(comment, subscribe.user) + end + end + + def self.subscribe_user_to_commit(comment, user) + Subscribe.set_subscribe(comment.project, comment.commentable, user, Subscribe::ON) if Subscribe.subscribed_for_commit?(comment.project, User.find(user), comment.commentable) end def self.subscribed_for_commit?(project, user, commentable) is_owner = (project.owner_id == user.id) is_commentor = (Comment.where(:commentable_type => commentable.class.name, :commentable_id => commentable.id).exists?(:user_id => user.id)) is_committer = (user.email == commentable.committer.email) - (is_owner && user.notifier.new_comment_commit_repo_owner) or (is_commentor && user.notifier.new_comment_commit_commentor) or (is_committer && committer.notifier.new_comment_commit_owner) + return false if Subscribe.where(:subscribeable_id => commentable.id, :subscribeable_type => commentable.class.name, + :user_id => user.id, :project_id => project.id, :status => Subscribe::OFF).first.present? + (is_owner && user.notifier.new_comment_commit_repo_owner) or + (is_commentor && user.notifier.new_comment_commit_commentor) or + (is_committer && user.notifier.new_comment_commit_owner) end def self.set_subscribe(project, commit, user, status) # FIXME maybe? - subscribe = Subscribe.where(:subscribeable_id => commit.id, :subscribeable_type => commit.class.name.to_s, + subscribe = Subscribe.where(:subscribeable_id => commit.id, :subscribeable_type => commit.class.name, :user_id => user, :project_id => project).first if subscribe subscribe.update_attribute(:status, status) else - Subscribe.create(:subscribeable_id => commit.id, :subscribeable_type => commit.class.name.to_s, - :user_id => user, :project_id => project, :status => status) + Subscribe.create(:subscribeable_id => commit.id, :subscribeable_type => commit.class.name, + :user_id => user, :project_id => project.id, :status => status) end end end diff --git a/app/views/git/commits/show.html.haml b/app/views/git/commits/show.html.haml index 90131adb8..85feb2c47 100644 --- a/app/views/git/commits/show.html.haml +++ b/app/views/git/commits/show.html.haml @@ -33,7 +33,7 @@ = t('layout.issues.subscribe') \: - subscribe = Subscribe.where(:subscribeable_id => @commit.id, :subscribeable_type => @commit.class.name.to_s, :project_id => @project, :user_id => current_user.id).first - - if subscribe.try(:status) == 1 or (subscribe.nil? && Subscribe.subscribed_for_commit?(@project, current_user, @commit)) + - if subscribe.try(:status) == Subscribe::ON or (subscribe.nil? && Subscribe.subscribed_for_commit?(@project, current_user, @commit)) = link_to t('layout.issues.unsubscribe_btn'), unsubscribe_commit_path(@project, @commit), :method => :delete - else = link_to t('layout.issues.subscribe_btn'), subscribe_commit_path(@project, @commit), :method => :post diff --git a/db/schema.rb b/db/schema.rb index 19d47b0c9..a4692cd75 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -278,6 +278,19 @@ ActiveRecord::Schema.define(:version => 20120123161250) do t.datetime "updated_at" end + create_table "role_lines", :force => true do |t| + t.integer "role_id" + t.integer "relation_id" + t.datetime "created_at" + t.datetime "updated_at" + end + + create_table "roles", :force => true do |t| + t.string "name" + t.datetime "created_at" + t.datetime "updated_at" + end + create_table "rpms", :force => true do |t| t.string "name", :null => false t.integer "arch_id", :null => false From 61be317b21ce1ab1a8fd7bacdf397c3ed5012d3b Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Wed, 25 Jan 2012 01:00:51 +0600 Subject: [PATCH 17/35] [refs #114] i18n --- app/controllers/commit_subscribes_controller.rb | 4 ++-- app/mailers/user_mailer.rb | 2 +- app/models/subscribe.rb | 2 +- app/views/git/commits/show.html.haml | 4 ++-- .../user_mailer/new_comment_notification.ru.haml | 10 +++++----- config/locales/devise.en.yml | 1 + config/locales/en.yml | 16 +++++++++++++--- config/locales/ru.yml | 9 +++++++-- spec/models/comment_for_commit_spec.rb | 11 +++++++++++ 9 files changed, 43 insertions(+), 16 deletions(-) diff --git a/app/controllers/commit_subscribes_controller.rb b/app/controllers/commit_subscribes_controller.rb index cd22dfd87..5fbf50eac 100644 --- a/app/controllers/commit_subscribes_controller.rb +++ b/app/controllers/commit_subscribes_controller.rb @@ -7,7 +7,7 @@ class CommitSubscribesController < ApplicationController def create if Subscribe.set_subscribe(@project, @commit, current_user.id, Subscribe::ON) - flash[:notice] = I18n.t("flash.subscribe.saved") + flash[:notice] = I18n.t("flash.subscribe.commit.saved") # TODO js redirect_to commit_path(@project, @commit) else @@ -18,7 +18,7 @@ class CommitSubscribesController < ApplicationController def destroy Subscribe.set_subscribe(@project, @commit, current_user.id, Subscribe::OFF) - flash[:notice] = t("flash.subscribe.destroyed") + flash[:notice] = t("flash.subscribe.commit.destroyed") redirect_to commit_path(@project, @commit) end diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb index 9be195dd4..2923e62fd 100644 --- a/app/mailers/user_mailer.rb +++ b/app/mailers/user_mailer.rb @@ -13,7 +13,7 @@ class UserMailer < ActionMailer::Base def new_comment_notification(comment, user) @user = user @comment = comment - mail(:to => user.email, :subject => I18n.t("notifications.subjects.new_comment_notification")) do |format| + mail(:to => user.email, :subject => I18n.t("notifications.subjects.new_#{comment.commentable.class == Grit::Commit ? 'commit_' : ''}comment_notification")) do |format| format.html end end diff --git a/app/models/subscribe.rb b/app/models/subscribe.rb index fe3a9b522..d9d9cfe47 100644 --- a/app/models/subscribe.rb +++ b/app/models/subscribe.rb @@ -12,7 +12,7 @@ class Subscribe < ActiveRecord::Base scope :finder_hack, order('') # FIXME .subscribes - error; .subscribes.finder_hack - success Oo def self.comment_subscribes(comment) - Subscribe.where(:subscribeable_id => comment.commentable.id, :subscribeable_type => comment.commentable.class.name.to_s, :project_id => comment.project) + Subscribe.where(:subscribeable_id => comment.commentable.id, :subscribeable_type => comment.commentable.class.name, :project_id => comment.project) end def self.new_comment_notification(comment) diff --git a/app/views/git/commits/show.html.haml b/app/views/git/commits/show.html.haml index 85feb2c47..a9fd66404 100644 --- a/app/views/git/commits/show.html.haml +++ b/app/views/git/commits/show.html.haml @@ -34,6 +34,6 @@ \: - subscribe = Subscribe.where(:subscribeable_id => @commit.id, :subscribeable_type => @commit.class.name.to_s, :project_id => @project, :user_id => current_user.id).first - if subscribe.try(:status) == Subscribe::ON or (subscribe.nil? && Subscribe.subscribed_for_commit?(@project, current_user, @commit)) - = link_to t('layout.issues.unsubscribe_btn'), unsubscribe_commit_path(@project, @commit), :method => :delete + = link_to t('layout.commits.unsubscribe_btn'), unsubscribe_commit_path(@project, @commit), :method => :delete - else - = link_to t('layout.issues.subscribe_btn'), subscribe_commit_path(@project, @commit), :method => :post + = link_to t('layout.commits.subscribe_btn'), subscribe_commit_path(@project, @commit), :method => :post diff --git a/app/views/user_mailer/new_comment_notification.ru.haml b/app/views/user_mailer/new_comment_notification.ru.haml index c0f4e27e3..eff4915be 100644 --- a/app/views/user_mailer/new_comment_notification.ru.haml +++ b/app/views/user_mailer/new_comment_notification.ru.haml @@ -1,14 +1,14 @@ -%p== Hello, #{@user.name}. +%p== Здравствуйте, #{@user.name}. - if @comment.commentable.class == Issue - link = link_to @comment.commentable.title, [@comment.commentable.project, @comment.commentable] - - object = 'issue' + - object = 'задаче' - elsif @comment.commentable.class == Grit::Commit - link = link_to @comment.commentable.message, commit_path(@comment.project, @comment.commentable_id]) - - object = 'commit' -%p User #{ link_to @comment.user.uname, user_path(@comment.user)} added new comment to #{object} #{link}. + - object = 'коммиту' +%p #{ link_to @comment.user.uname, user_path(@comment.user)} добавил комментарий к #{object} #{link}. %p "#{ @comment.body }" -%p== Support Team «ROSA Build System» +%p== Команда поддержки «ROSA Build System» diff --git a/config/locales/devise.en.yml b/config/locales/devise.en.yml index 5e4e43321..5b5dfb3b5 100644 --- a/config/locales/devise.en.yml +++ b/config/locales/devise.en.yml @@ -27,6 +27,7 @@ en: signed_up: 'You have signed up successfully. If enabled, a confirmation was sent to your e-mail.' updated: 'You updated your account successfully.' destroyed: 'Bye! Your account was successfully cancelled. We hope to see you again soon.' + sign_up_header: 'Signup' unlocks: send_instructions: 'You will receive an email with instructions about how to unlock your account in a few minutes.' unlocked: 'Your account was successfully unlocked. You are now signed in.' diff --git a/config/locales/en.yml b/config/locales/en.yml index 4fa4edbc8..a92b84bab 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1,9 +1,9 @@ en: will_paginate: - previous_label: ‹ Previous!!! + previous_label: ‹ Previous!!! next_label: Next › page_gap: ... - + datatables: previous_label: ‹ Prev. next_label: Next ›b r @@ -40,6 +40,7 @@ en: not_access: Access denied! owner: Owner confirm: Sure? + back: Back settings: notifier: Notifier setting notifiers: @@ -126,6 +127,10 @@ en: subscribe_btn: Subscribe unsubscribe_btn: Unsubscribe + commits: + subscribe_btn: Subscribe to commit + unsubscribe_btn: Unsubscribe from commit + comments: confirm_delete: Are you sure to delete the comment? new_header: New comment @@ -365,7 +370,11 @@ en: subscribe: saved: Subscription on notifications for this task is created + saved_error: Subscription create error destroyed: Subscription on notifications for this task is cleaned + commit: + saved: Subscription on notifications for this commit is created + destroyed: Subscription on notifications for this commit is cleaned exception_message: Access violation to this page! @@ -690,4 +699,5 @@ en: new_comment_notification: New comment to your task new_issue_notification: New task added to project new_user_notification: Registered on project «%{ project_name }» - issue_assign_notification: New task assigned \ No newline at end of file + issue_assign_notification: New task assigned + new_commit_comment_notification: New comment to commit diff --git a/config/locales/ru.yml b/config/locales/ru.yml index a4786dee7..ed32edbf5 100644 --- a/config/locales/ru.yml +++ b/config/locales/ru.yml @@ -365,7 +365,11 @@ ru: subscribe: saved: Вы подписаны на оповещения для этой задачи + saved_error: При создании подписки произошла ошибка destroyed: Подписка на оповещения для этой задачи убрана + commit: + saved: Вы подписаны на оповещения для этого коммита + destroyed: Подписка на оповещения для этого коммита убрана exception_message: У Вас нет доступа к этой странице! @@ -680,17 +684,18 @@ ru: status: Статус version: Версия build_list: Сборочный лист - + download: name: Название version: Версия distro: Дистрибутив platform: Архитектура counter: Закачки - + notifications: subjects: new_comment_notification: Новый комментарий к Вашей задаче new_issue_notification: Новая задача добавлена к проекту new_user_notification: Регистрация на проекте «%{ project_name }» issue_assign_notification: Вам назначили задачу + new_commit_comment_notification: Новый комментарий к коммиту diff --git a/spec/models/comment_for_commit_spec.rb b/spec/models/comment_for_commit_spec.rb index 054a2c74d..fa731fb73 100644 --- a/spec/models/comment_for_commit_spec.rb +++ b/spec/models/comment_for_commit_spec.rb @@ -348,6 +348,17 @@ describe Comment do ActionMailer::Base.deliveries.last.to.include?(@stranger.email).should == true end + it 'should send an e-mail with user locale' do + ActionMailer::Base.deliveries = [] + @stranger.update_attribute :email, 'code@tpope.net' + @stranger.update_attribute :language, 'ru' + comment = Comment.create(:user => @user, :body => 'hello!', :project => @project, + :commentable_type => @commit.class.name, :commentable_id => @commit.id) + ActionMailer::Base.deliveries.count.should == 1 + ActionMailer::Base.deliveries.last.to.include?(@stranger.email).should == true + ActionMailer::Base.deliveries.last.subject.include?('Новый комментарий к коммиту').should == true + end + it 'should send a one e-mail when subscribed to commit' do ActionMailer::Base.deliveries = [] Subscribe.set_subscribe(@project, @commit, @stranger.id, Subscribe::ON) From b7c2b4d35bcb70dbb82f7c0b364956e4fd186444 Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Wed, 25 Jan 2012 13:21:41 +0600 Subject: [PATCH 18/35] [refs #114] fix email bug --- app/views/user_mailer/new_comment_notification.ru.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/user_mailer/new_comment_notification.ru.haml b/app/views/user_mailer/new_comment_notification.ru.haml index eff4915be..d1c0734dd 100644 --- a/app/views/user_mailer/new_comment_notification.ru.haml +++ b/app/views/user_mailer/new_comment_notification.ru.haml @@ -4,7 +4,7 @@ - link = link_to @comment.commentable.title, [@comment.commentable.project, @comment.commentable] - object = 'задаче' - elsif @comment.commentable.class == Grit::Commit - - link = link_to @comment.commentable.message, commit_path(@comment.project, @comment.commentable_id]) + - link = link_to @comment.commentable.message, commit_path(@comment.project, @comment.commentable_id) - object = 'коммиту' %p #{ link_to @comment.user.uname, user_path(@comment.user)} добавил комментарий к #{object} #{link}. From 82339dcb344640c437911e0a8867979ea8d9eb6a Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Wed, 25 Jan 2012 14:28:00 +0600 Subject: [PATCH 19/35] send email in user's language --- app/mailers/user_mailer.rb | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb index 2923e62fd..2c25b14a0 100644 --- a/app/mailers/user_mailer.rb +++ b/app/mailers/user_mailer.rb @@ -13,24 +13,41 @@ class UserMailer < ActionMailer::Base def new_comment_notification(comment, user) @user = user @comment = comment + set_locale mail(:to => user.email, :subject => I18n.t("notifications.subjects.new_#{comment.commentable.class == Grit::Commit ? 'commit_' : ''}comment_notification")) do |format| format.html end + ensure reset_locale end def new_issue_notification(issue, user) @user = user @issue = issue + set_locale mail(:to => user.email, :subject => I18n.t("notifications.subjects.new_issue_notification")) do |format| format.html end + ensure reset_locale end def issue_assign_notification(issue, user) @user = user @issue = issue + set_locale mail(:to => user.email, :subject => I18n.t("notifications.subjects.issue_assign_notification")) do |format| format.html end + ensure reset_locale end + + protected + + def set_locale + @initial_locale, I18n.locale = I18n.locale, @user.language + end + + def reset_locale + I18n.locale = @initial_locale + end + end From 4cca8f7bb1bb881531a41a46925b280fc0f8585d Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Wed, 25 Jan 2012 23:33:26 +0600 Subject: [PATCH 20/35] [refs #114] some refactoring --- .../commit_subscribes_controller.rb | 4 ++-- app/models/comment.rb | 21 +++++++--------- app/models/project.rb | 4 ++++ app/models/subscribe.rb | 24 ++++++++----------- app/models/user.rb | 5 ++-- app/views/git/commits/show.html.haml | 4 ++-- spec/models/comment_for_commit_spec.rb | 10 ++++---- 7 files changed, 35 insertions(+), 37 deletions(-) diff --git a/app/controllers/commit_subscribes_controller.rb b/app/controllers/commit_subscribes_controller.rb index 5fbf50eac..e335fdff6 100644 --- a/app/controllers/commit_subscribes_controller.rb +++ b/app/controllers/commit_subscribes_controller.rb @@ -6,7 +6,7 @@ class CommitSubscribesController < ApplicationController before_filter :find_commit def create - if Subscribe.set_subscribe(@project, @commit, current_user.id, Subscribe::ON) + if Subscribe.set_subscribe_to_commit(@project, @commit, current_user.id, Subscribe::ON) flash[:notice] = I18n.t("flash.subscribe.commit.saved") # TODO js redirect_to commit_path(@project, @commit) @@ -17,7 +17,7 @@ class CommitSubscribesController < ApplicationController end def destroy - Subscribe.set_subscribe(@project, @commit, current_user.id, Subscribe::OFF) + Subscribe.set_subscribe_to_commit(@project, @commit, current_user.id, Subscribe::OFF) flash[:notice] = t("flash.subscribe.commit.destroyed") redirect_to commit_path(@project, @commit) end diff --git a/app/models/comment.rb b/app/models/comment.rb index 9bb9c4f8b..d59d670e8 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -6,8 +6,7 @@ class Comment < ActiveRecord::Base validates :body, :user_id, :commentable_id, :commentable_type, :presence => true after_create :invoke_helper, :if => "commentable_type == 'Grit::Commit'" - after_create :subscribe_on_reply - after_create :subscribe_users, :if => "commentable_type == 'Grit::Commit'" + after_create :subscribe_users after_create {|comment| Subscribe.new_comment_notification(comment)} def helper @@ -20,20 +19,18 @@ class Comment < ActiveRecord::Base protected - def subscribe_on_reply - self.commentable.subscribes.create(:user_id => self.user_id) if self.commentable_type == 'Issue' && !self.commentable.subscribes.exists?(:user_id => self.user_id) - Subscribe.subscribe_user_to_commit(self, self.user.id) if self.commentable_type == 'Grit::Commit' - end - def invoke_helper self.helper end def subscribe_users - recipients = self.project.relations.by_role('admin').where(:object_type => 'User').map { |rel| rel.read_attribute(:object_id) } - committer = User.where(:email => self.commentable.committer.email).first - recipients = recipients | [committer.id] if committer - recipients = recipients | [self.project.owner_id] if self.project.owner_type == 'User' - recipients.each {|user| Subscribe.subscribe_user_to_commit(self, user)} + if self.commentable.class == Issue + self.commentable.subscribes.create(:user_id => self.user_id) if !self.commentable.subscribes.exists?(:user_id => self.user_id) + elsif self.commentable.class == Grit::Commit + recipients = self.project.relations.by_role('admin').where(:object_type => 'User').map &:object # admins + recipients << self.user << User.where(:email => self.commentable.committer.email).first # commentor and committer + recipients << self.project.owner if self.project.owner_type == 'User' # project owner + recipients.compact.uniq.each {|user| Subscribe.subscribe_user_to_commit(self, user.id)} + end end end diff --git a/app/models/project.rb b/app/models/project.rb index c7b827ebe..52aeb75f0 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -141,6 +141,10 @@ class Project < ActiveRecord::Base end end + def owner?(user) + owner_id == user.id + end + protected def build_path(dir) diff --git a/app/models/subscribe.rb b/app/models/subscribe.rb index d9d9cfe47..7ea9f612e 100644 --- a/app/models/subscribe.rb +++ b/app/models/subscribe.rb @@ -29,7 +29,7 @@ class Subscribe < ActiveRecord::Base end def self.new_comment_commit_notification(comment) - subscribes = Subscribe.comment_subscribes(comment).on(true) # FIXME (true) for rspec + subscribes = Subscribe.comment_subscribes(comment).on#(true) # FIXME (true) for rspec subscribes.each do |subscribe| next if comment.own_comment?(subscribe.user) || !subscribe.user.notifier.can_notify UserMailer.delay.new_comment_notification(comment, subscribe.user) @@ -37,29 +37,25 @@ class Subscribe < ActiveRecord::Base end def self.subscribe_user_to_commit(comment, user) - Subscribe.set_subscribe(comment.project, comment.commentable, user, Subscribe::ON) if Subscribe.subscribed_for_commit?(comment.project, User.find(user), comment.commentable) + Subscribe.set_subscribe_to_commit(comment.project, comment.commentable, user, Subscribe::ON) if Subscribe.subscribed_to_commit?(comment.project, User.find(user), comment.commentable) end - def self.subscribed_for_commit?(project, user, commentable) - is_owner = (project.owner_id == user.id) + def self.subscribed_to_commit?(project, user, commentable) is_commentor = (Comment.where(:commentable_type => commentable.class.name, :commentable_id => commentable.id).exists?(:user_id => user.id)) is_committer = (user.email == commentable.committer.email) - return false if Subscribe.where(:subscribeable_id => commentable.id, :subscribeable_type => commentable.class.name, - :user_id => user.id, :project_id => project.id, :status => Subscribe::OFF).first.present? - (is_owner && user.notifier.new_comment_commit_repo_owner) or + return false if user.subscribes.where(:subscribeable_id => commentable.id, :subscribeable_type => commentable.class.name, + :project_id => project.id, :status => Subscribe::OFF).first.present? + (project.owner?(user) && user.notifier.new_comment_commit_repo_owner) or (is_commentor && user.notifier.new_comment_commit_commentor) or (is_committer && user.notifier.new_comment_commit_owner) end - def self.set_subscribe(project, commit, user, status) - # FIXME maybe? - subscribe = Subscribe.where(:subscribeable_id => commit.id, :subscribeable_type => commit.class.name, - :user_id => user, :project_id => project).first - if subscribe + def self.set_subscribe_to_commit(project, commit, user, status) + options = {:subscribeable_id => commit.id, :subscribeable_type => commit.class.name, :user_id => user, :project_id => project.id} + if subscribe = Subscribe.where(options).first # FIXME maybe? subscribe.update_attribute(:status, status) else - Subscribe.create(:subscribeable_id => commit.id, :subscribeable_type => commit.class.name, - :user_id => user, :project_id => project.id, :status => status) + Subscribe.create(options.merge(:status => status)) end end end diff --git a/app/models/user.rb b/app/models/user.rb index 16a399caa..b0dd1de44 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -23,6 +23,7 @@ class User < ActiveRecord::Base has_many :projects, :through => :targets, :source => :target, :source_type => 'Project', :autosave => true has_many :platforms, :through => :targets, :source => :target, :source_type => 'Platform', :autosave => true has_many :repositories, :through => :targets, :source => :target, :source_type => 'Repository', :autosave => true + has_many :subscribes, :foreign_key => :user_id, :dependent => :destroy include Modules::Models::PersonalRepository @@ -41,7 +42,7 @@ class User < ActiveRecord::Base def admin? role == 'admin' end - + def guest? self.id.blank? # persisted? end @@ -82,7 +83,7 @@ class User < ActiveRecord::Base clean_up_passwords result end - + private def create_settings_notifier diff --git a/app/views/git/commits/show.html.haml b/app/views/git/commits/show.html.haml index a9fd66404..510f9a045 100644 --- a/app/views/git/commits/show.html.haml +++ b/app/views/git/commits/show.html.haml @@ -32,8 +32,8 @@ %b = t('layout.issues.subscribe') \: - - subscribe = Subscribe.where(:subscribeable_id => @commit.id, :subscribeable_type => @commit.class.name.to_s, :project_id => @project, :user_id => current_user.id).first - - if subscribe.try(:status) == Subscribe::ON or (subscribe.nil? && Subscribe.subscribed_for_commit?(@project, current_user, @commit)) + - subscribe = current_user.subscribes.where(:subscribeable_id => @commit.id, :subscribeable_type => @commit.class.name.to_s, :project_id => @project, :status => Subscribe::OFF).first + - if subscribe.nil? or Subscribe.subscribed_to_commit?(@project, current_user, @commit) = link_to t('layout.commits.unsubscribe_btn'), unsubscribe_commit_path(@project, @commit), :method => :delete - else = link_to t('layout.commits.subscribe_btn'), subscribe_commit_path(@project, @commit), :method => :post diff --git a/spec/models/comment_for_commit_spec.rb b/spec/models/comment_for_commit_spec.rb index fa731fb73..3b862751b 100644 --- a/spec/models/comment_for_commit_spec.rb +++ b/spec/models/comment_for_commit_spec.rb @@ -131,7 +131,7 @@ describe Comment do context 'for unsubscribe commit' do it 'should not send an e-mail' do ActionMailer::Base.deliveries = [] - Subscribe.set_subscribe(@project, @commit, @user.id, Subscribe::OFF) + Subscribe.set_subscribe_to_commit(@project, @commit, @user.id, Subscribe::OFF) comment = Comment.create(:user => @stranger, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) ActionMailer::Base.deliveries.count.should == 0 # cache project.commit_comments_subscribes ... @@ -234,7 +234,7 @@ describe Comment do context 'for unsubscribe project' do it 'should not send an e-mail' do ActionMailer::Base.deliveries = [] - Subscribe.set_subscribe(@project, @commit, @user.id, Subscribe::OFF) + Subscribe.set_subscribe_to_commit(@project, @commit, @user.id, Subscribe::OFF) comment = Comment.create(:user => @stranger, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) ActionMailer::Base.deliveries.count.should == 0 @@ -321,7 +321,7 @@ describe Comment do @stranger.notifier.update_attribute :new_comment_commit_owner, false #@stranger.notifier.update_attribute :new_comment_commit_commentor, false - Subscribe.set_subscribe(@project, @commit, @stranger.id, Subscribe::ON) + Subscribe.set_subscribe_to_commit(@project, @commit, @stranger.id, Subscribe::ON) comment = Comment.create(:user => @project.owner, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) ActionMailer::Base.deliveries.count.should == 1 @@ -331,7 +331,7 @@ describe Comment do it 'should not send an e-mail for own comment' do ActionMailer::Base.deliveries = [] #@project.owner.notifier.update_attribute :can_notify, false - Subscribe.set_subscribe(@project, @commit, @stranger.id, Subscribe::ON) + Subscribe.set_subscribe_to_commit(@project, @commit, @stranger.id, Subscribe::ON) comment = Comment.create(:user => @owner, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) ActionMailer::Base.deliveries.count.should == 0 @@ -361,7 +361,7 @@ describe Comment do it 'should send a one e-mail when subscribed to commit' do ActionMailer::Base.deliveries = [] - Subscribe.set_subscribe(@project, @commit, @stranger.id, Subscribe::ON) + Subscribe.set_subscribe_to_commit(@project, @commit, @stranger.id, Subscribe::ON) @stranger.update_attribute :email, 'code@tpope.net' comment = Comment.create(:user => @user, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) From 77420d2a8f42648608ae866066645df0ae920710 Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Thu, 26 Jan 2012 14:53:40 +0600 Subject: [PATCH 21/35] [refs #114] user emails --- app/controllers/user_emails_controller.rb | 27 +++++++++++++++++++ app/models/subscribe.rb | 2 +- app/models/user.rb | 10 ++++--- app/models/user_email.rb | 7 +++++ app/views/devise/registrations/edit.html.haml | 3 +++ app/views/users/emails.html.haml | 0 config/locales/en.yml | 7 ++--- config/locales/ru.yml | 5 ++-- config/routes.rb | 4 +++ .../20120123051330_create_user_emails.rb | 21 +++++++++++++++ db/schema.rb | 16 +++++++++-- spec/factories/user_emails.rb | 8 ++++++ spec/models/user_email_spec.rb | 5 ++++ 13 files changed, 104 insertions(+), 11 deletions(-) create mode 100644 app/controllers/user_emails_controller.rb create mode 100644 app/models/user_email.rb create mode 100644 app/views/users/emails.html.haml create mode 100644 db/migrate/20120123051330_create_user_emails.rb create mode 100644 spec/factories/user_emails.rb create mode 100644 spec/models/user_email_spec.rb diff --git a/app/controllers/user_emails_controller.rb b/app/controllers/user_emails_controller.rb new file mode 100644 index 000000000..d85e930e5 --- /dev/null +++ b/app/controllers/user_emails_controller.rb @@ -0,0 +1,27 @@ +# coding: UTF-8 +class UserEmailsController < UsersController + before_filter :find_user + + def index + @emails = @user.emails + (5 - @user.emails.count).times {|e| @emails << UserEmail.new(:user_id => @user) } + end + + def update + @user.role = params[:user][:role] + if @user.update_attributes(params[:user]) + flash[:notice] = t('flash.user.saved') + redirect_to users_path + else + flash[:error] = t('flash.user.save_error') + render :action => :edit + end + end + + def destroy + @user.destroy + flash[:notice] = t("flash.user.destroyed") + redirect_to users_path + end + +end diff --git a/app/models/subscribe.rb b/app/models/subscribe.rb index 381feaf6c..785f0e9d5 100644 --- a/app/models/subscribe.rb +++ b/app/models/subscribe.rb @@ -7,7 +7,7 @@ class Subscribe < ActiveRecord::Base subscribes = comment.commentable.subscribes if commentable_class == Issue if commentable_class == Grit::Commit subscribes = comment.project.commit_comments_subscribes(true) # FIXME (true) for rspec - committer = User.where(:email => comment.commentable.committer.email).first + committer = User.includes(:user_emails).where("user_emails.email = ?", comment.commentable.committer.email).first UserMailer.delay.new_comment_notification(comment, committer) if committer && !comment.own_comment?(committer) && committer.notifier.new_comment_commit_owner && !committer.notifier.can_notify && subscribes.where(:user_id => committer).empty? end subscribes.each do |subscribe| diff --git a/app/models/user.rb b/app/models/user.rb index 16a399caa..f81faa602 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -24,6 +24,8 @@ class User < ActiveRecord::Base has_many :platforms, :through => :targets, :source => :target, :source_type => 'Platform', :autosave => true has_many :repositories, :through => :targets, :source => :target, :source_type => 'Repository', :autosave => true + has_many :user_emails, :dependent => :destroy, :as => :emails + include Modules::Models::PersonalRepository validates :uname, :presence => true, :uniqueness => {:case_sensitive => false}, :format => { :with => /^[a-z0-9_]+$/ } @@ -41,7 +43,7 @@ class User < ActiveRecord::Base def admin? role == 'admin' end - + def guest? self.id.blank? # persisted? end @@ -53,7 +55,9 @@ class User < ActiveRecord::Base def find_for_database_authentication(warden_conditions) conditions = warden_conditions.dup login = conditions.delete(:login) - where(conditions).where(["lower(uname) = :value OR lower(email) = :value", { :value => login.downcase }]).first + where(conditions).where("lower(uname) = :value OR " + + "exists (select null from user_emails m where users.user_id = m.user_id and lower(m.email) = :value)", + {:value => login.downcase}).first end def new_with_session(params, session) @@ -82,7 +86,7 @@ class User < ActiveRecord::Base clean_up_passwords result end - + private def create_settings_notifier diff --git a/app/models/user_email.rb b/app/models/user_email.rb new file mode 100644 index 000000000..43c2b7adf --- /dev/null +++ b/app/models/user_email.rb @@ -0,0 +1,7 @@ +class UserEmail < ActiveRecord::Base + belongs_to :user + + validates :email, :uniqueness => true + validates :email, :presence => true + +end diff --git a/app/views/devise/registrations/edit.html.haml b/app/views/devise/registrations/edit.html.haml index b983f1836..2202a2a97 100644 --- a/app/views/devise/registrations/edit.html.haml +++ b/app/views/devise/registrations/edit.html.haml @@ -76,3 +76,6 @@ .group.navform.wat-cf = link_to t('layout.settings.notifier'), user_settings_notifier_path(current_user)#, :class => "text_button_padding link_button" + + .group.navform.wat-cf + = link_to t('layout.settings.emails'), user_emails_path(current_user)#, :class => "text_button_padding link_button" diff --git a/app/views/users/emails.html.haml b/app/views/users/emails.html.haml new file mode 100644 index 000000000..e69de29bb diff --git a/config/locales/en.yml b/config/locales/en.yml index 4fa4edbc8..9c9fa1623 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1,9 +1,9 @@ en: will_paginate: - previous_label: ‹ Previous!!! + previous_label: ‹ Previous!!! next_label: Next › page_gap: ... - + datatables: previous_label: ‹ Prev. next_label: Next ›b r @@ -44,6 +44,7 @@ en: notifier: Notifier setting notifiers: edit_header: Notifier setting + email: Email addresses processing: working ... downloads: @@ -690,4 +691,4 @@ en: new_comment_notification: New comment to your task new_issue_notification: New task added to project new_user_notification: Registered on project «%{ project_name }» - issue_assign_notification: New task assigned \ No newline at end of file + issue_assign_notification: New task assigned diff --git a/config/locales/ru.yml b/config/locales/ru.yml index a4786dee7..05ba8dff7 100644 --- a/config/locales/ru.yml +++ b/config/locales/ru.yml @@ -44,6 +44,7 @@ ru: notifier: Настройки оповещений notifiers: edit_header: Настройки оповещений + emails: Список Email processing: Обрабатывается... downloads: @@ -680,14 +681,14 @@ ru: status: Статус version: Версия build_list: Сборочный лист - + download: name: Название version: Версия distro: Дистрибутив platform: Архитектура counter: Закачки - + notifications: subjects: new_comment_notification: Новый комментарий к Вашей задаче diff --git a/config/routes.rb b/config/routes.rb index d36e2b7e2..4f347b428 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -8,6 +8,10 @@ Rosa::Application.routes.draw do resources :users do resources :groups, :only => [:new, :create, :index] + resources :emails, :only => [:index, :destroy], :controller => :user_emails do + put :update, :as => :member + end + get :autocomplete_user_uname, :on => :collection namespace :settings do resource :notifier, :only => [:show, :update] diff --git a/db/migrate/20120123051330_create_user_emails.rb b/db/migrate/20120123051330_create_user_emails.rb new file mode 100644 index 000000000..b148e8d56 --- /dev/null +++ b/db/migrate/20120123051330_create_user_emails.rb @@ -0,0 +1,21 @@ +class CreateUserEmails < ActiveRecord::Migration + def self.up + create_table :user_emails do |t| + t.integer :user_id, :null => false + t.string :email, :null => false + + t.timestamps + end + + add_index :user_emails, :user_id + add_index :user_emails, :email + UserEmail.reset_column_information + User.all.each do |u| + UserEmail.create(:user_id => u.id, :email => u.email) + end + end + + def self.down + drop_table :user_emails + end +end diff --git a/db/schema.rb b/db/schema.rb index 86e064a69..19d47b0c9 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended to check this file into your version control system. -ActiveRecord::Schema.define(:version => 20120118173141) do +ActiveRecord::Schema.define(:version => 20120123161250) do create_table "arches", :force => true do |t| t.string "name", :null => false @@ -304,13 +304,25 @@ ActiveRecord::Schema.define(:version => 20120118173141) do end create_table "subscribes", :force => true do |t| - t.integer "subscribeable_id" + t.string "subscribeable_id" t.string "subscribeable_type" t.integer "user_id" t.datetime "created_at" t.datetime "updated_at" + t.integer "status", :default => 1 + t.integer "project_id" end + create_table "user_emails", :force => true do |t| + t.integer "user_id", :null => false + t.string "email", :null => false + t.datetime "created_at" + t.datetime "updated_at" + end + + add_index "user_emails", ["email"], :name => "index_user_emails_on_email" + add_index "user_emails", ["user_id"], :name => "index_user_emails_on_user_id" + create_table "users", :force => true do |t| t.string "name" t.string "email", :default => "", :null => false diff --git a/spec/factories/user_emails.rb b/spec/factories/user_emails.rb new file mode 100644 index 000000000..ed339e424 --- /dev/null +++ b/spec/factories/user_emails.rb @@ -0,0 +1,8 @@ +# Read about factories at http://github.com/thoughtbot/factory_girl + +FactoryGirl.define do + factory :user_email do + user_id 1 + email "MyString" + end +end \ No newline at end of file diff --git a/spec/models/user_email_spec.rb b/spec/models/user_email_spec.rb new file mode 100644 index 000000000..f1c88c94c --- /dev/null +++ b/spec/models/user_email_spec.rb @@ -0,0 +1,5 @@ +require 'spec_helper' + +describe UserEmail do + pending "add some examples to (or delete) #{__FILE__}" +end From e71a9e84cb847e1eca26ea05cd82d143ab1da9ad Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Fri, 27 Jan 2012 02:11:07 +0600 Subject: [PATCH 22/35] [refs #114] emails control --- app/controllers/user_emails_controller.rb | 32 ++++++++----------- app/models/comment.rb | 2 +- app/models/subscribe.rb | 2 +- app/models/user.rb | 10 ++++-- app/views/devise/registrations/edit.html.haml | 2 +- app/views/users/emails.html.haml | 0 app/views/users/emails/_form.html.haml | 15 +++++++++ app/views/users/emails/emails.html.haml | 5 +++ config/locales/en.yml | 9 ++++-- config/locales/ru.yml | 8 ++++- config/routes.rb | 6 ++-- spec/models/comment_for_commit_spec.rb | 12 +++---- 12 files changed, 67 insertions(+), 36 deletions(-) delete mode 100644 app/views/users/emails.html.haml create mode 100644 app/views/users/emails/_form.html.haml create mode 100644 app/views/users/emails/emails.html.haml diff --git a/app/controllers/user_emails_controller.rb b/app/controllers/user_emails_controller.rb index d85e930e5..64ba07341 100644 --- a/app/controllers/user_emails_controller.rb +++ b/app/controllers/user_emails_controller.rb @@ -1,27 +1,21 @@ # coding: UTF-8 -class UserEmailsController < UsersController - before_filter :find_user +class UserEmailsController < ApplicationController + layout 'sessions' + before_filter :authenticate_user! - def index - @emails = @user.emails - (5 - @user.emails.count).times {|e| @emails << UserEmail.new(:user_id => @user) } + def edit + (5 - current_user.emails.count).times {current_user.emails.build } + render 'users/emails/emails' end def update - @user.role = params[:user][:role] - if @user.update_attributes(params[:user]) - flash[:notice] = t('flash.user.saved') - redirect_to users_path - else - flash[:error] = t('flash.user.save_error') - render :action => :edit - end - end + new_emails = [] + params[:user][:emails_attributes].each_value {|x| new_emails << x[:email] if x[:email].present?} + emails = current_user.emails + emails.each {|e| e.destroy unless new_emails.include?(e.email)} + new_emails.each {|e| emails.create(:email => e) unless emails.include? e} - def destroy - @user.destroy - flash[:notice] = t("flash.user.destroyed") - redirect_to users_path + flash[:notice] = t('flash.user.emails.saved') + redirect_to edit_user_emails_path end - end diff --git a/app/models/comment.rb b/app/models/comment.rb index d59d670e8..48db60efc 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -28,7 +28,7 @@ class Comment < ActiveRecord::Base self.commentable.subscribes.create(:user_id => self.user_id) if !self.commentable.subscribes.exists?(:user_id => self.user_id) elsif self.commentable.class == Grit::Commit recipients = self.project.relations.by_role('admin').where(:object_type => 'User').map &:object # admins - recipients << self.user << User.where(:email => self.commentable.committer.email).first # commentor and committer + recipients << self.user << UserEmail.where(:email => self.commentable.committer.email).first.try(:user) # commentor and committer recipients << self.project.owner if self.project.owner_type == 'User' # project owner recipients.compact.uniq.each {|user| Subscribe.subscribe_user_to_commit(self, user.id)} end diff --git a/app/models/subscribe.rb b/app/models/subscribe.rb index 7ea9f612e..089c6c97e 100644 --- a/app/models/subscribe.rb +++ b/app/models/subscribe.rb @@ -42,7 +42,7 @@ class Subscribe < ActiveRecord::Base def self.subscribed_to_commit?(project, user, commentable) is_commentor = (Comment.where(:commentable_type => commentable.class.name, :commentable_id => commentable.id).exists?(:user_id => user.id)) - is_committer = (user.email == commentable.committer.email) + is_committer = (user.emails.exists? :email => commentable.committer.email) return false if user.subscribes.where(:subscribeable_id => commentable.id, :subscribeable_type => commentable.class.name, :project_id => project.id, :status => Subscribe::OFF).first.present? (project.owner?(user) && user.notifier.new_comment_commit_repo_owner) or diff --git a/app/models/user.rb b/app/models/user.rb index 80dc16abe..1665dcf09 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -25,7 +25,7 @@ class User < ActiveRecord::Base has_many :repositories, :through => :targets, :source => :target, :source_type => 'Repository', :autosave => true has_many :subscribes, :foreign_key => :user_id, :dependent => :destroy - has_many :user_emails, :dependent => :destroy, :as => :emails + has_many :emails, :class_name => 'UserEmail', :dependent => :destroy include Modules::Models::PersonalRepository @@ -39,7 +39,10 @@ class User < ActiveRecord::Base attr_readonly :uname attr_accessor :login + accepts_nested_attributes_for :emails, :allow_destroy => true + after_create :create_settings_notifier + after_create :add_user_email def admin? role == 'admin' @@ -57,7 +60,7 @@ class User < ActiveRecord::Base conditions = warden_conditions.dup login = conditions.delete(:login) where(conditions).where("lower(uname) = :value OR " + - "exists (select null from user_emails m where users.user_id = m.user_id and lower(m.email) = :value)", + "exists (select null from user_emails m where m.user_id = m.user_id and lower(m.email) = :value)", {:value => login.downcase}).first end @@ -94,4 +97,7 @@ class User < ActiveRecord::Base self.create_notifier end + def add_user_email + UserEmail.create(:user_id => self.id, :email => self.email) + end end diff --git a/app/views/devise/registrations/edit.html.haml b/app/views/devise/registrations/edit.html.haml index 2202a2a97..090c2c1e2 100644 --- a/app/views/devise/registrations/edit.html.haml +++ b/app/views/devise/registrations/edit.html.haml @@ -78,4 +78,4 @@ = link_to t('layout.settings.notifier'), user_settings_notifier_path(current_user)#, :class => "text_button_padding link_button" .group.navform.wat-cf - = link_to t('layout.settings.emails'), user_emails_path(current_user)#, :class => "text_button_padding link_button" + = link_to t('layout.settings.emails'), edit_user_emails_path(current_user.id)#, :class => "text_button_padding link_button" diff --git a/app/views/users/emails.html.haml b/app/views/users/emails.html.haml deleted file mode 100644 index e69de29bb..000000000 diff --git a/app/views/users/emails/_form.html.haml b/app/views/users/emails/_form.html.haml new file mode 100644 index 000000000..3fdb58400 --- /dev/null +++ b/app/views/users/emails/_form.html.haml @@ -0,0 +1,15 @@ += f.fields_for :emails do |m| + .group + .left + = m.text_field :email, :class => :text_field + .right + +.group.navform.wat-cf + %button.button{:type => "submit"} + = image_tag("web-app-theme/icons/tick.png", :alt => t("layout.save")) + = t("layout.save") + %span.text_button_padding= t("layout.or") + = link_to t("layout.cancel"), edit_user_emails_path(current_user.id), :class => "text_button_padding link_button" +.group.navform.wat-cf + %span.text_button_padding + = link_to t('layout.back'), edit_user_registration_path, :class => "text_button_padding link_button" diff --git a/app/views/users/emails/emails.html.haml b/app/views/users/emails/emails.html.haml new file mode 100644 index 000000000..096e1055d --- /dev/null +++ b/app/views/users/emails/emails.html.haml @@ -0,0 +1,5 @@ +#block-signup.block + %h2= title t("layout.settings.email.list") + .content + = form_for current_user, :url => update_user_emails_path(@user), :html => {:method => :put, :id => current_user.id } do |f| + = render :partial => "users/emails/form", :locals => {:f => f} diff --git a/config/locales/en.yml b/config/locales/en.yml index f8ae03bcf..5cebbccc9 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -45,7 +45,10 @@ en: notifier: Notifier setting notifiers: edit_header: Notifier setting - email: Email addresses + email: + list: Email addresses + delete: Destroy + confirm_delete: Are you sure? processing: working ... downloads: @@ -430,7 +433,9 @@ en: saved: User saved save_error: User data saves error destroyed: User account deleted - + emails: + saved: User emails saved + error: That emails addresses is already in use group: saved: Group saved save_error: Group saves error diff --git a/config/locales/ru.yml b/config/locales/ru.yml index 80aed8c2b..a558ceb75 100644 --- a/config/locales/ru.yml +++ b/config/locales/ru.yml @@ -44,7 +44,10 @@ ru: notifier: Настройки оповещений notifiers: edit_header: Настройки оповещений - emails: Список Email + email: + list: Список Email + delete: Удалить + confirm_delete: Вы уверены? processing: Обрабатывается... downloads: @@ -425,6 +428,9 @@ ru: saved: Пользователь успешно сохранен save_error: Не удалось сохранить данные о пользователе destroyed: Учетная запись успешно удалена + emails: + saved: Список еmail успешно сохранен + error: Такой email уже используется group: saved: Группа успешно сохранена diff --git a/config/routes.rb b/config/routes.rb index 318af8f5f..671ffa1f9 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -8,9 +8,6 @@ Rosa::Application.routes.draw do resources :users do resources :groups, :only => [:new, :create, :index] - resources :emails, :only => [:index, :destroy], :controller => :user_emails do - put :update, :as => :member - end get :autocomplete_user_uname, :on => :collection namespace :settings do @@ -18,6 +15,9 @@ Rosa::Application.routes.draw do end end + match 'users/:id/emails' => 'user_emails#edit', :as => :edit_user_emails, :via => :get, :action => :edit + match 'users/:id/emails' => 'user_emails#update', :as => :update_user_emails, :via => :put, :action => :update + resources :event_logs, :only => :index #resources :downloads, :only => :index diff --git a/spec/models/comment_for_commit_spec.rb b/spec/models/comment_for_commit_spec.rb index 3b862751b..cc16b2d60 100644 --- a/spec/models/comment_for_commit_spec.rb +++ b/spec/models/comment_for_commit_spec.rb @@ -341,7 +341,7 @@ describe Comment do context 'for committer' do it 'should send an e-mail' do ActionMailer::Base.deliveries = [] - @stranger.update_attribute :email, 'code@tpope.net' + @stranger.emails.first.update_attribute :email, 'code@tpope.net' comment = Comment.create(:user => @user, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) ActionMailer::Base.deliveries.count.should == 1 @@ -350,7 +350,7 @@ describe Comment do it 'should send an e-mail with user locale' do ActionMailer::Base.deliveries = [] - @stranger.update_attribute :email, 'code@tpope.net' + @stranger.emails.first.update_attribute :email, 'code@tpope.net' @stranger.update_attribute :language, 'ru' comment = Comment.create(:user => @user, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) @@ -362,7 +362,7 @@ describe Comment do it 'should send a one e-mail when subscribed to commit' do ActionMailer::Base.deliveries = [] Subscribe.set_subscribe_to_commit(@project, @commit, @stranger.id, Subscribe::ON) - @stranger.update_attribute :email, 'code@tpope.net' + @stranger.emails.first.update_attribute :email, 'code@tpope.net' comment = Comment.create(:user => @user, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) ActionMailer::Base.deliveries.count.should == 1 @@ -372,7 +372,7 @@ describe Comment do it 'should not send an e-mail for own comment' do ActionMailer::Base.deliveries = [] #@project.owner.notifier.update_attribute :can_notify, false - @stranger.update_attribute :email, 'code@tpope.net' + @stranger.emails.first.update_attribute :email, 'code@tpope.net' comment = Comment.create(:user => @stranger, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) ActionMailer::Base.deliveries.count.should == 1 @@ -382,7 +382,7 @@ describe Comment do it 'should not send an e-mail if global notify off' do ActionMailer::Base.deliveries = [] @project.owner.notifier.update_attribute :can_notify, false - @stranger.update_attribute :email, 'code@tpope.net' + @stranger.emails.first.update_attribute :email, 'code@tpope.net' @stranger.notifier.update_attribute :can_notify, false comment = Comment.create(:user => @user, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) @@ -392,7 +392,7 @@ describe Comment do it 'should not send an e-mail if notify for my commits off' do ActionMailer::Base.deliveries = [] @stranger.notifier.update_attribute :new_comment_commit_owner, false - @stranger.update_attribute :email, 'code@tpope.net' + @stranger.emails.first.update_attribute :email, 'code@tpope.net' comment = Comment.create(:user => @user, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) ActionMailer::Base.deliveries.count.should == 0 From 06ab99a1d0e378248c452197e03459eb6da5acad Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Fri, 27 Jan 2012 18:18:52 +0600 Subject: [PATCH 23/35] [refs #114] add js --- app/controllers/user_emails_controller.rb | 29 ++++++++++---- app/models/user.rb | 4 +- app/models/user_email.rb | 2 + app/views/devise/registrations/edit.html.haml | 2 +- app/views/users/emails/_form.html.haml | 19 +++++++--- app/views/users/emails/emails.html.haml | 3 +- config/locales/en.yml | 1 + config/locales/ru.yml | 3 +- config/routes.rb | 5 ++- public/javascripts/emails.js | 38 +++++++++++++++++++ spec/models/user_email_spec.rb | 14 ++++++- 11 files changed, 98 insertions(+), 22 deletions(-) create mode 100644 public/javascripts/emails.js diff --git a/app/controllers/user_emails_controller.rb b/app/controllers/user_emails_controller.rb index 64ba07341..3fcaa13b8 100644 --- a/app/controllers/user_emails_controller.rb +++ b/app/controllers/user_emails_controller.rb @@ -2,20 +2,33 @@ class UserEmailsController < ApplicationController layout 'sessions' before_filter :authenticate_user! + respond_to :html + + #def create + # current_user.emails.new + # respond_with current_user, :location => edit_user_emails_path + #end def edit - (5 - current_user.emails.count).times {current_user.emails.build } + @emails = current_user.emails render 'users/emails/emails' end def update - new_emails = [] - params[:user][:emails_attributes].each_value {|x| new_emails << x[:email] if x[:email].present?} - emails = current_user.emails - emails.each {|e| e.destroy unless new_emails.include?(e.email)} - new_emails.each {|e| emails.create(:email => e) unless emails.include? e} + if current_user.update_attributes params[:user] + flash[:notice] = t('flash.user.emails.saved') + else + flash[:error] = t('flash.user.emails.error') + end + respond_with current_user, :location => edit_user_emails_path + end - flash[:notice] = t('flash.user.emails.saved') - redirect_to edit_user_emails_path + def destroy +# if current_user.emails.where(:id => params[:email_id]).first.destroy +# flash[:notice] = t('flash.user.emails.delete') +# else + flash[:error] = t('flash.user.save_error') +# end + respond_with current_user, :location => edit_user_emails_path end end diff --git a/app/models/user.rb b/app/models/user.rb index 1665dcf09..b370c197b 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -35,11 +35,11 @@ class User < ActiveRecord::Base validates :role, :inclusion => {:in => ROLES}, :allow_blank => true validates :language, :inclusion => {:in => LANGUAGES}, :allow_blank => true - attr_accessible :email, :password, :password_confirmation, :remember_me, :login, :name, :ssh_key, :uname, :language + attr_accessible :email, :password, :password_confirmation, :remember_me, :login, :name, :ssh_key, :uname, :language, :emails_attributes attr_readonly :uname attr_accessor :login - accepts_nested_attributes_for :emails, :allow_destroy => true + accepts_nested_attributes_for :emails, :allow_destroy => true, :reject_if => proc {|attributes| attributes['email'].blank?} after_create :create_settings_notifier after_create :add_user_email diff --git a/app/models/user_email.rb b/app/models/user_email.rb index 43c2b7adf..2cec06022 100644 --- a/app/models/user_email.rb +++ b/app/models/user_email.rb @@ -1,4 +1,6 @@ class UserEmail < ActiveRecord::Base + MAX_EMAILS = 10 + belongs_to :user validates :email, :uniqueness => true diff --git a/app/views/devise/registrations/edit.html.haml b/app/views/devise/registrations/edit.html.haml index 090c2c1e2..10f756d6a 100644 --- a/app/views/devise/registrations/edit.html.haml +++ b/app/views/devise/registrations/edit.html.haml @@ -78,4 +78,4 @@ = link_to t('layout.settings.notifier'), user_settings_notifier_path(current_user)#, :class => "text_button_padding link_button" .group.navform.wat-cf - = link_to t('layout.settings.emails'), edit_user_emails_path(current_user.id)#, :class => "text_button_padding link_button" + = link_to t('layout.settings.emails'), edit_user_emails_path#, :class => "text_button_padding link_button" diff --git a/app/views/users/emails/_form.html.haml b/app/views/users/emails/_form.html.haml index 3fdb58400..03da31108 100644 --- a/app/views/users/emails/_form.html.haml +++ b/app/views/users/emails/_form.html.haml @@ -1,15 +1,22 @@ -= f.fields_for :emails do |m| - .group - .left - = m.text_field :email, :class => :text_field - .right +- @emails.each do |email| + = f.fields_for :emails, email do |m| + .group.wat-cf.emails + = m.text_field :email, :class => :text_field, :style => "width: 300px;" + = link_to image_tag("web-app-theme/icons/cross.png", :alt => t("layout.settings.email.delete")), email.id ? destroy_user_email_path(email.id) : '#', :class => "link_button delete_button" .group.navform.wat-cf + %button.button{:type => "submit"} = image_tag("web-app-theme/icons/tick.png", :alt => t("layout.save")) = t("layout.save") + + + %button.button{:type => "submit", :id => 'add_button'} + = image_tag("web-app-theme/icons/tick.png", :alt => t("layout.add")) + = t("layout.add") + %span.text_button_padding= t("layout.or") - = link_to t("layout.cancel"), edit_user_emails_path(current_user.id), :class => "text_button_padding link_button" + = link_to t("layout.cancel"), edit_user_emails_path, :class => "text_button_padding link_button" .group.navform.wat-cf %span.text_button_padding = link_to t('layout.back'), edit_user_registration_path, :class => "text_button_padding link_button" diff --git a/app/views/users/emails/emails.html.haml b/app/views/users/emails/emails.html.haml index 096e1055d..f1b58d386 100644 --- a/app/views/users/emails/emails.html.haml +++ b/app/views/users/emails/emails.html.haml @@ -1,5 +1,6 @@ += javascript_include_tag 'emails' #block-signup.block %h2= title t("layout.settings.email.list") .content - = form_for current_user, :url => update_user_emails_path(@user), :html => {:method => :put, :id => current_user.id } do |f| + = form_for current_user, :url => update_user_emails_path(@user), :html => {:class => 'form', :method => :put, :id => current_user.id } do |f| = render :partial => "users/emails/form", :locals => {:f => f} diff --git a/config/locales/en.yml b/config/locales/en.yml index 5cebbccc9..3008dad59 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -436,6 +436,7 @@ en: emails: saved: User emails saved error: That emails addresses is already in use + delete: User email delete group: saved: Group saved save_error: Group saves error diff --git a/config/locales/ru.yml b/config/locales/ru.yml index a558ceb75..b5c20665a 100644 --- a/config/locales/ru.yml +++ b/config/locales/ru.yml @@ -430,7 +430,8 @@ ru: destroyed: Учетная запись успешно удалена emails: saved: Список еmail успешно сохранен - error: Такой email уже используется + error: Такой адрес уже используется + delete: Email удален group: saved: Группа успешно сохранена diff --git a/config/routes.rb b/config/routes.rb index 671ffa1f9..4343134eb 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -15,8 +15,9 @@ Rosa::Application.routes.draw do end end - match 'users/:id/emails' => 'user_emails#edit', :as => :edit_user_emails, :via => :get, :action => :edit - match 'users/:id/emails' => 'user_emails#update', :as => :update_user_emails, :via => :put, :action => :update + match 'user/emails' => 'user_emails#edit', :as => :edit_user_emails, :via => :get, :action => :edit + match 'user/emails' => 'user_emails#update', :as => :update_user_emails, :via => :put, :action => :update + match 'user/emails/:email_id' => 'user_emails#destroy', :as => :destroy_user_email, :via => :post, :action => :destroy resources :event_logs, :only => :index diff --git a/public/javascripts/emails.js b/public/javascripts/emails.js new file mode 100644 index 000000000..aa148dcee --- /dev/null +++ b/public/javascripts/emails.js @@ -0,0 +1,38 @@ +$(function() { + + $("#add_button").click(function(){ + var c = $('.emails').length; + var clone = $('.emails:first').clone(); + clone.find('input') + .removeAttr('id').attr("id", "user_emails_attributes_"+c+"_email") + .removeAttr('name').attr("name", "user[emails_attributes]["+c+"][email]").val(''); + clone.find(".delete_button").attr('href', "#"); + clone.insertAfter('.emails:last'); + return false; + }); + + $("a.delete_button").live("click", function(){ + function del_question() { + var p = a.parentNode; + p.parentNode.removeChild(p); + } + var a = this; + if(/#$/.test(a.href)) { del_question(); /* delete new email */} + else { /* delete exists email */ + $.ajax({ + type: "POST", + url: this.href, + //data: "authenticity_token=" + encodeURIComponent(AUTH_TOKEN), + success: function(msg){ + del_question(); + var q_id = a.href.match(/user\/emails\/(\d+)/); + if(q_id) { + var e = $('input:hidden[value='+q_id[1]+']'); + e.remove(); + } + } + }); + } + return false; + }); +}); \ No newline at end of file diff --git a/spec/models/user_email_spec.rb b/spec/models/user_email_spec.rb index f1c88c94c..3d663cd6b 100644 --- a/spec/models/user_email_spec.rb +++ b/spec/models/user_email_spec.rb @@ -1,5 +1,17 @@ require 'spec_helper' describe UserEmail do - pending "add some examples to (or delete) #{__FILE__}" + context 'for simple user' do + before(:each) do + @user = Factory(:user) + #@ability = Ability.new(@stranger) + @create_params = {:email => 'test@test.com'} + end + + it 'should not create duplicate email' do + @stranger = Factory(:user) + @stranger.emails.create(:email => @user.emails.first.email) + @stranger.emails.exists?(:email => @user.emails.first.email).should be_false + end + end end From 39a4479fa3a8ce82dea426456a0536f757950fe9 Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Sun, 29 Jan 2012 22:27:46 +0600 Subject: [PATCH 24/35] [refs #114] fix emails uniqueness --- app/models/comment.rb | 2 +- app/models/subscribe.rb | 2 +- app/models/user.rb | 4 +- app/models/user_email.rb | 9 +++- ...29120025_add_email_lower_to_user_emails.rb | 18 ++++++++ db/schema.rb | 46 ++++--------------- spec/models/user_email_spec.rb | 11 +++++ 7 files changed, 50 insertions(+), 42 deletions(-) create mode 100644 db/migrate/20120129120025_add_email_lower_to_user_emails.rb diff --git a/app/models/comment.rb b/app/models/comment.rb index 48db60efc..a8b74d3d6 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -28,7 +28,7 @@ class Comment < ActiveRecord::Base self.commentable.subscribes.create(:user_id => self.user_id) if !self.commentable.subscribes.exists?(:user_id => self.user_id) elsif self.commentable.class == Grit::Commit recipients = self.project.relations.by_role('admin').where(:object_type => 'User').map &:object # admins - recipients << self.user << UserEmail.where(:email => self.commentable.committer.email).first.try(:user) # commentor and committer + recipients << self.user << UserEmail.where(:email_lower => self.commentable.committer.email.downcase).first.try(:user) # commentor and committer recipients << self.project.owner if self.project.owner_type == 'User' # project owner recipients.compact.uniq.each {|user| Subscribe.subscribe_user_to_commit(self, user.id)} end diff --git a/app/models/subscribe.rb b/app/models/subscribe.rb index 089c6c97e..cd08c4f1d 100644 --- a/app/models/subscribe.rb +++ b/app/models/subscribe.rb @@ -42,7 +42,7 @@ class Subscribe < ActiveRecord::Base def self.subscribed_to_commit?(project, user, commentable) is_commentor = (Comment.where(:commentable_type => commentable.class.name, :commentable_id => commentable.id).exists?(:user_id => user.id)) - is_committer = (user.emails.exists? :email => commentable.committer.email) + is_committer = (user.emails.exists? :email_lower => commentable.committer.email.downcase) return false if user.subscribes.where(:subscribeable_id => commentable.id, :subscribeable_type => commentable.class.name, :project_id => project.id, :status => Subscribe::OFF).first.present? (project.owner?(user) && user.notifier.new_comment_commit_repo_owner) or diff --git a/app/models/user.rb b/app/models/user.rb index b370c197b..2fe798ebe 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -59,8 +59,8 @@ class User < ActiveRecord::Base def find_for_database_authentication(warden_conditions) conditions = warden_conditions.dup login = conditions.delete(:login) - where(conditions).where("lower(uname) = :value OR " + - "exists (select null from user_emails m where m.user_id = m.user_id and lower(m.email) = :value)", + where(conditions).where("(lower(uname) = :value OR \ + exists (select null from user_emails m where m.user_id = m.user_id and m.email_lower = :value))", {:value => login.downcase}).first end diff --git a/app/models/user_email.rb b/app/models/user_email.rb index 2cec06022..485b9a6f7 100644 --- a/app/models/user_email.rb +++ b/app/models/user_email.rb @@ -3,7 +3,14 @@ class UserEmail < ActiveRecord::Base belongs_to :user - validates :email, :uniqueness => true + validates :email_lower, :uniqueness => true validates :email, :presence => true + before_validation :set_lower + + private + + def set_lower + self.email_lower = self.email.downcase + end end diff --git a/db/migrate/20120129120025_add_email_lower_to_user_emails.rb b/db/migrate/20120129120025_add_email_lower_to_user_emails.rb new file mode 100644 index 000000000..8983c0091 --- /dev/null +++ b/db/migrate/20120129120025_add_email_lower_to_user_emails.rb @@ -0,0 +1,18 @@ +class AddEmailLowerToUserEmails < ActiveRecord::Migration + def self.up + add_column :user_emails, :email_lower, :string + remove_index :user_emails, :email + + UserEmail.reset_column_information + UserEmail.update_all("email_lower = lower(email)") + + change_column :user_emails, :email_lower, :string, :null => false + add_index :user_emails, :email_lower + end + + def self.down + remove_column :user_emails, :email_lower + add_index :user_emails, :email + remove_index :user_emails, :email_lower + end +end diff --git a/db/schema.rb b/db/schema.rb index a4692cd75..1bdee6ff5 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended to check this file into your version control system. -ActiveRecord::Schema.define(:version => 20120123161250) do +ActiveRecord::Schema.define(:version => 20120129120025) do create_table "arches", :force => true do |t| t.string "name", :null => false @@ -167,13 +167,6 @@ ActiveRecord::Schema.define(:version => 20120123161250) do add_index "issues", ["project_id", "serial_id"], :name => "index_issues_on_project_id_and_serial_id", :unique => true - create_table "permissions", :force => true do |t| - t.integer "right_id" - t.integer "role_id" - t.datetime "created_at" - t.datetime "updated_at" - end - create_table "platforms", :force => true do |t| t.string "description" t.string "name" @@ -270,27 +263,6 @@ ActiveRecord::Schema.define(:version => 20120123161250) do t.string "owner_type" end - create_table "rights", :force => true do |t| - t.string "name", :null => false - t.string "controller", :null => false - t.string "action", :null => false - t.datetime "created_at" - t.datetime "updated_at" - end - - create_table "role_lines", :force => true do |t| - t.integer "role_id" - t.integer "relation_id" - t.datetime "created_at" - t.datetime "updated_at" - end - - create_table "roles", :force => true do |t| - t.string "name" - t.datetime "created_at" - t.datetime "updated_at" - end - create_table "rpms", :force => true do |t| t.string "name", :null => false t.integer "arch_id", :null => false @@ -327,29 +299,29 @@ ActiveRecord::Schema.define(:version => 20120123161250) do end create_table "user_emails", :force => true do |t| - t.integer "user_id", :null => false - t.string "email", :null => false + t.integer "user_id", :null => false + t.string "email", :null => false t.datetime "created_at" t.datetime "updated_at" + t.string "email_lower", :null => false end - add_index "user_emails", ["email"], :name => "index_user_emails_on_email" + add_index "user_emails", ["email_lower"], :name => "index_user_emails_on_email_lower" add_index "user_emails", ["user_id"], :name => "index_user_emails_on_user_id" create_table "users", :force => true do |t| t.string "name" - t.string "email", :default => "", :null => false - t.string "encrypted_password", :limit => 128, :default => "", :null => false - t.string "password_salt", :default => "", :null => false + t.string "email", :default => "", :null => false + t.string "encrypted_password", :limit => 128, :default => "", :null => false t.string "reset_password_token" - t.string "remember_token" + t.datetime "reset_password_sent_at" t.datetime "remember_created_at" t.datetime "created_at" t.datetime "updated_at" t.text "ssh_key" t.string "uname" t.string "role" - t.string "language", :default => "en" + t.string "language", :default => "en" end add_index "users", ["email"], :name => "index_users_on_email", :unique => true diff --git a/spec/models/user_email_spec.rb b/spec/models/user_email_spec.rb index 3d663cd6b..ecd6e82e2 100644 --- a/spec/models/user_email_spec.rb +++ b/spec/models/user_email_spec.rb @@ -13,5 +13,16 @@ describe UserEmail do @stranger.emails.create(:email => @user.emails.first.email) @stranger.emails.exists?(:email => @user.emails.first.email).should be_false end + + it 'should not create duplicate lowercase emails' do + @stranger = Factory(:user) + @stranger.emails.create(:email => @user.email.upcase) + @stranger.emails.exists?(:email => @user.email.upcase).should be_false + end + + it 'should not create too many emails' do + 15.times {|i| @user.emails.create(:email => Factory.next(:email))} + @user.emails.count.should be_equal(UserEmail::MAX_EMAILS) + end end end From 2e2e7e43d3dc5f3cbe73d54b36ce80a58ffb6f3f Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Mon, 30 Jan 2012 02:18:14 +0600 Subject: [PATCH 25/35] [refs #114] some refactoring --- .../commit_subscribes_controller.rb | 5 ++-- app/models/comment.rb | 7 +++-- app/models/project.rb | 2 +- app/models/subscribe.rb | 28 +++++++++---------- app/models/user.rb | 9 ++++++ app/models/user_email.rb | 1 + app/views/git/commits/show.html.haml | 3 +- spec/models/comment_for_commit_spec.rb | 10 +++---- 8 files changed, 38 insertions(+), 27 deletions(-) diff --git a/app/controllers/commit_subscribes_controller.rb b/app/controllers/commit_subscribes_controller.rb index e335fdff6..4d33c06ee 100644 --- a/app/controllers/commit_subscribes_controller.rb +++ b/app/controllers/commit_subscribes_controller.rb @@ -6,7 +6,7 @@ class CommitSubscribesController < ApplicationController before_filter :find_commit def create - if Subscribe.set_subscribe_to_commit(@project, @commit, current_user.id, Subscribe::ON) + if Subscribe.set_subscribe_to_commit(@options, Subscribe::ON) flash[:notice] = I18n.t("flash.subscribe.commit.saved") # TODO js redirect_to commit_path(@project, @commit) @@ -17,7 +17,7 @@ class CommitSubscribesController < ApplicationController end def destroy - Subscribe.set_subscribe_to_commit(@project, @commit, current_user.id, Subscribe::OFF) + Subscribe.set_subscribe_to_commit(@options, Subscribe::OFF) flash[:notice] = t("flash.subscribe.commit.destroyed") redirect_to commit_path(@project, @commit) end @@ -26,5 +26,6 @@ class CommitSubscribesController < ApplicationController def find_commit @commit = @project.git_repository.commit(params[:commit_id]) + @options = {:project_id => @project.id, :subscribeable_id => @commit.id, :subscribeable_type => @commit.class.name, :user_id => current_user.id} end end diff --git a/app/models/comment.rb b/app/models/comment.rb index a8b74d3d6..b51501258 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -25,12 +25,15 @@ class Comment < ActiveRecord::Base def subscribe_users if self.commentable.class == Issue - self.commentable.subscribes.create(:user_id => self.user_id) if !self.commentable.subscribes.exists?(:user_id => self.user_id) + self.commentable.subscribes.create(:user => self.user) if !self.commentable.subscribes.exists?(:user_id => self.user.id) elsif self.commentable.class == Grit::Commit recipients = self.project.relations.by_role('admin').where(:object_type => 'User').map &:object # admins recipients << self.user << UserEmail.where(:email_lower => self.commentable.committer.email.downcase).first.try(:user) # commentor and committer recipients << self.project.owner if self.project.owner_type == 'User' # project owner - recipients.compact.uniq.each {|user| Subscribe.subscribe_user_to_commit(self, user.id)} + recipients.compact.uniq.each do |user| + options = {:project_id => self.project.id, :subscribeable_id => self.commentable.id, :subscribeable_type => self.commentable.class.name, :user_id => user.id} + Subscribe.set_subscribe_to_commit(options, Subscribe::ON) if Subscribe.subscribed_to_commit?(self.project, user, self.commentable) + end end end end diff --git a/app/models/project.rb b/app/models/project.rb index 52aeb75f0..904795442 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -142,7 +142,7 @@ class Project < ActiveRecord::Base end def owner?(user) - owner_id == user.id + owner == user end protected diff --git a/app/models/subscribe.rb b/app/models/subscribe.rb index cd08c4f1d..b0c31a216 100644 --- a/app/models/subscribe.rb +++ b/app/models/subscribe.rb @@ -11,6 +11,10 @@ class Subscribe < ActiveRecord::Base scope :off, where(:status => OFF) scope :finder_hack, order('') # FIXME .subscribes - error; .subscribes.finder_hack - success Oo + def subscribed? + status == ON + end + def self.comment_subscribes(comment) Subscribe.where(:subscribeable_id => comment.commentable.id, :subscribeable_type => comment.commentable.class.name, :project_id => comment.project) end @@ -29,30 +33,24 @@ class Subscribe < ActiveRecord::Base end def self.new_comment_commit_notification(comment) - subscribes = Subscribe.comment_subscribes(comment).on#(true) # FIXME (true) for rspec + subscribes = Subscribe.comment_subscribes(comment).on subscribes.each do |subscribe| next if comment.own_comment?(subscribe.user) || !subscribe.user.notifier.can_notify UserMailer.delay.new_comment_notification(comment, subscribe.user) end end - def self.subscribe_user_to_commit(comment, user) - Subscribe.set_subscribe_to_commit(comment.project, comment.commentable, user, Subscribe::ON) if Subscribe.subscribed_to_commit?(comment.project, User.find(user), comment.commentable) - end - - def self.subscribed_to_commit?(project, user, commentable) - is_commentor = (Comment.where(:commentable_type => commentable.class.name, :commentable_id => commentable.id).exists?(:user_id => user.id)) - is_committer = (user.emails.exists? :email_lower => commentable.committer.email.downcase) - return false if user.subscribes.where(:subscribeable_id => commentable.id, :subscribeable_type => commentable.class.name, - :project_id => project.id, :status => Subscribe::OFF).first.present? + def self.subscribed_to_commit?(project, user, commit) + subscribe = user.subscribes.where(:subscribeable_id => commit.id, :subscribeable_type => commit.class.name, :project_id => project.id).first + return subscribe.subscribed? if subscribe # return status if already subscribe present + # return status by settings (project.owner?(user) && user.notifier.new_comment_commit_repo_owner) or - (is_commentor && user.notifier.new_comment_commit_commentor) or - (is_committer && user.notifier.new_comment_commit_owner) + (user.commentor?(commit) && user.notifier.new_comment_commit_commentor) or + (user.committer?(commit) && user.notifier.new_comment_commit_owner) end - def self.set_subscribe_to_commit(project, commit, user, status) - options = {:subscribeable_id => commit.id, :subscribeable_type => commit.class.name, :user_id => user, :project_id => project.id} - if subscribe = Subscribe.where(options).first # FIXME maybe? + def self.set_subscribe_to_commit(options, status) + if subscribe = Subscribe.where(options).first subscribe.update_attribute(:status, status) else Subscribe.create(options.merge(:status => status)) diff --git a/app/models/user.rb b/app/models/user.rb index 2fe798ebe..6e77e08b2 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -25,6 +25,7 @@ class User < ActiveRecord::Base has_many :repositories, :through => :targets, :source => :target, :source_type => 'Repository', :autosave => true has_many :subscribes, :foreign_key => :user_id, :dependent => :destroy + has_many :comments, :dependent => :destroy has_many :emails, :class_name => 'UserEmail', :dependent => :destroy include Modules::Models::PersonalRepository @@ -91,6 +92,14 @@ class User < ActiveRecord::Base result end + def commentor?(commentable) + comments.exists?(:commentable_type => commentable.class.name, :commentable_id => commentable.id) + end + + def committer?(commit) + emails.exists? :email_lower => commit.committer.email.downcase + end + private def create_settings_notifier diff --git a/app/models/user_email.rb b/app/models/user_email.rb index 485b9a6f7..a7eb17b1c 100644 --- a/app/models/user_email.rb +++ b/app/models/user_email.rb @@ -6,6 +6,7 @@ class UserEmail < ActiveRecord::Base validates :email_lower, :uniqueness => true validates :email, :presence => true + before_save :set_lower before_validation :set_lower private diff --git a/app/views/git/commits/show.html.haml b/app/views/git/commits/show.html.haml index 510f9a045..e1c739a35 100644 --- a/app/views/git/commits/show.html.haml +++ b/app/views/git/commits/show.html.haml @@ -32,8 +32,7 @@ %b = t('layout.issues.subscribe') \: - - subscribe = current_user.subscribes.where(:subscribeable_id => @commit.id, :subscribeable_type => @commit.class.name.to_s, :project_id => @project, :status => Subscribe::OFF).first - - if subscribe.nil? or Subscribe.subscribed_to_commit?(@project, current_user, @commit) + - if Subscribe.subscribed_to_commit?(@project, current_user, @commit) = link_to t('layout.commits.unsubscribe_btn'), unsubscribe_commit_path(@project, @commit), :method => :delete - else = link_to t('layout.commits.subscribe_btn'), subscribe_commit_path(@project, @commit), :method => :post diff --git a/spec/models/comment_for_commit_spec.rb b/spec/models/comment_for_commit_spec.rb index cc16b2d60..c36001a03 100644 --- a/spec/models/comment_for_commit_spec.rb +++ b/spec/models/comment_for_commit_spec.rb @@ -131,7 +131,7 @@ describe Comment do context 'for unsubscribe commit' do it 'should not send an e-mail' do ActionMailer::Base.deliveries = [] - Subscribe.set_subscribe_to_commit(@project, @commit, @user.id, Subscribe::OFF) + Subscribe.set_subscribe_to_commit({:project_id => @project.id, :subscribeable_id => @commit.id, :subscribeable_type => @commit.class.name, :user_id => @user.id}, Subscribe::OFF) comment = Comment.create(:user => @stranger, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) ActionMailer::Base.deliveries.count.should == 0 # cache project.commit_comments_subscribes ... @@ -234,7 +234,7 @@ describe Comment do context 'for unsubscribe project' do it 'should not send an e-mail' do ActionMailer::Base.deliveries = [] - Subscribe.set_subscribe_to_commit(@project, @commit, @user.id, Subscribe::OFF) + Subscribe.set_subscribe_to_commit({:project_id => @project.id, :subscribeable_id => @commit.id, :subscribeable_type => @commit.class.name, :user_id => @user.id}, Subscribe::OFF) comment = Comment.create(:user => @stranger, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) ActionMailer::Base.deliveries.count.should == 0 @@ -321,7 +321,7 @@ describe Comment do @stranger.notifier.update_attribute :new_comment_commit_owner, false #@stranger.notifier.update_attribute :new_comment_commit_commentor, false - Subscribe.set_subscribe_to_commit(@project, @commit, @stranger.id, Subscribe::ON) + Subscribe.set_subscribe_to_commit({:project_id => @project.id, :subscribeable_id => @commit.id, :subscribeable_type => @commit.class.name, :user_id => @stranger.id}, Subscribe::ON) comment = Comment.create(:user => @project.owner, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) ActionMailer::Base.deliveries.count.should == 1 @@ -331,7 +331,7 @@ describe Comment do it 'should not send an e-mail for own comment' do ActionMailer::Base.deliveries = [] #@project.owner.notifier.update_attribute :can_notify, false - Subscribe.set_subscribe_to_commit(@project, @commit, @stranger.id, Subscribe::ON) + Subscribe.set_subscribe_to_commit({:project_id => @project.id, :subscribeable_id => @commit.id, :subscribeable_type => @commit.class.name, :user_id => @stranger.id}, Subscribe::ON) comment = Comment.create(:user => @owner, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) ActionMailer::Base.deliveries.count.should == 0 @@ -361,7 +361,7 @@ describe Comment do it 'should send a one e-mail when subscribed to commit' do ActionMailer::Base.deliveries = [] - Subscribe.set_subscribe_to_commit(@project, @commit, @stranger.id, Subscribe::ON) + Subscribe.set_subscribe_to_commit({:project_id => @project.id, :subscribeable_id => @commit.id, :subscribeable_type => @commit.class.name, :user_id => @stranger.id}, Subscribe::ON) @stranger.emails.first.update_attribute :email, 'code@tpope.net' comment = Comment.create(:user => @user, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) From 255c39ef5a7e5c3e70e140921487e3ac9cbdc1c8 Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Mon, 30 Jan 2012 12:09:52 +0600 Subject: [PATCH 26/35] Revert "Merge branch 'fix-email-i18n' into 114-comment_notifications" This reverts commit 9bf06e5e1a595f320dc4527ebd44f14be05fcd64, reversing changes made to f2bc0e67a91e6283f4881e754e5da95c59cf341b. --- app/mailers/user_mailer.rb | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb index 2c25b14a0..2923e62fd 100644 --- a/app/mailers/user_mailer.rb +++ b/app/mailers/user_mailer.rb @@ -13,41 +13,24 @@ class UserMailer < ActionMailer::Base def new_comment_notification(comment, user) @user = user @comment = comment - set_locale mail(:to => user.email, :subject => I18n.t("notifications.subjects.new_#{comment.commentable.class == Grit::Commit ? 'commit_' : ''}comment_notification")) do |format| format.html end - ensure reset_locale end def new_issue_notification(issue, user) @user = user @issue = issue - set_locale mail(:to => user.email, :subject => I18n.t("notifications.subjects.new_issue_notification")) do |format| format.html end - ensure reset_locale end def issue_assign_notification(issue, user) @user = user @issue = issue - set_locale mail(:to => user.email, :subject => I18n.t("notifications.subjects.issue_assign_notification")) do |format| format.html end - ensure reset_locale end - - protected - - def set_locale - @initial_locale, I18n.locale = I18n.locale, @user.language - end - - def reset_locale - I18n.locale = @initial_locale - end - end From ea7cbe1908ff6d597aec2f5b9e30878ba5f7541d Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Mon, 30 Jan 2012 12:15:33 +0600 Subject: [PATCH 27/35] Revert "[refs #114] fix emails uniqueness" This reverts commit bc9649561f5b8ac58f9b27d31d66365f16d0cce0. Conflicts: app/models/subscribe.rb app/models/user_email.rb --- app/models/comment.rb | 2 +- app/models/user.rb | 4 +- app/models/user_email.rb | 10 +--- ...29120025_add_email_lower_to_user_emails.rb | 18 -------- db/schema.rb | 46 +++++++++++++++---- spec/models/user_email_spec.rb | 11 ----- 6 files changed, 41 insertions(+), 50 deletions(-) delete mode 100644 db/migrate/20120129120025_add_email_lower_to_user_emails.rb diff --git a/app/models/comment.rb b/app/models/comment.rb index b51501258..683b7c742 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -28,7 +28,7 @@ class Comment < ActiveRecord::Base self.commentable.subscribes.create(:user => self.user) if !self.commentable.subscribes.exists?(:user_id => self.user.id) elsif self.commentable.class == Grit::Commit recipients = self.project.relations.by_role('admin').where(:object_type => 'User').map &:object # admins - recipients << self.user << UserEmail.where(:email_lower => self.commentable.committer.email.downcase).first.try(:user) # commentor and committer + recipients << self.user << UserEmail.where(:email => self.commentable.committer.email).first.try(:user) # commentor and committer recipients << self.project.owner if self.project.owner_type == 'User' # project owner recipients.compact.uniq.each do |user| options = {:project_id => self.project.id, :subscribeable_id => self.commentable.id, :subscribeable_type => self.commentable.class.name, :user_id => user.id} diff --git a/app/models/user.rb b/app/models/user.rb index 6e77e08b2..3296f0dda 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -60,8 +60,8 @@ class User < ActiveRecord::Base def find_for_database_authentication(warden_conditions) conditions = warden_conditions.dup login = conditions.delete(:login) - where(conditions).where("(lower(uname) = :value OR \ - exists (select null from user_emails m where m.user_id = m.user_id and m.email_lower = :value))", + where(conditions).where("lower(uname) = :value OR " + + "exists (select null from user_emails m where m.user_id = m.user_id and lower(m.email) = :value)", {:value => login.downcase}).first end diff --git a/app/models/user_email.rb b/app/models/user_email.rb index a7eb17b1c..2cec06022 100644 --- a/app/models/user_email.rb +++ b/app/models/user_email.rb @@ -3,15 +3,7 @@ class UserEmail < ActiveRecord::Base belongs_to :user - validates :email_lower, :uniqueness => true + validates :email, :uniqueness => true validates :email, :presence => true - before_save :set_lower - before_validation :set_lower - - private - - def set_lower - self.email_lower = self.email.downcase - end end diff --git a/db/migrate/20120129120025_add_email_lower_to_user_emails.rb b/db/migrate/20120129120025_add_email_lower_to_user_emails.rb deleted file mode 100644 index 8983c0091..000000000 --- a/db/migrate/20120129120025_add_email_lower_to_user_emails.rb +++ /dev/null @@ -1,18 +0,0 @@ -class AddEmailLowerToUserEmails < ActiveRecord::Migration - def self.up - add_column :user_emails, :email_lower, :string - remove_index :user_emails, :email - - UserEmail.reset_column_information - UserEmail.update_all("email_lower = lower(email)") - - change_column :user_emails, :email_lower, :string, :null => false - add_index :user_emails, :email_lower - end - - def self.down - remove_column :user_emails, :email_lower - add_index :user_emails, :email - remove_index :user_emails, :email_lower - end -end diff --git a/db/schema.rb b/db/schema.rb index 1bdee6ff5..a4692cd75 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended to check this file into your version control system. -ActiveRecord::Schema.define(:version => 20120129120025) do +ActiveRecord::Schema.define(:version => 20120123161250) do create_table "arches", :force => true do |t| t.string "name", :null => false @@ -167,6 +167,13 @@ ActiveRecord::Schema.define(:version => 20120129120025) do add_index "issues", ["project_id", "serial_id"], :name => "index_issues_on_project_id_and_serial_id", :unique => true + create_table "permissions", :force => true do |t| + t.integer "right_id" + t.integer "role_id" + t.datetime "created_at" + t.datetime "updated_at" + end + create_table "platforms", :force => true do |t| t.string "description" t.string "name" @@ -263,6 +270,27 @@ ActiveRecord::Schema.define(:version => 20120129120025) do t.string "owner_type" end + create_table "rights", :force => true do |t| + t.string "name", :null => false + t.string "controller", :null => false + t.string "action", :null => false + t.datetime "created_at" + t.datetime "updated_at" + end + + create_table "role_lines", :force => true do |t| + t.integer "role_id" + t.integer "relation_id" + t.datetime "created_at" + t.datetime "updated_at" + end + + create_table "roles", :force => true do |t| + t.string "name" + t.datetime "created_at" + t.datetime "updated_at" + end + create_table "rpms", :force => true do |t| t.string "name", :null => false t.integer "arch_id", :null => false @@ -299,29 +327,29 @@ ActiveRecord::Schema.define(:version => 20120129120025) do end create_table "user_emails", :force => true do |t| - t.integer "user_id", :null => false - t.string "email", :null => false + t.integer "user_id", :null => false + t.string "email", :null => false t.datetime "created_at" t.datetime "updated_at" - t.string "email_lower", :null => false end - add_index "user_emails", ["email_lower"], :name => "index_user_emails_on_email_lower" + add_index "user_emails", ["email"], :name => "index_user_emails_on_email" add_index "user_emails", ["user_id"], :name => "index_user_emails_on_user_id" create_table "users", :force => true do |t| t.string "name" - t.string "email", :default => "", :null => false - t.string "encrypted_password", :limit => 128, :default => "", :null => false + t.string "email", :default => "", :null => false + t.string "encrypted_password", :limit => 128, :default => "", :null => false + t.string "password_salt", :default => "", :null => false t.string "reset_password_token" - t.datetime "reset_password_sent_at" + t.string "remember_token" t.datetime "remember_created_at" t.datetime "created_at" t.datetime "updated_at" t.text "ssh_key" t.string "uname" t.string "role" - t.string "language", :default => "en" + t.string "language", :default => "en" end add_index "users", ["email"], :name => "index_users_on_email", :unique => true diff --git a/spec/models/user_email_spec.rb b/spec/models/user_email_spec.rb index ecd6e82e2..3d663cd6b 100644 --- a/spec/models/user_email_spec.rb +++ b/spec/models/user_email_spec.rb @@ -13,16 +13,5 @@ describe UserEmail do @stranger.emails.create(:email => @user.emails.first.email) @stranger.emails.exists?(:email => @user.emails.first.email).should be_false end - - it 'should not create duplicate lowercase emails' do - @stranger = Factory(:user) - @stranger.emails.create(:email => @user.email.upcase) - @stranger.emails.exists?(:email => @user.email.upcase).should be_false - end - - it 'should not create too many emails' do - 15.times {|i| @user.emails.create(:email => Factory.next(:email))} - @user.emails.count.should be_equal(UserEmail::MAX_EMAILS) - end end end From dc39eefe62bbb014b528df521bc00900e6dbaa7f Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Mon, 30 Jan 2012 12:16:09 +0600 Subject: [PATCH 28/35] Revert "[refs #114] add js" This reverts commit 48a9c4d50b9f11a11cc4f2c5831c4b5b25fdc4d1. --- app/controllers/user_emails_controller.rb | 29 ++++---------- app/models/user.rb | 4 +- app/models/user_email.rb | 2 - app/views/devise/registrations/edit.html.haml | 2 +- app/views/users/emails/_form.html.haml | 19 +++------- app/views/users/emails/emails.html.haml | 3 +- config/locales/en.yml | 1 - config/locales/ru.yml | 3 +- config/routes.rb | 5 +-- public/javascripts/emails.js | 38 ------------------- spec/models/user_email_spec.rb | 14 +------ 11 files changed, 22 insertions(+), 98 deletions(-) delete mode 100644 public/javascripts/emails.js diff --git a/app/controllers/user_emails_controller.rb b/app/controllers/user_emails_controller.rb index 3fcaa13b8..64ba07341 100644 --- a/app/controllers/user_emails_controller.rb +++ b/app/controllers/user_emails_controller.rb @@ -2,33 +2,20 @@ class UserEmailsController < ApplicationController layout 'sessions' before_filter :authenticate_user! - respond_to :html - - #def create - # current_user.emails.new - # respond_with current_user, :location => edit_user_emails_path - #end def edit - @emails = current_user.emails + (5 - current_user.emails.count).times {current_user.emails.build } render 'users/emails/emails' end def update - if current_user.update_attributes params[:user] - flash[:notice] = t('flash.user.emails.saved') - else - flash[:error] = t('flash.user.emails.error') - end - respond_with current_user, :location => edit_user_emails_path - end + new_emails = [] + params[:user][:emails_attributes].each_value {|x| new_emails << x[:email] if x[:email].present?} + emails = current_user.emails + emails.each {|e| e.destroy unless new_emails.include?(e.email)} + new_emails.each {|e| emails.create(:email => e) unless emails.include? e} - def destroy -# if current_user.emails.where(:id => params[:email_id]).first.destroy -# flash[:notice] = t('flash.user.emails.delete') -# else - flash[:error] = t('flash.user.save_error') -# end - respond_with current_user, :location => edit_user_emails_path + flash[:notice] = t('flash.user.emails.saved') + redirect_to edit_user_emails_path end end diff --git a/app/models/user.rb b/app/models/user.rb index 3296f0dda..d6e9886a1 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -36,11 +36,11 @@ class User < ActiveRecord::Base validates :role, :inclusion => {:in => ROLES}, :allow_blank => true validates :language, :inclusion => {:in => LANGUAGES}, :allow_blank => true - attr_accessible :email, :password, :password_confirmation, :remember_me, :login, :name, :ssh_key, :uname, :language, :emails_attributes + attr_accessible :email, :password, :password_confirmation, :remember_me, :login, :name, :ssh_key, :uname, :language attr_readonly :uname attr_accessor :login - accepts_nested_attributes_for :emails, :allow_destroy => true, :reject_if => proc {|attributes| attributes['email'].blank?} + accepts_nested_attributes_for :emails, :allow_destroy => true after_create :create_settings_notifier after_create :add_user_email diff --git a/app/models/user_email.rb b/app/models/user_email.rb index 2cec06022..43c2b7adf 100644 --- a/app/models/user_email.rb +++ b/app/models/user_email.rb @@ -1,6 +1,4 @@ class UserEmail < ActiveRecord::Base - MAX_EMAILS = 10 - belongs_to :user validates :email, :uniqueness => true diff --git a/app/views/devise/registrations/edit.html.haml b/app/views/devise/registrations/edit.html.haml index 10f756d6a..090c2c1e2 100644 --- a/app/views/devise/registrations/edit.html.haml +++ b/app/views/devise/registrations/edit.html.haml @@ -78,4 +78,4 @@ = link_to t('layout.settings.notifier'), user_settings_notifier_path(current_user)#, :class => "text_button_padding link_button" .group.navform.wat-cf - = link_to t('layout.settings.emails'), edit_user_emails_path#, :class => "text_button_padding link_button" + = link_to t('layout.settings.emails'), edit_user_emails_path(current_user.id)#, :class => "text_button_padding link_button" diff --git a/app/views/users/emails/_form.html.haml b/app/views/users/emails/_form.html.haml index 03da31108..3fdb58400 100644 --- a/app/views/users/emails/_form.html.haml +++ b/app/views/users/emails/_form.html.haml @@ -1,22 +1,15 @@ -- @emails.each do |email| - = f.fields_for :emails, email do |m| - .group.wat-cf.emails - = m.text_field :email, :class => :text_field, :style => "width: 300px;" - = link_to image_tag("web-app-theme/icons/cross.png", :alt => t("layout.settings.email.delete")), email.id ? destroy_user_email_path(email.id) : '#', :class => "link_button delete_button" += f.fields_for :emails do |m| + .group + .left + = m.text_field :email, :class => :text_field + .right .group.navform.wat-cf - %button.button{:type => "submit"} = image_tag("web-app-theme/icons/tick.png", :alt => t("layout.save")) = t("layout.save") - - - %button.button{:type => "submit", :id => 'add_button'} - = image_tag("web-app-theme/icons/tick.png", :alt => t("layout.add")) - = t("layout.add") - %span.text_button_padding= t("layout.or") - = link_to t("layout.cancel"), edit_user_emails_path, :class => "text_button_padding link_button" + = link_to t("layout.cancel"), edit_user_emails_path(current_user.id), :class => "text_button_padding link_button" .group.navform.wat-cf %span.text_button_padding = link_to t('layout.back'), edit_user_registration_path, :class => "text_button_padding link_button" diff --git a/app/views/users/emails/emails.html.haml b/app/views/users/emails/emails.html.haml index f1b58d386..096e1055d 100644 --- a/app/views/users/emails/emails.html.haml +++ b/app/views/users/emails/emails.html.haml @@ -1,6 +1,5 @@ -= javascript_include_tag 'emails' #block-signup.block %h2= title t("layout.settings.email.list") .content - = form_for current_user, :url => update_user_emails_path(@user), :html => {:class => 'form', :method => :put, :id => current_user.id } do |f| + = form_for current_user, :url => update_user_emails_path(@user), :html => {:method => :put, :id => current_user.id } do |f| = render :partial => "users/emails/form", :locals => {:f => f} diff --git a/config/locales/en.yml b/config/locales/en.yml index 3008dad59..5cebbccc9 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -436,7 +436,6 @@ en: emails: saved: User emails saved error: That emails addresses is already in use - delete: User email delete group: saved: Group saved save_error: Group saves error diff --git a/config/locales/ru.yml b/config/locales/ru.yml index b5c20665a..a558ceb75 100644 --- a/config/locales/ru.yml +++ b/config/locales/ru.yml @@ -430,8 +430,7 @@ ru: destroyed: Учетная запись успешно удалена emails: saved: Список еmail успешно сохранен - error: Такой адрес уже используется - delete: Email удален + error: Такой email уже используется group: saved: Группа успешно сохранена diff --git a/config/routes.rb b/config/routes.rb index 4343134eb..671ffa1f9 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -15,9 +15,8 @@ Rosa::Application.routes.draw do end end - match 'user/emails' => 'user_emails#edit', :as => :edit_user_emails, :via => :get, :action => :edit - match 'user/emails' => 'user_emails#update', :as => :update_user_emails, :via => :put, :action => :update - match 'user/emails/:email_id' => 'user_emails#destroy', :as => :destroy_user_email, :via => :post, :action => :destroy + match 'users/:id/emails' => 'user_emails#edit', :as => :edit_user_emails, :via => :get, :action => :edit + match 'users/:id/emails' => 'user_emails#update', :as => :update_user_emails, :via => :put, :action => :update resources :event_logs, :only => :index diff --git a/public/javascripts/emails.js b/public/javascripts/emails.js deleted file mode 100644 index aa148dcee..000000000 --- a/public/javascripts/emails.js +++ /dev/null @@ -1,38 +0,0 @@ -$(function() { - - $("#add_button").click(function(){ - var c = $('.emails').length; - var clone = $('.emails:first').clone(); - clone.find('input') - .removeAttr('id').attr("id", "user_emails_attributes_"+c+"_email") - .removeAttr('name').attr("name", "user[emails_attributes]["+c+"][email]").val(''); - clone.find(".delete_button").attr('href', "#"); - clone.insertAfter('.emails:last'); - return false; - }); - - $("a.delete_button").live("click", function(){ - function del_question() { - var p = a.parentNode; - p.parentNode.removeChild(p); - } - var a = this; - if(/#$/.test(a.href)) { del_question(); /* delete new email */} - else { /* delete exists email */ - $.ajax({ - type: "POST", - url: this.href, - //data: "authenticity_token=" + encodeURIComponent(AUTH_TOKEN), - success: function(msg){ - del_question(); - var q_id = a.href.match(/user\/emails\/(\d+)/); - if(q_id) { - var e = $('input:hidden[value='+q_id[1]+']'); - e.remove(); - } - } - }); - } - return false; - }); -}); \ No newline at end of file diff --git a/spec/models/user_email_spec.rb b/spec/models/user_email_spec.rb index 3d663cd6b..f1c88c94c 100644 --- a/spec/models/user_email_spec.rb +++ b/spec/models/user_email_spec.rb @@ -1,17 +1,5 @@ require 'spec_helper' describe UserEmail do - context 'for simple user' do - before(:each) do - @user = Factory(:user) - #@ability = Ability.new(@stranger) - @create_params = {:email => 'test@test.com'} - end - - it 'should not create duplicate email' do - @stranger = Factory(:user) - @stranger.emails.create(:email => @user.emails.first.email) - @stranger.emails.exists?(:email => @user.emails.first.email).should be_false - end - end + pending "add some examples to (or delete) #{__FILE__}" end From f6b6bbfb5454626ff322a18d68da69f0c9862548 Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Mon, 30 Jan 2012 12:19:54 +0600 Subject: [PATCH 29/35] Revert "[refs #114] emails control" This reverts commit 26fb1fcbfb843163231d8b8f0caf582bafccbe57. Conflicts: app/models/subscribe.rb app/models/user.rb spec/models/comment_for_commit_spec.rb --- app/controllers/user_emails_controller.rb | 34 +++++++++++-------- app/models/comment.rb | 2 +- app/models/user.rb | 8 +---- app/views/devise/registrations/edit.html.haml | 2 +- app/views/users/emails.html.haml | 0 app/views/users/emails/_form.html.haml | 15 -------- app/views/users/emails/emails.html.haml | 5 --- config/locales/en.yml | 9 ++--- config/locales/ru.yml | 8 +---- config/routes.rb | 6 ++-- spec/models/comment_for_commit_spec.rb | 12 +++---- 11 files changed, 35 insertions(+), 66 deletions(-) create mode 100644 app/views/users/emails.html.haml delete mode 100644 app/views/users/emails/_form.html.haml delete mode 100644 app/views/users/emails/emails.html.haml diff --git a/app/controllers/user_emails_controller.rb b/app/controllers/user_emails_controller.rb index 64ba07341..d85e930e5 100644 --- a/app/controllers/user_emails_controller.rb +++ b/app/controllers/user_emails_controller.rb @@ -1,21 +1,27 @@ # coding: UTF-8 -class UserEmailsController < ApplicationController - layout 'sessions' - before_filter :authenticate_user! +class UserEmailsController < UsersController + before_filter :find_user - def edit - (5 - current_user.emails.count).times {current_user.emails.build } - render 'users/emails/emails' + def index + @emails = @user.emails + (5 - @user.emails.count).times {|e| @emails << UserEmail.new(:user_id => @user) } end def update - new_emails = [] - params[:user][:emails_attributes].each_value {|x| new_emails << x[:email] if x[:email].present?} - emails = current_user.emails - emails.each {|e| e.destroy unless new_emails.include?(e.email)} - new_emails.each {|e| emails.create(:email => e) unless emails.include? e} - - flash[:notice] = t('flash.user.emails.saved') - redirect_to edit_user_emails_path + @user.role = params[:user][:role] + if @user.update_attributes(params[:user]) + flash[:notice] = t('flash.user.saved') + redirect_to users_path + else + flash[:error] = t('flash.user.save_error') + render :action => :edit + end end + + def destroy + @user.destroy + flash[:notice] = t("flash.user.destroyed") + redirect_to users_path + end + end diff --git a/app/models/comment.rb b/app/models/comment.rb index 683b7c742..13118e964 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -28,7 +28,7 @@ class Comment < ActiveRecord::Base self.commentable.subscribes.create(:user => self.user) if !self.commentable.subscribes.exists?(:user_id => self.user.id) elsif self.commentable.class == Grit::Commit recipients = self.project.relations.by_role('admin').where(:object_type => 'User').map &:object # admins - recipients << self.user << UserEmail.where(:email => self.commentable.committer.email).first.try(:user) # commentor and committer + recipients << self.user << User.where(:email => self.commentable.committer.email).first # commentor and committer recipients << self.project.owner if self.project.owner_type == 'User' # project owner recipients.compact.uniq.each do |user| options = {:project_id => self.project.id, :subscribeable_id => self.commentable.id, :subscribeable_type => self.commentable.class.name, :user_id => user.id} diff --git a/app/models/user.rb b/app/models/user.rb index d6e9886a1..7494b06a2 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -40,10 +40,7 @@ class User < ActiveRecord::Base attr_readonly :uname attr_accessor :login - accepts_nested_attributes_for :emails, :allow_destroy => true - after_create :create_settings_notifier - after_create :add_user_email def admin? role == 'admin' @@ -61,7 +58,7 @@ class User < ActiveRecord::Base conditions = warden_conditions.dup login = conditions.delete(:login) where(conditions).where("lower(uname) = :value OR " + - "exists (select null from user_emails m where m.user_id = m.user_id and lower(m.email) = :value)", + "exists (select null from user_emails m where users.user_id = m.user_id and lower(m.email) = :value)", {:value => login.downcase}).first end @@ -106,7 +103,4 @@ class User < ActiveRecord::Base self.create_notifier end - def add_user_email - UserEmail.create(:user_id => self.id, :email => self.email) - end end diff --git a/app/views/devise/registrations/edit.html.haml b/app/views/devise/registrations/edit.html.haml index 090c2c1e2..2202a2a97 100644 --- a/app/views/devise/registrations/edit.html.haml +++ b/app/views/devise/registrations/edit.html.haml @@ -78,4 +78,4 @@ = link_to t('layout.settings.notifier'), user_settings_notifier_path(current_user)#, :class => "text_button_padding link_button" .group.navform.wat-cf - = link_to t('layout.settings.emails'), edit_user_emails_path(current_user.id)#, :class => "text_button_padding link_button" + = link_to t('layout.settings.emails'), user_emails_path(current_user)#, :class => "text_button_padding link_button" diff --git a/app/views/users/emails.html.haml b/app/views/users/emails.html.haml new file mode 100644 index 000000000..e69de29bb diff --git a/app/views/users/emails/_form.html.haml b/app/views/users/emails/_form.html.haml deleted file mode 100644 index 3fdb58400..000000000 --- a/app/views/users/emails/_form.html.haml +++ /dev/null @@ -1,15 +0,0 @@ -= f.fields_for :emails do |m| - .group - .left - = m.text_field :email, :class => :text_field - .right - -.group.navform.wat-cf - %button.button{:type => "submit"} - = image_tag("web-app-theme/icons/tick.png", :alt => t("layout.save")) - = t("layout.save") - %span.text_button_padding= t("layout.or") - = link_to t("layout.cancel"), edit_user_emails_path(current_user.id), :class => "text_button_padding link_button" -.group.navform.wat-cf - %span.text_button_padding - = link_to t('layout.back'), edit_user_registration_path, :class => "text_button_padding link_button" diff --git a/app/views/users/emails/emails.html.haml b/app/views/users/emails/emails.html.haml deleted file mode 100644 index 096e1055d..000000000 --- a/app/views/users/emails/emails.html.haml +++ /dev/null @@ -1,5 +0,0 @@ -#block-signup.block - %h2= title t("layout.settings.email.list") - .content - = form_for current_user, :url => update_user_emails_path(@user), :html => {:method => :put, :id => current_user.id } do |f| - = render :partial => "users/emails/form", :locals => {:f => f} diff --git a/config/locales/en.yml b/config/locales/en.yml index 5cebbccc9..f8ae03bcf 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -45,10 +45,7 @@ en: notifier: Notifier setting notifiers: edit_header: Notifier setting - email: - list: Email addresses - delete: Destroy - confirm_delete: Are you sure? + email: Email addresses processing: working ... downloads: @@ -433,9 +430,7 @@ en: saved: User saved save_error: User data saves error destroyed: User account deleted - emails: - saved: User emails saved - error: That emails addresses is already in use + group: saved: Group saved save_error: Group saves error diff --git a/config/locales/ru.yml b/config/locales/ru.yml index a558ceb75..80aed8c2b 100644 --- a/config/locales/ru.yml +++ b/config/locales/ru.yml @@ -44,10 +44,7 @@ ru: notifier: Настройки оповещений notifiers: edit_header: Настройки оповещений - email: - list: Список Email - delete: Удалить - confirm_delete: Вы уверены? + emails: Список Email processing: Обрабатывается... downloads: @@ -428,9 +425,6 @@ ru: saved: Пользователь успешно сохранен save_error: Не удалось сохранить данные о пользователе destroyed: Учетная запись успешно удалена - emails: - saved: Список еmail успешно сохранен - error: Такой email уже используется group: saved: Группа успешно сохранена diff --git a/config/routes.rb b/config/routes.rb index 671ffa1f9..318af8f5f 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -8,6 +8,9 @@ Rosa::Application.routes.draw do resources :users do resources :groups, :only => [:new, :create, :index] + resources :emails, :only => [:index, :destroy], :controller => :user_emails do + put :update, :as => :member + end get :autocomplete_user_uname, :on => :collection namespace :settings do @@ -15,9 +18,6 @@ Rosa::Application.routes.draw do end end - match 'users/:id/emails' => 'user_emails#edit', :as => :edit_user_emails, :via => :get, :action => :edit - match 'users/:id/emails' => 'user_emails#update', :as => :update_user_emails, :via => :put, :action => :update - resources :event_logs, :only => :index #resources :downloads, :only => :index diff --git a/spec/models/comment_for_commit_spec.rb b/spec/models/comment_for_commit_spec.rb index c36001a03..e39f6e1b1 100644 --- a/spec/models/comment_for_commit_spec.rb +++ b/spec/models/comment_for_commit_spec.rb @@ -341,7 +341,7 @@ describe Comment do context 'for committer' do it 'should send an e-mail' do ActionMailer::Base.deliveries = [] - @stranger.emails.first.update_attribute :email, 'code@tpope.net' + @stranger.update_attribute :email, 'code@tpope.net' comment = Comment.create(:user => @user, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) ActionMailer::Base.deliveries.count.should == 1 @@ -350,7 +350,7 @@ describe Comment do it 'should send an e-mail with user locale' do ActionMailer::Base.deliveries = [] - @stranger.emails.first.update_attribute :email, 'code@tpope.net' + @stranger.update_attribute :email, 'code@tpope.net' @stranger.update_attribute :language, 'ru' comment = Comment.create(:user => @user, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) @@ -362,7 +362,7 @@ describe Comment do it 'should send a one e-mail when subscribed to commit' do ActionMailer::Base.deliveries = [] Subscribe.set_subscribe_to_commit({:project_id => @project.id, :subscribeable_id => @commit.id, :subscribeable_type => @commit.class.name, :user_id => @stranger.id}, Subscribe::ON) - @stranger.emails.first.update_attribute :email, 'code@tpope.net' + @stranger.update_attribute :email, 'code@tpope.net' comment = Comment.create(:user => @user, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) ActionMailer::Base.deliveries.count.should == 1 @@ -372,7 +372,7 @@ describe Comment do it 'should not send an e-mail for own comment' do ActionMailer::Base.deliveries = [] #@project.owner.notifier.update_attribute :can_notify, false - @stranger.emails.first.update_attribute :email, 'code@tpope.net' + @stranger.update_attribute :email, 'code@tpope.net' comment = Comment.create(:user => @stranger, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) ActionMailer::Base.deliveries.count.should == 1 @@ -382,7 +382,7 @@ describe Comment do it 'should not send an e-mail if global notify off' do ActionMailer::Base.deliveries = [] @project.owner.notifier.update_attribute :can_notify, false - @stranger.emails.first.update_attribute :email, 'code@tpope.net' + @stranger.update_attribute :email, 'code@tpope.net' @stranger.notifier.update_attribute :can_notify, false comment = Comment.create(:user => @user, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) @@ -392,7 +392,7 @@ describe Comment do it 'should not send an e-mail if notify for my commits off' do ActionMailer::Base.deliveries = [] @stranger.notifier.update_attribute :new_comment_commit_owner, false - @stranger.emails.first.update_attribute :email, 'code@tpope.net' + @stranger.update_attribute :email, 'code@tpope.net' comment = Comment.create(:user => @user, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) ActionMailer::Base.deliveries.count.should == 0 From e0b7699ffd27bf80c0cdc2528bb60fd54440b309 Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Mon, 30 Jan 2012 12:22:54 +0600 Subject: [PATCH 30/35] Revert "Merge branch '114-comment_notifications' into user-emails-list" This reverts commit 2179cbbf40334cbe5d4fa9ba3a7f297d8f543a0a, reversing changes made to 9bf06e5e1a595f320dc4527ebd44f14be05fcd64. Conflicts: app/models/user.rb --- app/controllers/user_emails_controller.rb | 27 ------------------- app/models/user.rb | 5 +--- app/models/user_email.rb | 7 ----- app/views/devise/registrations/edit.html.haml | 3 --- app/views/users/emails.html.haml | 0 config/locales/en.yml | 1 - config/locales/ru.yml | 1 - config/routes.rb | 4 --- .../20120123051330_create_user_emails.rb | 21 --------------- spec/factories/user_emails.rb | 8 ------ spec/models/user_email_spec.rb | 5 ---- 11 files changed, 1 insertion(+), 81 deletions(-) delete mode 100644 app/controllers/user_emails_controller.rb delete mode 100644 app/models/user_email.rb delete mode 100644 app/views/users/emails.html.haml delete mode 100644 db/migrate/20120123051330_create_user_emails.rb delete mode 100644 spec/factories/user_emails.rb delete mode 100644 spec/models/user_email_spec.rb diff --git a/app/controllers/user_emails_controller.rb b/app/controllers/user_emails_controller.rb deleted file mode 100644 index d85e930e5..000000000 --- a/app/controllers/user_emails_controller.rb +++ /dev/null @@ -1,27 +0,0 @@ -# coding: UTF-8 -class UserEmailsController < UsersController - before_filter :find_user - - def index - @emails = @user.emails - (5 - @user.emails.count).times {|e| @emails << UserEmail.new(:user_id => @user) } - end - - def update - @user.role = params[:user][:role] - if @user.update_attributes(params[:user]) - flash[:notice] = t('flash.user.saved') - redirect_to users_path - else - flash[:error] = t('flash.user.save_error') - render :action => :edit - end - end - - def destroy - @user.destroy - flash[:notice] = t("flash.user.destroyed") - redirect_to users_path - end - -end diff --git a/app/models/user.rb b/app/models/user.rb index 7494b06a2..680c187f8 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -26,7 +26,6 @@ class User < ActiveRecord::Base has_many :subscribes, :foreign_key => :user_id, :dependent => :destroy has_many :comments, :dependent => :destroy - has_many :emails, :class_name => 'UserEmail', :dependent => :destroy include Modules::Models::PersonalRepository @@ -57,9 +56,7 @@ class User < ActiveRecord::Base def find_for_database_authentication(warden_conditions) conditions = warden_conditions.dup login = conditions.delete(:login) - where(conditions).where("lower(uname) = :value OR " + - "exists (select null from user_emails m where users.user_id = m.user_id and lower(m.email) = :value)", - {:value => login.downcase}).first + where(conditions).where(["lower(uname) = :value OR lower(email) = :value", { :value => login.downcase }]).first end def new_with_session(params, session) diff --git a/app/models/user_email.rb b/app/models/user_email.rb deleted file mode 100644 index 43c2b7adf..000000000 --- a/app/models/user_email.rb +++ /dev/null @@ -1,7 +0,0 @@ -class UserEmail < ActiveRecord::Base - belongs_to :user - - validates :email, :uniqueness => true - validates :email, :presence => true - -end diff --git a/app/views/devise/registrations/edit.html.haml b/app/views/devise/registrations/edit.html.haml index 2202a2a97..b983f1836 100644 --- a/app/views/devise/registrations/edit.html.haml +++ b/app/views/devise/registrations/edit.html.haml @@ -76,6 +76,3 @@ .group.navform.wat-cf = link_to t('layout.settings.notifier'), user_settings_notifier_path(current_user)#, :class => "text_button_padding link_button" - - .group.navform.wat-cf - = link_to t('layout.settings.emails'), user_emails_path(current_user)#, :class => "text_button_padding link_button" diff --git a/app/views/users/emails.html.haml b/app/views/users/emails.html.haml deleted file mode 100644 index e69de29bb..000000000 diff --git a/config/locales/en.yml b/config/locales/en.yml index f8ae03bcf..a92b84bab 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -45,7 +45,6 @@ en: notifier: Notifier setting notifiers: edit_header: Notifier setting - email: Email addresses processing: working ... downloads: diff --git a/config/locales/ru.yml b/config/locales/ru.yml index 80aed8c2b..ed32edbf5 100644 --- a/config/locales/ru.yml +++ b/config/locales/ru.yml @@ -44,7 +44,6 @@ ru: notifier: Настройки оповещений notifiers: edit_header: Настройки оповещений - emails: Список Email processing: Обрабатывается... downloads: diff --git a/config/routes.rb b/config/routes.rb index 318af8f5f..4ee716fe8 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -8,10 +8,6 @@ Rosa::Application.routes.draw do resources :users do resources :groups, :only => [:new, :create, :index] - resources :emails, :only => [:index, :destroy], :controller => :user_emails do - put :update, :as => :member - end - get :autocomplete_user_uname, :on => :collection namespace :settings do resource :notifier, :only => [:show, :update] diff --git a/db/migrate/20120123051330_create_user_emails.rb b/db/migrate/20120123051330_create_user_emails.rb deleted file mode 100644 index b148e8d56..000000000 --- a/db/migrate/20120123051330_create_user_emails.rb +++ /dev/null @@ -1,21 +0,0 @@ -class CreateUserEmails < ActiveRecord::Migration - def self.up - create_table :user_emails do |t| - t.integer :user_id, :null => false - t.string :email, :null => false - - t.timestamps - end - - add_index :user_emails, :user_id - add_index :user_emails, :email - UserEmail.reset_column_information - User.all.each do |u| - UserEmail.create(:user_id => u.id, :email => u.email) - end - end - - def self.down - drop_table :user_emails - end -end diff --git a/spec/factories/user_emails.rb b/spec/factories/user_emails.rb deleted file mode 100644 index ed339e424..000000000 --- a/spec/factories/user_emails.rb +++ /dev/null @@ -1,8 +0,0 @@ -# Read about factories at http://github.com/thoughtbot/factory_girl - -FactoryGirl.define do - factory :user_email do - user_id 1 - email "MyString" - end -end \ No newline at end of file diff --git a/spec/models/user_email_spec.rb b/spec/models/user_email_spec.rb deleted file mode 100644 index f1c88c94c..000000000 --- a/spec/models/user_email_spec.rb +++ /dev/null @@ -1,5 +0,0 @@ -require 'spec_helper' - -describe UserEmail do - pending "add some examples to (or delete) #{__FILE__}" -end From 8962e85b2dcf7721e9a592485165626541edbb68 Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Mon, 30 Jan 2012 12:38:23 +0600 Subject: [PATCH 31/35] [refs #114] fix reverting --- app/models/user.rb | 2 +- spec/models/comment_for_commit_spec.rb | 11 ----------- 2 files changed, 1 insertion(+), 12 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 680c187f8..68cc966a8 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -91,7 +91,7 @@ class User < ActiveRecord::Base end def committer?(commit) - emails.exists? :email_lower => commit.committer.email.downcase + email.downcase == commit.committer.email.downcase end private diff --git a/spec/models/comment_for_commit_spec.rb b/spec/models/comment_for_commit_spec.rb index e39f6e1b1..f5eed5cf7 100644 --- a/spec/models/comment_for_commit_spec.rb +++ b/spec/models/comment_for_commit_spec.rb @@ -348,17 +348,6 @@ describe Comment do ActionMailer::Base.deliveries.last.to.include?(@stranger.email).should == true end - it 'should send an e-mail with user locale' do - ActionMailer::Base.deliveries = [] - @stranger.update_attribute :email, 'code@tpope.net' - @stranger.update_attribute :language, 'ru' - comment = Comment.create(:user => @user, :body => 'hello!', :project => @project, - :commentable_type => @commit.class.name, :commentable_id => @commit.id) - ActionMailer::Base.deliveries.count.should == 1 - ActionMailer::Base.deliveries.last.to.include?(@stranger.email).should == true - ActionMailer::Base.deliveries.last.subject.include?('Новый комментарий к коммиту').should == true - end - it 'should send a one e-mail when subscribed to commit' do ActionMailer::Base.deliveries = [] Subscribe.set_subscribe_to_commit({:project_id => @project.id, :subscribeable_id => @commit.id, :subscribeable_type => @commit.class.name, :user_id => @stranger.id}, Subscribe::ON) From 119c378149e988ed71147ba072ad56948c85d063 Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Mon, 30 Jan 2012 17:59:57 +0600 Subject: [PATCH 32/35] [refs #114] change subsrcibe status to boolean --- .../commit_subscribes_controller.rb | 4 +-- app/models/comment.rb | 2 +- app/models/subscribe.rb | 25 ++++++++++++------- ...20120130111133_change_status_subscribes.rb | 11 ++++++++ db/schema.rb | 4 +-- spec/models/comment_for_commit_spec.rb | 10 ++++---- 6 files changed, 37 insertions(+), 19 deletions(-) create mode 100644 db/migrate/20120130111133_change_status_subscribes.rb diff --git a/app/controllers/commit_subscribes_controller.rb b/app/controllers/commit_subscribes_controller.rb index 4d33c06ee..c50902dca 100644 --- a/app/controllers/commit_subscribes_controller.rb +++ b/app/controllers/commit_subscribes_controller.rb @@ -6,7 +6,7 @@ class CommitSubscribesController < ApplicationController before_filter :find_commit def create - if Subscribe.set_subscribe_to_commit(@options, Subscribe::ON) + if Subscribe.subscribe_to_commit(@options) flash[:notice] = I18n.t("flash.subscribe.commit.saved") # TODO js redirect_to commit_path(@project, @commit) @@ -17,7 +17,7 @@ class CommitSubscribesController < ApplicationController end def destroy - Subscribe.set_subscribe_to_commit(@options, Subscribe::OFF) + Subscribe.unsubscribe_from_commit(@options) flash[:notice] = t("flash.subscribe.commit.destroyed") redirect_to commit_path(@project, @commit) end diff --git a/app/models/comment.rb b/app/models/comment.rb index 13118e964..e1e0ad22f 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -32,7 +32,7 @@ class Comment < ActiveRecord::Base recipients << self.project.owner if self.project.owner_type == 'User' # project owner recipients.compact.uniq.each do |user| options = {:project_id => self.project.id, :subscribeable_id => self.commentable.id, :subscribeable_type => self.commentable.class.name, :user_id => user.id} - Subscribe.set_subscribe_to_commit(options, Subscribe::ON) if Subscribe.subscribed_to_commit?(self.project, user, self.commentable) + Subscribe.subscribe_to_commit(options) if Subscribe.subscribed_to_commit?(self.project, user, self.commentable) end end end diff --git a/app/models/subscribe.rb b/app/models/subscribe.rb index b0c31a216..66b5727b1 100644 --- a/app/models/subscribe.rb +++ b/app/models/subscribe.rb @@ -1,18 +1,12 @@ class Subscribe < ActiveRecord::Base - ON = 1 - OFF = 0 belongs_to :subscribeable, :polymorphic => true belongs_to :user belongs_to :project - validates :status, :inclusion => {:in => 0..1} - - scope :on, where(:status => ON) - scope :off, where(:status => OFF) scope :finder_hack, order('') # FIXME .subscribes - error; .subscribes.finder_hack - success Oo def subscribed? - status == ON + status end def self.comment_subscribes(comment) @@ -33,7 +27,7 @@ class Subscribe < ActiveRecord::Base end def self.new_comment_commit_notification(comment) - subscribes = Subscribe.comment_subscribes(comment).on + subscribes = Subscribe.comment_subscribes(comment).where(:status => true) subscribes.each do |subscribe| next if comment.own_comment?(subscribe.user) || !subscribe.user.notifier.can_notify UserMailer.delay.new_comment_notification(comment, subscribe.user) @@ -49,6 +43,18 @@ class Subscribe < ActiveRecord::Base (user.committer?(commit) && user.notifier.new_comment_commit_owner) end + + def self.subscribe_to_commit(options) + Subscribe.set_subscribe_to_commit(options, true) + end + + + def self.unsubscribe_from_commit(options) + Subscribe.set_subscribe_to_commit(options, false) + end + + private + def self.set_subscribe_to_commit(options, status) if subscribe = Subscribe.where(options).first subscribe.update_attribute(:status, status) @@ -56,4 +62,5 @@ class Subscribe < ActiveRecord::Base Subscribe.create(options.merge(:status => status)) end end -end + +end \ No newline at end of file diff --git a/db/migrate/20120130111133_change_status_subscribes.rb b/db/migrate/20120130111133_change_status_subscribes.rb new file mode 100644 index 000000000..9d31898c8 --- /dev/null +++ b/db/migrate/20120130111133_change_status_subscribes.rb @@ -0,0 +1,11 @@ +class ChangeStatusSubscribes < ActiveRecord::Migration + def self.up + remove_column :subscribes, :status + add_column :subscribes, :status, :boolean, :default => true + end + + def self.down + remove_column :subscribes, :status + add_column :subscribes, :status, :integer, :default => 1 + end +end diff --git a/db/schema.rb b/db/schema.rb index db5afc7d7..b0c0d3a73 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended to check this file into your version control system. -ActiveRecord::Schema.define(:version => 20120129120025) do +ActiveRecord::Schema.define(:version => 20120130111133) do create_table "arches", :force => true do |t| t.string "name", :null => false @@ -299,8 +299,8 @@ ActiveRecord::Schema.define(:version => 20120129120025) do t.integer "user_id" t.datetime "created_at" t.datetime "updated_at" - t.integer "status", :default => 1 t.integer "project_id" + t.boolean "status", :default => true end create_table "users", :force => true do |t| diff --git a/spec/models/comment_for_commit_spec.rb b/spec/models/comment_for_commit_spec.rb index f5eed5cf7..c190b9f77 100644 --- a/spec/models/comment_for_commit_spec.rb +++ b/spec/models/comment_for_commit_spec.rb @@ -131,7 +131,7 @@ describe Comment do context 'for unsubscribe commit' do it 'should not send an e-mail' do ActionMailer::Base.deliveries = [] - Subscribe.set_subscribe_to_commit({:project_id => @project.id, :subscribeable_id => @commit.id, :subscribeable_type => @commit.class.name, :user_id => @user.id}, Subscribe::OFF) + Subscribe.unsubscribe_from_commit(:project_id => @project.id, :subscribeable_id => @commit.id, :subscribeable_type => @commit.class.name, :user_id => @user.id) comment = Comment.create(:user => @stranger, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) ActionMailer::Base.deliveries.count.should == 0 # cache project.commit_comments_subscribes ... @@ -234,7 +234,7 @@ describe Comment do context 'for unsubscribe project' do it 'should not send an e-mail' do ActionMailer::Base.deliveries = [] - Subscribe.set_subscribe_to_commit({:project_id => @project.id, :subscribeable_id => @commit.id, :subscribeable_type => @commit.class.name, :user_id => @user.id}, Subscribe::OFF) + Subscribe.unsubscribe_from_commit(:project_id => @project.id, :subscribeable_id => @commit.id, :subscribeable_type => @commit.class.name, :user_id => @user.id) comment = Comment.create(:user => @stranger, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) ActionMailer::Base.deliveries.count.should == 0 @@ -321,7 +321,7 @@ describe Comment do @stranger.notifier.update_attribute :new_comment_commit_owner, false #@stranger.notifier.update_attribute :new_comment_commit_commentor, false - Subscribe.set_subscribe_to_commit({:project_id => @project.id, :subscribeable_id => @commit.id, :subscribeable_type => @commit.class.name, :user_id => @stranger.id}, Subscribe::ON) + Subscribe.subscribe_to_commit(:project_id => @project.id, :subscribeable_id => @commit.id, :subscribeable_type => @commit.class.name, :user_id => @stranger.id) comment = Comment.create(:user => @project.owner, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) ActionMailer::Base.deliveries.count.should == 1 @@ -331,7 +331,7 @@ describe Comment do it 'should not send an e-mail for own comment' do ActionMailer::Base.deliveries = [] #@project.owner.notifier.update_attribute :can_notify, false - Subscribe.set_subscribe_to_commit({:project_id => @project.id, :subscribeable_id => @commit.id, :subscribeable_type => @commit.class.name, :user_id => @stranger.id}, Subscribe::ON) + Subscribe.subscribe_to_commit(:project_id => @project.id, :subscribeable_id => @commit.id, :subscribeable_type => @commit.class.name, :user_id => @stranger.id) comment = Comment.create(:user => @owner, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) ActionMailer::Base.deliveries.count.should == 0 @@ -350,7 +350,7 @@ describe Comment do it 'should send a one e-mail when subscribed to commit' do ActionMailer::Base.deliveries = [] - Subscribe.set_subscribe_to_commit({:project_id => @project.id, :subscribeable_id => @commit.id, :subscribeable_type => @commit.class.name, :user_id => @stranger.id}, Subscribe::ON) + Subscribe.subscribe_to_commit(:project_id => @project.id, :subscribeable_id => @commit.id, :subscribeable_type => @commit.class.name, :user_id => @stranger.id) @stranger.update_attribute :email, 'code@tpope.net' comment = Comment.create(:user => @user, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) From aa9652409cf267fe05bb06833621ced0097f30f6 Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Tue, 31 Jan 2012 21:43:42 +0600 Subject: [PATCH 33/35] [refs #114] clean --- db/migrate/20120123120400_add_status_to_subscribe.rb | 2 +- db/migrate/20120130111133_change_status_subscribes.rb | 11 ----------- db/schema.rb | 2 +- spec/models/comment_for_commit_spec.rb | 10 ++-------- 4 files changed, 4 insertions(+), 21 deletions(-) delete mode 100644 db/migrate/20120130111133_change_status_subscribes.rb diff --git a/db/migrate/20120123120400_add_status_to_subscribe.rb b/db/migrate/20120123120400_add_status_to_subscribe.rb index 27fef24af..69f24e6f9 100644 --- a/db/migrate/20120123120400_add_status_to_subscribe.rb +++ b/db/migrate/20120123120400_add_status_to_subscribe.rb @@ -1,6 +1,6 @@ class AddStatusToSubscribe < ActiveRecord::Migration def self.up - add_column :subscribes, :status, :integer, :default => 1 + add_column :subscribes, :status, :boolean, :default => true end def self.down diff --git a/db/migrate/20120130111133_change_status_subscribes.rb b/db/migrate/20120130111133_change_status_subscribes.rb deleted file mode 100644 index 9d31898c8..000000000 --- a/db/migrate/20120130111133_change_status_subscribes.rb +++ /dev/null @@ -1,11 +0,0 @@ -class ChangeStatusSubscribes < ActiveRecord::Migration - def self.up - remove_column :subscribes, :status - add_column :subscribes, :status, :boolean, :default => true - end - - def self.down - remove_column :subscribes, :status - add_column :subscribes, :status, :integer, :default => 1 - end -end diff --git a/db/schema.rb b/db/schema.rb index 754823099..8c4af8b8f 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended to check this file into your version control system. -ActiveRecord::Schema.define(:version => 20120130111133) do +ActiveRecord::Schema.define(:version => 20120127234602) do create_table "arches", :force => true do |t| t.string "name", :null => false diff --git a/spec/models/comment_for_commit_spec.rb b/spec/models/comment_for_commit_spec.rb index 7be5c2a06..6f693d03e 100644 --- a/spec/models/comment_for_commit_spec.rb +++ b/spec/models/comment_for_commit_spec.rb @@ -155,11 +155,8 @@ describe Comment do before(:each) do @user = Factory(:user) @stranger = Factory(:user) - set_comments_data_for_commit - @project.update_attribute(:owner, @user) - #@project.relations.create!(:object_type => 'User', :object_id => @user.id, :role => 'admin') end it 'should create comment' do @@ -204,7 +201,7 @@ describe Comment do @user.notifier.update_attribute :new_comment_commit_owner, false comment = Comment.create(:user => @stranger, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) - ActionMailer::Base.deliveries.count.should == 1 # cache project.commit_comments_subscribes ... + ActionMailer::Base.deliveries.count.should == 1 ActionMailer::Base.deliveries.last.to.include?(@user.email).should == true end end @@ -215,7 +212,7 @@ describe Comment do @user.notifier.update_attribute :new_comment_commit_commentor, false comment = Comment.create(:user => @stranger, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) - ActionMailer::Base.deliveries.count.should == 1 # cache project.commit_comments_subscribes ... + ActionMailer::Base.deliveries.count.should == 1 ActionMailer::Base.deliveries.last.to.include?(@user.email).should == true end end @@ -320,7 +317,6 @@ describe Comment do @project.owner.notifier.update_attribute :can_notify, false @stranger.notifier.update_attribute :new_comment_commit_repo_owner, false @stranger.notifier.update_attribute :new_comment_commit_owner, false - #@stranger.notifier.update_attribute :new_comment_commit_commentor, false Subscribe.subscribe_to_commit(:project_id => @project.id, :subscribeable_id => @commit.id, :subscribeable_type => @commit.class.name, :user_id => @stranger.id) comment = Comment.create(:user => @project.owner, :body => 'hello!', :project => @project, @@ -331,7 +327,6 @@ describe Comment do it 'should not send an e-mail for own comment' do ActionMailer::Base.deliveries = [] - #@project.owner.notifier.update_attribute :can_notify, false Subscribe.subscribe_to_commit(:project_id => @project.id, :subscribeable_id => @commit.id, :subscribeable_type => @commit.class.name, :user_id => @stranger.id) comment = Comment.create(:user => @owner, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) @@ -361,7 +356,6 @@ describe Comment do it 'should not send an e-mail for own comment' do ActionMailer::Base.deliveries = [] - #@project.owner.notifier.update_attribute :can_notify, false @stranger.update_attribute :email, 'code@tpope.net' comment = Comment.create(:user => @stranger, :body => 'hello!', :project => @project, :commentable_type => @commit.class.name, :commentable_id => @commit.id) From cb7a9fcc218cd8f318e6fed0103a3671dbc7939e Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Thu, 2 Feb 2012 02:42:23 +0600 Subject: [PATCH 34/35] fix group autocomplete --- app/models/ability.rb | 2 +- app/models/group.rb | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/app/models/ability.rb b/app/models/ability.rb index f2412a566..735aaed2a 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -31,7 +31,7 @@ class Ability can [:show, :update], Settings::Notifier, :user_id => user.id - can [:read, :create], Group + can [:read, :create, :autocomplete_group_uname], Group can [:update, :manage_members], Group do |group| group.objects.exists?(:object_type => 'User', :object_id => user.id, :role => 'admin') # or group.owner_id = user.id end diff --git a/app/models/group.rb b/app/models/group.rb index 999c4a626..ccd4db52e 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -22,7 +22,6 @@ class Group < ActiveRecord::Base delegate :ssh_key, :to => :owner after_create :add_owner_to_members - after_initialize lambda {|r| r.name ||= r.uname } # default include Modules::Models::PersonalRepository # include Modules::Models::Owner From 0006bf3b63e19dffc55a2fffdfe5224479d07fde Mon Sep 17 00:00:00 2001 From: George Vinogradov Date: Thu, 2 Feb 2012 01:20:10 +0400 Subject: [PATCH 35/35] Returned before_initialize to Group. --- app/models/group.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/models/group.rb b/app/models/group.rb index ccd4db52e..999c4a626 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -22,6 +22,7 @@ class Group < ActiveRecord::Base delegate :ssh_key, :to => :owner after_create :add_owner_to_members + after_initialize lambda {|r| r.name ||= r.uname } # default include Modules::Models::PersonalRepository # include Modules::Models::Owner