allow invited users to be watchers (#5520)

[ci skip]

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.

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

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

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