diff --git a/app/controllers/study_groups_controller.rb b/app/controllers/study_groups_controller.rb index 2cabd7de..a4190d47 100644 --- a/app/controllers/study_groups_controller.rb +++ b/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 diff --git a/app/models/study_group.rb b/app/models/study_group.rb index 7688c89d..552b3006 100644 --- a/app/models/study_group.rb +++ b/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 diff --git a/app/models/user.rb b/app/models/user.rb index 874854a7..f6dbc976 100644 --- a/app/models/user.rb +++ b/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] diff --git a/app/policies/internal_user_policy.rb b/app/policies/internal_user_policy.rb index e5bdc6c7..2de787e5 100644 --- a/app/policies/internal_user_policy.rb +++ b/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 diff --git a/app/views/external_users/index.html.slim b/app/views/external_users/index.html.slim index 25a0395b..24279460 100644 --- a/app/views/external_users/index.html.slim +++ b/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) diff --git a/app/views/external_users/show.html.slim b/app/views/external_users/show.html.slim index 1bcb87c1..f44e9ca5 100644 --- a/app/views/external_users/show.html.slim +++ b/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? diff --git a/app/views/internal_users/index.html.slim b/app/views/internal_users/index.html.slim index 425c99eb..92dd6c37 100644 --- a/app/views/internal_users/index.html.slim +++ b/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,14 +15,16 @@ h1 = InternalUser.model_name.human(count: 2) tr th = t('activerecord.attributes.internal_user.name') th = t('activerecord.attributes.internal_user.consumer') - th = t('activerecord.attributes.internal_user.platform_admin') + - if current_user.admin? + th = t('activerecord.attributes.internal_user.platform_admin') th colspan=3 = t('shared.actions') tbody - @users.each do |user| 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 - td = symbol_for(user.platform_admin?) + - 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? td = link_to(t('shared.destroy'), user, data: {confirm: t('shared.confirm_destroy')}, method: :delete) if policy(user).destroy? diff --git a/app/views/internal_users/show.html.slim b/app/views/internal_users/show.html.slim index 334e7175..b260cb62 100644 --- a/app/views/internal_users/show.html.slim +++ b/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')) diff --git a/app/views/study_groups/_form.html.slim b/app/views/study_groups/_form.html.slim index b0e705fc..3e93dcaa 100644 --- a/app/views/study_groups/_form.html.slim +++ b/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' diff --git a/app/views/study_groups/show.html.slim b/app/views/study_groups/show.html.slim index 073cd0fe..84d6d1fa 100644 --- a/app/views/study_groups/show.html.slim +++ b/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) diff --git a/spec/policies/internal_user_policy_spec.rb b/spec/policies/internal_user_policy_spec.rb index 1b2378a3..32db3791 100644 --- a/spec/policies/internal_user_policy_spec.rb +++ b/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