diff --git a/app/models/principal.rb b/app/models/principal.rb index 5aeb0e9609..b1ea1a64f3 100644 --- a/app/models/principal.rb +++ b/app/models/principal.rb @@ -131,6 +131,10 @@ class Principal < ActiveRecord::Base 'active' end + def active_or_registered? + [STATUSES[:active], STATUSES[:registered], STATUSES[:invited]].include?(status) + end + ## # Allows the API and other sources to determine locking actions # on represented collections of children of Principals. diff --git a/app/models/watcher.rb b/app/models/watcher.rb index 4102822d83..184f39a7b9 100644 --- a/app/models/watcher.rb +++ b/app/models/watcher.rb @@ -50,7 +50,7 @@ class Watcher < ActiveRecord::Base def validate_active_user # TODO add informative error message return if user.blank? - errors.add :user_id, :invalid unless user.active? + errors.add :user_id, :invalid unless user.active_or_registered? end def validate_user_allowed_to_watch diff --git a/lib/services/create_watcher.rb b/lib/services/create_watcher.rb index d8a529b5f2..739ff4c66e 100644 --- a/lib/services/create_watcher.rb +++ b/lib/services/create_watcher.rb @@ -37,16 +37,14 @@ class Services::CreateWatcher def run(success: -> {}, failure: -> {}) if @work_package.watcher_users.include?(@user) success.(created: false) + elsif @watcher.valid? + @work_package.watchers << @watcher + success.(created: true) + OpenProject::Notifications.send('watcher_added', + watcher: @watcher, + watcher_setter: User.current) else - if @watcher.valid? - @work_package.watchers << @watcher - success.(created: true) - OpenProject::Notifications.send('watcher_added', - watcher: @watcher, - watcher_setter: User.current) - else - failure.(@watcher) - end + failure.(@watcher) end end end diff --git a/spec/models/watcher_spec.rb b/spec/models/watcher_spec.rb index 426378a7e3..9e0e98c23e 100644 --- a/spec/models/watcher_spec.rb +++ b/spec/models/watcher_spec.rb @@ -45,6 +45,22 @@ describe Watcher, type: :model do let(:other_project) { FactoryGirl.create(:project) } let(:other_user) { FactoryGirl.create(:user, admin: true) } + describe '#valid' do + it 'is valid for an active user' do + expect(watcher).to be_valid + end + + it 'is valid for an invited user' do + user.status = Principal::STATUSES[:invited] + expect(watcher).to be_valid + end + + it 'is valid for a registered user' do + user.status = Principal::STATUSES[:registered] + expect(watcher).to be_valid + end + end + describe '.prune' do shared_examples_for 'a pruned watchable' do before do