Add CRUD operations for Programming Groups

* Correct sorting in table
* Modify page when nested in exercises
* Fix links between pages
* Link from statistics page to programming_groups/index
* Link from submission page to programming_groups/<id>
* Allow filtering for exercise ID on ProgrammingGroup#index
* Add search fields for internal and external user id on pg/index

app/controllers/programming_groups_controller.rb

@@ -4,7 +4,17 @@ class ProgrammingGroupsController < ApplicationController
   include CommonBehavior
   include LtiHelper
 
-  before_action :set_exercise_and_authorize
+  before_action :set_exercise_and_authorize, only: %i[new create]
+  before_action :set_programming_group_and_authorize, only: MEMBER_ACTIONS
+
+  def index
+    set_exercise_and_authorize if params[:exercise_id].present?
+    @search = ProgrammingGroup.ransack(params[:q], {auth_object: current_user})
+    @programming_groups = @search.result.includes(:exercise, :programming_group_memberships, :internal_users, :external_users).order(:id).paginate(page: params[:page], per_page: per_page_param)
+    authorize!
+  end
+
+  def show; end
 
   def new
     Event.create(category: 'page_visit', user: current_user, exercise: @exercise, data: 'programming_groups_new', file_id: nil)
@@ -23,9 +33,13 @@ class ProgrammingGroupsController < ApplicationController
     end
   end
 
+  def edit
+    @members = @programming_group.programming_group_memberships.includes(:user)
+  end
+
   def create
-    programming_partner_ids = programming_group_params[:programming_partner_ids].split(',').map(&:strip).uniq
-    users = programming_partner_ids.map do |partner_id|
+    programming_partner_ids = programming_group_params&.fetch(:programming_partner_ids, [])&.split(',')&.map(&:strip)&.uniq
+    users = programming_partner_ids&.map do |partner_id|
       User.find_by_id_with_type(partner_id)
     rescue ActiveRecord::RecordNotFound
       partner_id
@@ -33,12 +47,12 @@ class ProgrammingGroupsController < ApplicationController
     @programming_group = ProgrammingGroup.new(exercise: @exercise, users:)
     authorize!
 
-    unless programming_partner_ids.include? current_user.id_with_type
+    unless programming_partner_ids&.include? current_user.id_with_type
       @programming_group.add(current_user)
     end
 
     unless @programming_group.valid?
-      Event.create(category: 'pp_invalid_partners', user: current_user, exercise: @exercise, data: programming_group_params[:programming_partner_ids], file_id: nil)
+      Event.create(category: 'pp_invalid_partners', user: current_user, exercise: @exercise, data: programming_group_params&.fetch(:programming_partner_ids), file_id: nil)
     end
 
     create_and_respond(object: @programming_group, path: proc { implement_exercise_path(@exercise) }) do
@@ -65,14 +79,26 @@ class ProgrammingGroupsController < ApplicationController
     end
   end
 
+  def update
+    myparams = programming_group_params || {}
+    @members = @programming_group.programming_group_memberships.includes(:user)
+    myparams[:users] = @members.where(id: myparams&.fetch(:programming_group_membership_ids, [])&.compact_blank).map(&:user)
+    update_and_respond(object: @programming_group, params: myparams)
+  end
+
+  def destroy
+    session.delete(:pg_id) if current_contributor == @programming_group
+    destroy_and_respond(object: @programming_group)
+  end
+
   private
 
   def authorize!
-    authorize(@programming_group)
+    authorize(@programming_group || @programming_groups)
   end
 
   def programming_group_params
-    params.require(:programming_group).permit(:programming_partner_ids)
+    params.require(:programming_group).permit(:programming_partner_ids, programming_group_membership_ids: []) if params[:programming_group].present?
   end
 
   def set_exercise_and_authorize
@@ -80,6 +106,11 @@ class ProgrammingGroupsController < ApplicationController
     authorize(@exercise, :implement?)
   end
 
+  def set_programming_group_and_authorize
+    @programming_group = ProgrammingGroup.find(params[:id])
+    authorize!
+  end
+
   def redirect_to_exercise
     skip_authorization
     redirect_to(implement_exercise_path(@exercise),

app/helpers/statistics_helper.rb

@@ -44,6 +44,7 @@ module StatisticsHelper
         key: 'programming_groups',
         name: t('activerecord.models.programming_group.other'),
         data: ProgrammingGroup.count,
+        url: programming_groups_path,
       },
       {
         key: 'currently_active',

app/models/exercise.rb

@@ -36,6 +36,7 @@ class Exercise < ApplicationRecord
   has_many :request_for_comments
 
   scope :with_submissions, -> { where('id IN (SELECT exercise_id FROM submissions)') }
+  scope :with_programming_groups, -> { where('id IN (SELECT exercise_id FROM programming_groups)') }
 
   validate :valid_main_file?
   validate :valid_submission_deadlines?
@@ -612,7 +613,7 @@ class Exercise < ApplicationRecord
   end
 
   def self.ransackable_attributes(_auth_object = nil)
-    %w[title internal_title]
+    %w[title id internal_title]
   end
 
   def self.ransackable_associations(_auth_object = nil)

app/models/programming_group.rb

@@ -9,10 +9,10 @@ class ProgrammingGroup < ApplicationRecord
   has_many :internal_users, through: :programming_group_memberships, source_type: 'InternalUser', source: :user
   has_many :testruns, through: :submissions
   has_many :runners, as: :contributor, dependent: :destroy
-  has_many :events
-  has_many :events_synchronized_editor, class_name: 'Event::SynchronizedEditor'
-  has_many :pair_programming_exercise_feedbacks
-  has_many :pair_programming_waiting_users
+  has_many :events, dependent: :destroy
+  has_many :events_synchronized_editor, class_name: 'Event::SynchronizedEditor', dependent: :destroy
+  has_many :pair_programming_exercise_feedbacks, dependent: :destroy
+  has_many :pair_programming_waiting_users, dependent: :destroy
   belongs_to :exercise
 
   validate :min_group_size
@@ -20,11 +20,6 @@ class ProgrammingGroup < ApplicationRecord
   validate :no_erroneous_users
   accepts_nested_attributes_for :programming_group_memberships
 
-  def initialize(attributes = nil)
-    @erroneous_users = []
-    super
-  end
-
   def external_user?
     false
   end
@@ -76,15 +71,27 @@ class ProgrammingGroup < ApplicationRecord
   def users=(users)
     self.internal_users = []
     self.external_users = []
-    users.each do |user|
-      next @erroneous_users << user unless user.is_a?(User)
+    users&.each do |user|
+      next erroneous_users << user unless user.is_a?(User)
 
       add(user)
     end
   end
 
+  def self.ransackable_associations(_auth_object = nil)
+    %w[exercise programming_group_memberships]
+  end
+
+  def self.ransortable_attributes(_auth_object = nil)
+    %w[id created_at]
+  end
+
   private
 
+  def erroneous_users
+    @erroneous_users ||= []
+  end
+
   def min_group_size
     if users.size < 2
       errors.add(:base, :size_too_small)
@@ -98,7 +105,7 @@ class ProgrammingGroup < ApplicationRecord
   end
 
   def no_erroneous_users
-    @erroneous_users.each do |partner_id|
+    erroneous_users.each do |partner_id|
       errors.add(:base, :invalid_partner_id, partner_id:)
     end
   end

app/models/programming_group_membership.rb

@@ -12,4 +12,8 @@ class ProgrammingGroupMembership < ApplicationRecord
       errors.add(:base, :already_exists, id_with_type: user.id_with_type)
     end
   end
+
+  def self.ransackable_associations(_auth_object = nil)
+    %w[user]
+  end
 end

app/models/user.rb

@@ -111,9 +111,9 @@ class User < ApplicationRecord
 
   def self.ransackable_attributes(auth_object)
     if auth_object.present? && auth_object.admin?
-      %w[name email external_id consumer_id platform_admin]
+      %w[name email external_id consumer_id platform_admin id]
     else
-      %w[name external_id]
+      %w[name external_id id]
     end
   end
 end

app/policies/exercise_policy.rb

@@ -37,6 +37,10 @@ class ExercisePolicy < AdminOrAuthorPolicy
     end
   end
 
+  def programming_groups_for_exercise?
+    admin?
+  end
+
   def submit?
     everyone && @record.teacher_defined_assessment?
   end

app/policies/programming_group_policy.rb

@@ -1,11 +1,11 @@
 # frozen_string_literal: true
 
-class ProgrammingGroupPolicy < ApplicationPolicy
-  def new?
+class ProgrammingGroupPolicy < AdminOnlyPolicy
+  def create?
     everyone
   end
 
-  def create?
+  def new?
     everyone
   end
 

app/views/application/_navigation.html.slim

@@ -12,7 +12,7 @@
           li.dropdown-divider role='separator'
         = render('navigation_submenu', title: t('activerecord.models.exercise.other'),
                 models: [Exercise, ExerciseCollection, ProxyExercise, Tag, Tip, Submission], link: exercises_path, cached: true)
-        = render('navigation_submenu', title: t('navigation.sections.users'), models: [InternalUser, ExternalUser],
+        = render('navigation_submenu', title: t('navigation.sections.contributors'), models: [InternalUser, ExternalUser, ProgrammingGroup],
                 cached: true)
         = render('navigation_collection_link', model: StudyGroup, cached: true)
         = render('navigation_collection_link', model: ExecutionEnvironment, cached: true)

app/views/exercises/index.html.slim

@@ -50,6 +50,7 @@ h1 = Exercise.model_name.human(count: 2)
                 li = link_to(t('shared.show'), exercise, 'data-turbolinks' => "false", class: 'dropdown-item') if policy(exercise).show?
                 li = link_to(t('activerecord.models.user_exercise_feedback.other'), feedback_exercise_path(exercise), class: 'dropdown-item') if policy(exercise).feedback?
                 li = link_to(t('activerecord.models.request_for_comment.other'), rfcs_for_exercise_path(exercise), class: 'dropdown-item') if policy(exercise).rfcs_for_exercise?
+                li = link_to(t('activerecord.models.programming_group.other'), exercise_programming_groups_path(exercise), class: 'dropdown-item') if policy(exercise).programming_groups_for_exercise?
                 li = link_to(t('shared.destroy'), exercise, data: {confirm: t('shared.confirm_destroy')}, method: :delete, class: 'dropdown-item') if policy(exercise).destroy?
                 li = link_to(t('.clone'), clone_exercise_path(exercise), data: {confirm: t('shared.confirm_destroy')}, method: :post, class: 'dropdown-item') if policy(exercise).clone?
                 li = link_to(t('exercises.export_codeharbor.label'), '', class: 'dropdown-item export-start', data: {'exercise-id' => exercise.id}) if policy(exercise).export_external_confirm?

app/views/exercises/show.html.slim

@@ -19,6 +19,7 @@ h1.d-inline-block
     li = link_to(t('shared.statistics'), statistics_exercise_path(@exercise), 'data-turbolinks' => "false", class: 'dropdown-item') if policy(@exercise).statistics?
     li = link_to(t('activerecord.models.user_exercise_feedback.other'), feedback_exercise_path(@exercise), class: 'dropdown-item') if policy(@exercise).feedback?
     li = link_to(t('activerecord.models.request_for_comment.other'), rfcs_for_exercise_path(@exercise), class: 'dropdown-item') if policy(@exercise).rfcs_for_exercise?
+    li = link_to(t('activerecord.models.programming_group.other'), exercise_programming_groups_path(@exercise), class: 'dropdown-item') if policy(@exercise).programming_groups_for_exercise?
     li = link_to(t('shared.destroy'), @exercise, data: {confirm: t('shared.confirm_destroy')}, method: :delete, class: 'dropdown-item') if policy(@exercise).destroy?
     li = link_to(t('exercises.index.clone'), clone_exercise_path(@exercise), data: {confirm: t('shared.confirm_destroy')}, method: :post, class: 'dropdown-item') if policy(@exercise).clone?
     li = link_to(t('exercises.export_codeharbor.label'), '', class: 'dropdown-item export-start', data: {'exercise-id' => @exercise.id}) if policy(@exercise).export_external_confirm?

app/views/programming_groups/_form_edit.html.slim

@@ -0,0 +1,16 @@
+= form_for(@programming_group) do |f|
+  = render('shared/form_errors', object: @programming_group)
+
+  h3 = t('activerecord.attributes.programming_group.member')
+  .table-responsive
+    table.table class="#{@members.present? ? 'sortable' : ''}"
+      thead
+        tr
+          th = t('activerecord.attributes.exercise.selection')
+          th = t('navigation.sections.contributors')
+      = collection_check_boxes :programming_group, :programming_group_membership_ids, @members, :id, :id do |b|
+        tr
+          td = b.check_box class: 'form-check-input'
+          td = link_to_if(policy(b.object.user).show?, b.object.user.displayname, b.object.user)
+
+  .actions = render('shared/submit_button', f: f, object: @programming_group)

app/views/programming_groups/edit.html.slim

@@ -0,0 +1,3 @@
+h1 = @programming_group
+
+= render('form_edit')

app/views/programming_groups/index.html.slim

@@ -0,0 +1,43 @@
+- if params[:exercise_id].nil?
+  h1 = ProgrammingGroup.model_name.human(count: 2)
+  = render(layout: 'shared/form_filters') do |f|
+    .col-auto
+      = f.label(:exercise_id_eq, t('activerecord.attributes.programming_group.exercise'), class: 'visually-hidden form-label')
+      = f.collection_select(:exercise_id_eq, Exercise.with_programming_groups, :id, :title, class: 'form-control', prompt: t('activerecord.attributes.programming_group.exercise'))
+    .col-auto
+      = f.label(:programming_group_memberships_user_of_ExternalUser_type_id_eq, t('activerecord.attributes.programming_group.external_user_id'), class: 'visually-hidden form-label')
+      = f.search_field(:programming_group_memberships_user_of_ExternalUser_type_id_eq, class: 'form-control', placeholder: t('activerecord.attributes.programming_group.external_user_id'))
+    .col-auto
+      = f.label(:programming_group_memberships_user_of_InternalUser_type_id_eq, t('activerecord.attributes.programming_group.internal_user_id'), class: 'visually-hidden form-label')
+      = f.search_field(:programming_group_memberships_user_of_InternalUser_type_id_eq, class: 'form-control', placeholder: t('activerecord.attributes.programming_group.internal_user_id'))
+- else
+  h1 = "#{ProgrammingGroup.model_name.human(count: 2)} for Exercise '#{@exercise.title}'"
+
+.table-responsive
+  table.table.mt-4 class="#{@programming_groups.present? ? 'sortable' : ''}"
+    thead
+      tr
+        th.sortable_nosort = sort_link(@search, :id, t('activerecord.attributes.programming_group.name'))
+        - if params[:exercise_id].blank?
+          th.sorttable_nosort = sort_link(@search, :exercise_id, t('activerecord.attributes.programming_group.exercise'))
+        th = t('activerecord.attributes.programming_group.member')
+        th = t('activerecord.attributes.programming_group.member_count')
+        th.sorttable_nosort = sort_link(@search, :created_at, t('shared.created_at'))
+        th colspan=3 = t('shared.actions')
+    tbody
+      - if params[:exercise_id].nil?
+        - filtered_programming_groups = @programming_groups
+      - else
+        - filtered_programming_groups = @programming_groups.where(exercise_id: params[:exercise_id])
+      - filtered_programming_groups.each do |programming_group|
+        tr
+          td = link_to_if(policy(programming_group).show?, programming_group.displayname, programming_group)
+          - if params[:exercise_id].blank?
+            td = link_to_if(policy(programming_group.exercise).show?, programming_group.exercise.title, programming_group.exercise, 'data-turbolinks' => "false")
+          td == programming_group.users.map { |user| link_to_if(policy(user).show?, user.name, user) }.join(', ')
+          td = programming_group.users.size
+          td = l(programming_group.created_at, format: :short)
+          td = link_to(t('shared.show'), programming_group) if policy(programming_group).show?
+          td = link_to(t('shared.edit'), edit_programming_group_path(programming_group)) if policy(programming_group).edit?
+          td = link_to(t('shared.destroy'), programming_group, data: { confirm: t('shared.confirm_destroy') }, method: :delete) if policy(programming_group).destroy?
+= render('shared/pagination', collection: @programming_groups)

app/views/programming_groups/show.html.slim

@@ -0,0 +1,19 @@
+h1
+  = @programming_group
+  - if policy(@programming_group).edit?
+    = render('shared/edit_button', object: @programming_group)
+
+= row(label: 'activerecord.attributes.programming_group.name', value: @programming_group.displayname)
+= row(label: 'activerecord.attributes.programming_group.exercise', value: link_to_if(policy(@programming_group.exercise).show?, @programming_group.exercise.title, @programming_group.exercise))
+= row(label: 'activerecord.attributes.programming_group.member_count', value: @programming_group.users.size)
+= row(label: 'shared.created_at', value: l(@programming_group.created_at, format: :short))
+
+h2.mt-4 = t('activerecord.attributes.study_group.members')
+.table-responsive
+  table.table class="#{@programming_group.users.present? ? 'sortable' : ''}"
+    thead
+      tr
+        th =  t('navigation.sections.contributors')
+    - @programming_group.users.each do |user|
+      tr
+        td = link_to_if(policy(user).show?, user.displayname, user)

app/views/submissions/index.html.slim

@@ -22,7 +22,7 @@ h1 = Submission.model_name.human(count: 2)
       - @submissions.each do |submission|
         tr
           td = link_to_if(submission.exercise && policy(submission.exercise).show?, submission.exercise, submission.exercise)
-          td = link_to_if(submission.contributor.is_a?(User) && policy(submission.contributor).show?, submission.contributor, submission.contributor)
+          td = link_to_if(policy(submission.contributor).show?, submission.contributor, submission.contributor)
           td = t("submissions.causes.#{submission.cause}")
           td = submission.score
           td = l(submission.created_at, format: :short)

app/views/submissions/show.html.slim

@@ -8,7 +8,7 @@
 h1 = @submission
 
 = row(label: 'submission.exercise', value: link_to_if(policy(@submission.exercise).show?, @submission.exercise, @submission.exercise))
-= row(label: 'submission.contributor', value: link_to_if(@submission.contributor.is_a?(User) && policy(@submission.contributor).show?, @submission.contributor, @submission.contributor))
+= row(label: 'submission.contributor', value: link_to_if(policy(@submission.contributor).show?, @submission.contributor, @submission.contributor))
 = row(label: 'submission.study_group', value: link_to_if(@submission.study_group.present? && policy(@submission.study_group).show?, @submission.study_group, @submission.study_group))
 = row(label: 'submission.cause', value: t("submissions.causes.#{@submission.cause}"))
 = row(label: 'submission.score', value: @submission.score)