Allow space administrators to manage sources in their spaces

Author: fbacall

app/controllers/sources_controller.rb

@@ -24,7 +24,7 @@ def show
 
   # GET /sources/new
   def new
-    authorize Source
+    authorize @content_provider, :create_source?
     @source = @content_provider.sources.build
   end
 
@@ -36,7 +36,7 @@ def edit
   # POST /sources
   # POST /sources.json
   def create
-    authorize Source
+    authorize @content_provider, :create_source?
     @source = @content_provider.sources.build(source_params)
     @source.user = current_user
     @source.space = current_space
@@ -146,13 +146,12 @@ def set_source
   def set_content_provider
     @content_provider = @source.content_provider if @source
     @content_provider ||= ContentProvider.friendly.find(params[:content_provider_id])
-    authorize @content_provider, :manage?
   end
 
   # Never trust parameters from the scary internet, only allow the white list through.
   def source_params
     permitted = [:url, :method, :token, :default_language, :enabled]
-    permitted << :approval_status if policy(Source).approve?
+    permitted << :approval_status if policy(@source || Source).approve?
     permitted << :content_provider_id if policy(Source).index?
 
     params.require(:source).permit(permitted)

app/mailers/curation_mailer.rb

@@ -17,7 +17,8 @@ def source_requires_approval(source, user)
     @user = user
     @source = source
     subject = "#{TeSS::Config.site['title_short']} source \"#{@source.title}\" requires approval"
-    mail(subject:, to: User.with_role('admin').map(&:email)) do |format|
+    space = @source.space || Space.default
+    mail(subject:, to: space.administrators.map(&:email)) do |format|
       format.html
       format.text
     end

app/models/default_space.rb

@@ -56,4 +56,8 @@ def learning_path_topics
   def default?
     true
   end
+
+  def administrators
+    User.with_role('admin')
+  end
 end

app/policies/application_policy.rb

@@ -16,6 +16,8 @@ def initialize(context, record)
     @user = context.user
     @request = context.request
     @record = record
+    @space = nil
+    @space = record.space if record.respond_to?(:space)
   end
 
   def index?
@@ -83,9 +85,11 @@ def scraper?
   end
 
   # Check if the user has any of the given roles.
+  # If we're in a space, also check they have any of those roles in the context of the space.
   def user_has_role?(*roles)
     return false if @user.nil?
-    roles.any? { |r| @user.has_role?(r) }
+    roles.any? { |r| @user.has_role?(r) } ||
+      (@space && roles.any? { |r| @user.has_space_role?(@space, r) })
   end
 
 end

app/policies/content_provider_policy.rb

@@ -1,3 +1,6 @@
 class ContentProviderPolicy < ScrapedResourcePolicy
-
+  def create_source?
+    TeSS::Config.feature['user_source_creation'] && manage? ||
+      user_has_role?(:admin, :curator)
+  end
 end

app/policies/source_policy.rb

@@ -13,11 +13,7 @@ def index?
   end
 
   def create?
-    if TeSS::Config.feature['user_source_creation']
-      super
-    else
-      administration?
-    end
+    administration?
   end
 
   def approve?
@@ -31,7 +27,7 @@ def request_approval?
   private
 
   def administration? # Can edit sources for any content provider
-    curators_and_admin
+    curators_and_admin || user_has_role?(:admin)
   end
 
   def user_management?

test/controllers/sources_controller_test.rb

@@ -203,6 +203,28 @@ class SourcesControllerTest < ActionController::TestCase
     assert_response :success
   end
 
+  test 'space admin should get edit for source in their space' do
+    sign_in users(:space_admin)
+    source = sources(:unapproved_source)
+    space = spaces(:plants)
+    source.space = space
+    source.save!
+
+    get :edit, params: { id: source }
+    assert_response :success
+  end
+
+  test 'space admin should not get edit for source in another space' do
+    sign_in users(:space_admin)
+    source = sources(:unapproved_source)
+    space = spaces(:astro)
+    source.space = space
+    source.save!
+
+    get :edit, params: { id: source }
+    assert_response :forbidden
+  end
+
   # CREATE Tests
   test 'public should not create source' do
     assert_no_difference 'Source.count' do
@@ -436,9 +458,48 @@ class SourcesControllerTest < ActionController::TestCase
     assert source.reload.approved?
   end
 
+  test 'space admin can approve source in their space' do
+    sign_in users(:space_admin)
+    source = sources(:unapproved_source)
+    space = spaces(:plants)
+    source.space = space
+    source.save!
+    refute source.approved?
+
+    patch :update, params: { id: source, source: { approval_status: 'approved' } }
+
+    assert_redirected_to source_path(assigns(:source))
+    assert source.reload.approved?
+  end
+
+  test 'space admin cannot approve source in other space' do
+    sign_in users(:space_admin)
+    source = sources(:unapproved_source)
+    space = spaces(:astro)
+    source.space = space
+    source.save!
+    refute source.approved?
+
+    patch :update, params: { id: source, source: { approval_status: 'approved' } }
+
+    assert_response :forbidden
+    refute source.reload.approved?
+  end
+
   test 'regular user cannot approve source' do
-    sign_in @user
+    sign_in users(:another_regular_user)
+    source = sources(:unapproved_source)
+    refute source.approved?
+
+    patch :update, params: { id: source, source: { approval_status: 'approved' } }
+
+    assert_response :forbidden
+    refute source.reload.approved?
+  end
+
+  test 'source owner cannot approve source' do
     source = sources(:unapproved_source)
+    sign_in source.user
     refute source.approved?
 
     patch :update, params: { id: source, source: { approval_status: 'approved' } }

test/mailers/curation_mailer_test.rb

@@ -249,4 +249,30 @@ class CurationMailerTest < ActionMailer::TestCase
       end
     end
   end
+
+  test 'source approval requests go to administrators' do
+    source = sources(:unapproved_source)
+    user = source.user
+    assert_nil source.space
+    email = CurationMailer.source_requires_approval(source, user)
+
+    admins = User.with_role('admin')
+    assert admins.any?
+    assert_equal admins.map(&:email).sort, email.to.sort
+  end
+
+  test 'source in space approval requests go to space administrators' do
+    source = sources(:unapproved_source)
+    space = spaces(:plants)
+    source.space = space
+    source.save!
+    user = source.user
+    assert source.space
+    email = CurationMailer.source_requires_approval(source, user)
+
+    space_admins = space.administrators
+    assert_equal 1, space_admins.length
+    assert_equal 1, email.to.length
+    assert_equal 'plantboss@example.com', email.to.first
+  end
 end