From dab8f777b310acf3c6cc90bf78ecac14db5b24af Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Tue, 13 Feb 2024 14:31:45 +0100 Subject: [PATCH] Extract Contributor concern as abstract class During documentation of the pair programming feature, we noticed that the Contributor should be an abstract class, which is parent for the User and the ProgrammingGroup. With this commit, we perform these changes. --- app/models/concerns/contributor.rb | 9 ---- app/models/concerns/contributor_creation.rb | 1 - app/models/contributor.rb | 48 +++++++++++++++++++++ app/models/programming_group.rb | 36 ++-------------- app/models/user.rb | 33 +------------- db/seeds.rb | 2 +- 6 files changed, 53 insertions(+), 76 deletions(-) delete mode 100644 app/models/concerns/contributor.rb create mode 100644 app/models/contributor.rb diff --git a/app/models/concerns/contributor.rb b/app/models/concerns/contributor.rb deleted file mode 100644 index 624818ea..00000000 --- a/app/models/concerns/contributor.rb +++ /dev/null @@ -1,9 +0,0 @@ -# frozen_string_literal: true - -module Contributor - extend ActiveSupport::Concern - - included do - has_many :submissions, as: :contributor - end -end diff --git a/app/models/concerns/contributor_creation.rb b/app/models/concerns/contributor_creation.rb index 015d2fa6..d1920400 100644 --- a/app/models/concerns/contributor_creation.rb +++ b/app/models/concerns/contributor_creation.rb @@ -2,7 +2,6 @@ module ContributorCreation extend ActiveSupport::Concern - include Contributor ALLOWED_CONTRIBUTOR_TYPES = [InternalUser, ExternalUser, ProgrammingGroup].map(&:to_s).freeze diff --git a/app/models/contributor.rb b/app/models/contributor.rb new file mode 100644 index 00000000..8c104135 --- /dev/null +++ b/app/models/contributor.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +class Contributor < ApplicationRecord + self.abstract_class = true + + has_many :anomaly_notifications, as: :contributor, dependent: :destroy + has_many :user_exercise_interventions, as: :contributor + has_many :runners, as: :contributor, dependent: :destroy + + has_many :submissions, as: :contributor + + def learner? + raise NotImplementedError + end + + def teacher? + raise NotImplementedError + end + + def admin? + raise NotImplementedError + end + + def internal_user? + is_a?(InternalUser) + end + + def external_user? + is_a?(ExternalUser) + end + + def programming_group? + is_a?(ProgrammingGroup) + end + + def to_s + displayname + end + + def to_page_context + { + id:, + type: self.class.name, + consumer: try(:consumer)&.name, # Only a user is associated with a consumer. + displayname:, + } + end +end diff --git a/app/models/programming_group.rb b/app/models/programming_group.rb index f2584474..58ae2171 100644 --- a/app/models/programming_group.rb +++ b/app/models/programming_group.rb @@ -1,19 +1,14 @@ # frozen_string_literal: true -class ProgrammingGroup < ApplicationRecord - include Contributor - - has_many :anomaly_notifications, as: :contributor, dependent: :destroy +class ProgrammingGroup < Contributor has_many :programming_group_memberships, dependent: :destroy has_many :external_users, through: :programming_group_memberships, source_type: 'ExternalUser', source: :user 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, dependent: :destroy + has_many :testruns, through: :submissions # Only a single user starts testruns, but the group has access to them through their submissions. + has_many :events, dependent: :destroy # Only a single user creates events, but the group might be attributed to them optionally. 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 - has_many :user_exercise_interventions, as: :contributor belongs_to :exercise validate :min_group_size @@ -21,14 +16,6 @@ class ProgrammingGroup < ApplicationRecord validate :no_erroneous_users accepts_nested_attributes_for :programming_group_memberships - def external_user? - false - end - - def internal_user? - false - end - def learner? true end @@ -45,10 +32,6 @@ class ProgrammingGroup < ApplicationRecord Exercise end - def programming_group? - true - end - def add(user) # Accessing the `users` method here will preload all users, which is otherwise done during validation. internal_users << user if user.internal_user? && users.exclude?(user) @@ -56,23 +39,10 @@ class ProgrammingGroup < ApplicationRecord user end - def to_s - displayname - end - def displayname "Programming Group #{id}" end - def to_page_context - { - id:, - type: self.class.name, - consumer: nil, # A programming group is not associated with a consumer. - displayname:, - } - end - def programming_partner_ids users.map(&:id_with_type) end diff --git a/app/models/user.rb b/app/models/user.rb index 35d27851..77151e21 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1,12 +1,11 @@ # frozen_string_literal: true -class User < ApplicationRecord +class User < Contributor self.abstract_class = true attr_reader :current_study_group_id belongs_to :consumer - has_many :anomaly_notifications, as: :contributor, dependent: :destroy has_many :authentication_token, dependent: :destroy has_many :comments, as: :user has_many :study_group_memberships, as: :user @@ -15,15 +14,12 @@ class User < ApplicationRecord has_many :programming_groups, through: :programming_group_memberships, as: :user has_many :exercises, as: :user has_many :file_types, as: :user - has_many :submissions, as: :contributor has_many :participations, through: :submissions, source: :exercise, as: :user has_many :user_proxy_exercise_exercises, as: :user - has_many :user_exercise_interventions, as: :contributor has_many :testruns, as: :user has_many :interventions, through: :user_exercise_interventions has_many :remote_evaluation_mappings, as: :user has_many :request_for_comments, as: :user - has_many :runners, as: :contributor has_many :events has_many :events_synchronized_editor, class_name: 'Event::SynchronizedEditor' has_many :pair_programming_exercise_feedbacks @@ -31,8 +27,6 @@ class User < ApplicationRecord has_one :codeharbor_link, dependent: :destroy accepts_nested_attributes_for :user_proxy_exercise_exercises - scope :with_submissions, -> { where('id IN (SELECT user_id FROM submissions)') } - scope :in_study_group_of, lambda {|user| unless user.admin? joins(:study_group_memberships) @@ -46,18 +40,6 @@ class User < ApplicationRecord validates :platform_admin, inclusion: [true, false] - def internal_user? - is_a?(InternalUser) - end - - def external_user? - is_a?(ExternalUser) - end - - def programming_group? - false - end - def learner? return true if current_study_group_id.nil? @@ -86,19 +68,6 @@ class User < ApplicationRecord study_group_memberships.where(study_group: current_study_group_id).limit(1) end - def to_s - displayname - end - - def to_page_context - { - id:, - type: self.class.name, - consumer: consumer.name, - displayname:, - } - end - def self.find_by_id_with_type(id_with_type) if id_with_type[0].casecmp('e').zero? ExternalUser.find(id_with_type[1..]) diff --git a/db/seeds.rb b/db/seeds.rb index 758fc503..11dec6f8 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -28,7 +28,7 @@ end # delete all present records Rails.application.eager_load! -(ApplicationRecord.descendants - [ActiveRecord::SchemaMigration, User]).each(&:delete_all) +(ApplicationRecord.descendants - [ActiveRecord::SchemaMigration, Contributor, User]).each(&:delete_all) # delete file uploads FileUtils.rm_rf(Rails.public_path.join('uploads'))