Allow teachers to access internal users and manage them in their study groups

app/controllers/study_groups_controller.rb

@@ -11,13 +11,10 @@ class StudyGroupsController < ApplicationController
     authorize!
   end
 
-  def show
-    @search = @study_group.external_users.ransack(params[:q])
-  end
+  def show; end
 
   def edit
-    @search = @study_group.external_users.ransack(params[:q])
-    @members = StudyGroupMembership.where(user: @search.result, study_group: @study_group)
+    @members = StudyGroupMembership.where(user: @study_group.users, study_group: @study_group).includes(:user)
   end
 
   def update

app/models/study_group.rb

@@ -15,7 +15,7 @@ class StudyGroup < ApplicationRecord
   end
 
   def user_count
-    external_users.count + internal_users.count
+    study_group_memberships.distinct.count
   end
 
   def to_s

app/models/user.rb

@@ -23,7 +23,14 @@ class User < ApplicationRecord
   scope :with_submissions, -> { where('id IN (SELECT user_id FROM submissions)') }
 
   scope :in_study_group_of, lambda {|user|
-                              joins(:study_group_memberships).where(study_group_memberships: {study_group_id: user.study_groups}) unless user.admin?
+                              unless user.admin?
+                                joins(:study_group_memberships)
+                                  .where(study_group_memberships: {
+                                    study_group_id: user.study_group_memberships
+                                                        .where(study_group_memberships: {role: StudyGroupMembership.roles[:teacher]})
+                                                        .select(:study_group_id),
+                                  })
+                              end
                             }
 
   validates :platform_admin, boolean_presence: true
@@ -65,7 +72,7 @@ class User < ApplicationRecord
   end
 
   def self.ransackable_attributes(auth_object)
-    if auth_object.admin?
+    if auth_object.present? && auth_object.admin?
       %w[name email external_id consumer_id platform_admin]
     else
       %w[name external_id]

app/policies/internal_user_policy.rb

@@ -2,10 +2,14 @@
 
 class InternalUserPolicy < AdminOnlyPolicy
   def destroy?
-    super && !@record.admin?
+    admin? && !@record.admin?
+  end
+
+  def index?
+    admin? || teacher?
   end
 
   def show?
-    super || @record == @user
+    admin? || @record == @user || teacher_in_study_group?
   end
 end

app/views/external_users/index.html.slim

@@ -32,12 +32,15 @@ h1 = ExternalUser.model_name.human(count: 2)
       tr
         th = t('activerecord.attributes.external_user.name')
         th = t('activerecord.attributes.external_user.consumer')
+        th = t('activerecord.attributes.external_user.platform_admin')
         th = t('shared.actions')
     tbody
       - @users.each do |user|
         tr
           td = link_to_if(policy(user).show?, user.displayname, user)
           td = link_to_if(user.consumer.present? && policy(user.consumer).show?, user.consumer, user.consumer)
+          - if current_user.admin?
+            td = symbol_for(user.platform_admin?)
           td = link_to(t('shared.show'), user) if policy(user).show?
 
 = render('shared/pagination', collection: @users)

app/views/external_users/show.html.slim

@@ -6,7 +6,7 @@ h1 = @user.displayname
   code
     = @user.external_id
 = row(label: 'external_user.consumer', value: link_to_if(policy(@user.consumer).show?, @user.consumer, @user.consumer))
-= row(label: 'external_user.platform_admin', value: @user.platform_admin?)
+= row(label: 'external_user.platform_admin', value: @user.platform_admin?) if current_user.admin?
 = row(label: 'users.show.study_groups') do
   - visible_memberships = @user.study_group_memberships.select { |study_group_membership| policy(study_group_membership.study_group).show? }
   - if visible_memberships.any?

app/views/internal_users/index.html.slim

@@ -1,6 +1,6 @@
 h1 = InternalUser.model_name.human(count: 2)
 
-= render(layout: 'shared/form_filters') do |f|
+= render(layout: 'shared/form_filters') if current_user.admin? do |f|
   .col-auto
     = f.label(:consumer_id_eq, t('activerecord.attributes.internal_user.consumer'), class: 'visually-hidden form-label')
     = f.collection_select(:consumer_id_eq, Consumer.with_internal_users, :id, :name, class: 'form-control', include_blank: true, prompt: t('activerecord.attributes.internal_user.consumer'))
@@ -15,6 +15,7 @@ h1 = InternalUser.model_name.human(count: 2)
       tr
         th = t('activerecord.attributes.internal_user.name')
         th = t('activerecord.attributes.internal_user.consumer')
+        - if current_user.admin?
           th = t('activerecord.attributes.internal_user.platform_admin')
         th colspan=3 = t('shared.actions')
     tbody
@@ -22,6 +23,7 @@ h1 = InternalUser.model_name.human(count: 2)
         tr
           td = link_to_if(policy(user).show?, user.name, user)
           td = user.consumer ? link_to_if(policy(user.consumer).show?, user.consumer, user.consumer) : empty
+          - if current_user.admin?
             td = symbol_for(user.platform_admin?)
           td = link_to(t('shared.show'), user) if policy(user).show?
           td = link_to(t('shared.edit'), edit_internal_user_path(user)) if policy(user).edit?

app/views/internal_users/show.html.slim

@@ -2,10 +2,10 @@ h1
   = @user
   = render('shared/edit_button', object: @user)
 
-= row(label: 'internal_user.email', value: @user.email)
+= row(label: 'internal_user.email', value: @user.email) if current_user.admin?
 = row(label: 'internal_user.name', value: @user.name)
 = row(label: 'internal_user.consumer', value: @user.consumer ? link_to_if(policy(@user.consumer).show?, @user.consumer, @user.consumer) : nil)
-= row(label: 'internal_user.platform_admin', value: @user.platform_admin?)
+= row(label: 'internal_user.platform_admin', value: @user.platform_admin?) if current_user.admin?
 = row(label: 'internal_user.activated', value: @user.activated?)
 = row(label: 'users.show.study_groups') do
   - visible_memberships = @user.study_group_memberships.select { |study_group_membership| policy(study_group_membership.study_group).show? }
@@ -28,4 +28,5 @@ h1
   - else
     = t('users.show.no_groups')
 
-= row(label: 'codeharbor_link.profile_label', value: @user.codeharbor_link.nil? ? link_to(t('codeharbor_link.new'), new_codeharbor_link_path, class: 'btn btn-secondary') : link_to(t('codeharbor_link.edit'), edit_codeharbor_link_path(@user.codeharbor_link), class: 'btn btn-secondary'))
+- if @user == current_user
+  = row(label: 'codeharbor_link.profile_label', value: @user.codeharbor_link.nil? ? link_to(t('codeharbor_link.new'), new_codeharbor_link_path, class: 'btn btn-secondary') : link_to(t('codeharbor_link.edit'), edit_codeharbor_link_path(@user.codeharbor_link), class: 'btn btn-secondary'))

app/views/study_groups/_form.html.slim

@@ -6,11 +6,11 @@
 
   h3 = t('activerecord.attributes.study_group.members')
   .table-responsive
-    table.table
+    table.sortable.table
       thead
         tr
           th = t('activerecord.attributes.exercise.selection')
-          th = sort_link(@search, :name, t('navigation.sections.users'))
+          th = t('navigation.sections.users')
       = collection_check_boxes :study_group, :study_group_membership_ids, @members, :id, :id do |b|
         tr
           td = b.check_box class: 'form-check-input'

app/views/study_groups/show.html.slim

@@ -12,10 +12,10 @@ h1
 
 h2.mt-4 = t('activerecord.attributes.study_group.members')
 .table-responsive
-  table.table
+  table.sortable.table
     thead
       tr
-        th = sort_link(@search, :name, t('navigation.sections.users'))
+        th =  t('navigation.sections.users')
     - @study_group.users.each do |user|
       tr
         td = link_to_if(policy(user).show?, user.displayname, user)

spec/policies/internal_user_policy_spec.rb

@@ -5,7 +5,7 @@ require 'rails_helper'
 describe InternalUserPolicy do
   subject(:policy) { described_class }
 
-  %i[create? edit? index? new? show? update?].each do |action|
+  %i[create? edit? new? show? update?].each do |action|
     permissions(action) do
       it 'grants access to admins only' do
         expect(policy).to permit(build(:admin), InternalUser.new)
@@ -16,6 +16,15 @@ describe InternalUserPolicy do
     end
   end
 
+  permissions :index? do
+    it 'grants access to admins and teachers only' do
+      expect(policy).not_to permit(build(:external_user), InternalUser.new)
+      %i[admin teacher].each do |factory_name|
+        expect(policy).to permit(create(factory_name), InternalUser.new)
+      end
+    end
+  end
+
   permissions :destroy? do
     context 'with an admin user' do
       it 'grants access to no one' do