TaskContributions

app/controllers/concerns/task_parameters.rb

@@ -0,0 +1,70 @@
+# frozen_string_literal: true
+
+module TaskParameters
+  private
+
+  def file_params
+    %i[id content attachment path name internal_description mime_type use_attached_file used_by_grader visible usage_by_lms xml_id _destroy
+       parent_id parent_blob_id]
+  end
+
+  def test_params
+    [:id, :title, :testing_framework_id, :description, :internal_description, :test_type, :xml_id, :validity, :timeout, :_destroy,
+     :parent_id, {files_attributes: file_params}]
+  end
+
+  def model_solution_params
+    [:id, :description, :internal_description, :xml_id, :_destroy, :parent_id, {files_attributes: file_params}]
+  end
+
+  def task_params
+    params.require(:task).permit(:title, :description, :internal_description, :parent_uuid, :language, :license_id,
+      :programming_language_id, :access_level, files_attributes: file_params, tests_attributes: test_params,
+      model_solutions_attributes: model_solution_params, label_names: []).tap {|parameters| fix_nested_files_params(parameters) }
+  end
+
+  def group_tasks_params
+    params.permit(group_tasks: {group_ids: []})[:group_tasks]
+  end
+
+  def import_confirm_params
+    params.permit(:import_id, :subfile_id, :import_type)
+  end
+
+  def fix_nested_files_params(parameters)
+    # The task_params might contain incomplete attributes for files, which we must fix before using the parameters.
+    # Those file attributes can be attached directly to the task, to any of test or to any model solution.
+    # For each of the potential file attributes, we must check if the attachment attribute should present and re-add it if missing.
+    # This behavior is needed, since SimpleForm does not keep the (empty) attachment attribute if no file got attached by the user.
+    # Otherwise, an old file version would be linked to the task, test or model solution after a failed model validation.
+    # When this happens, the old file content is used alongside the name of the new file, which is confusing for the user.
+
+    # Fix file attributes directly linked to the task.
+    fix_attachment_params(parameters)
+
+    # Fix file attributes linked to tests. The tests are stored in a hash with the test number as the key.
+    parameters[:tests_attributes]&.each_value {|test_parameters| fix_attachment_params(test_parameters) }
+
+    # Fix file attributes linked to model solutions. The model solutions are stored in a hash with the model solution number as the key.
+    parameters[:model_solutions_attributes]&.each_value {|model_solution_parameters| fix_attachment_params(model_solution_parameters) }
+  end
+
+  def fix_attachment_params(parameters)
+    # Files are stored in a hash with the file number as the key.
+    parameters[:files_attributes]&.each_value do |file|
+      # Do not modify the given file attributes if an attachment key is already present.
+      # This is needed to keep the attachment if the user sent a file.
+      next if file.key?(:attachment)
+      # Do not modify the given file attributes if the parent_blob_id is present.
+      # This ensures that an attachment is kept even if the user did not send a file.
+      # For example, a file has been stored previously and the user only wants to update unrelated attributes.
+      # As soon as the user chose a file manually and validations fail, the parent_blob_id is left empty.
+      # Then, on a second try, the (updated) file is no longer linked to the previous file version.
+      next if file[:parent_blob_id].present?
+
+      # If none of the above conditions is met, the user did not send a file despite previously making changes.
+      # In this case, we must re-add the attachment attribute and set it to `nil` to indicate the missing file.
+      file[:attachment] = nil
+    end
+  end
+end

app/controllers/task_contributions_controller.rb

@@ -0,0 +1,112 @@
+# frozen_string_literal: true
+
+class TaskContributionsController < ApplicationController
+  include TaskParameters
+
+  before_action :load_and_authorize_base_task
+  before_action :load_and_authorize_task_contribution, except: %i[index create new]
+
+  def approve_changes
+    if @task.apply_contribution(@task_contribution)
+      TaskContributionMailer.send_approval_info(@task_contribution).deliver_later
+      redirect_to @task, notice: t('.success')
+    else
+      redirect_to [@task, @task_contribution], alert: t('.error')
+    end
+  end
+
+  def discard_changes
+    duplicate = @task_contribution.decouple
+    if duplicate
+      redirect_to duplicate, notice: t('.success')
+    else
+      redirect_to [@task, @task_contribution], alert: t('.error')
+    end
+  end
+
+  def reject_changes
+    duplicate = @task_contribution.decouple
+    if duplicate
+      TaskContributionMailer.send_rejection_info(@task_contribution, duplicate).deliver_later
+      redirect_to @task, notice: t('.success')
+    else
+      redirect_to [@task, @task_contribution], alert: t('.error')
+    end
+  end
+
+  def index
+    @task_contributions = @task.contributions.order(created_at: :desc).paginate(page: params[:page], per_page: per_page_param)
+    raise Pundit::NotAuthorizedError unless policy(@task_contributions).index? base: @task
+  end
+
+  def show
+    @task = @task_contribution.suggestion
+
+    @files = @task.files
+    @tests = @task.tests
+    @model_solutions = @task.model_solutions
+
+    @user_rating = @task.ratings&.find_by(user: current_user)&.rating
+    render 'tasks/show'
+  end
+
+  def new
+    raise Pundit::NotAuthorizedError if current_user.nil?
+
+    @task_contribution = TaskContribution.new_for(@task, current_user)
+    authorize @task_contribution
+
+    @task = @task_contribution.suggestion
+  end
+
+  # The function should render the edit form used by TaskController
+  def edit
+    @task = @task_contribution.suggestion
+    render 'tasks/edit'
+  end
+
+  def create # rubocop:disable Metrics/AbcSize
+    @task_contribution = TaskContribution.new(suggestion_attributes: task_params, base: @task)
+    @task_contribution.suggestion.assign_attributes(
+      user: current_user,
+      access_level: :private,
+      meta_data: @task.meta_data,
+      submission_restrictions: @task.submission_restrictions,
+      external_resources: @task.external_resources,
+      grading_hints: @task.grading_hints
+    )
+    authorize @task_contribution
+
+    if @task_contribution.save(context: :force_validations)
+      TaskContributionMailer.send_contribution_request(@task_contribution).deliver_later
+      redirect_to [@task, @task_contribution], notice: t('common.notices.object_created', model: TaskContribution.model_name.human)
+    else
+      @task = @task_contribution.suggestion
+      render 'tasks/new'
+    end
+  end
+
+  def update
+    @task_contribution.suggestion.assign_attributes(task_params.except(:parent_uuid))
+    if @task_contribution.save(context: :force_validations)
+      redirect_to [@task, @task_contribution], notice: t('common.notices.object_updated', model: TaskContribution.model_name.human)
+    else
+      @task = @task_contribution.suggestion
+      render 'tasks/edit', danger: t('common.errors.changes_not_saved', model: TaskContribution.model_name.human)
+    end
+  end
+
+  private
+
+  def load_and_authorize_base_task
+    @task = Task.find(params[:task_id])
+    authorize @task, :show?
+  end
+
+  def load_and_authorize_task_contribution
+    @task_contribution = TaskContribution.find(params[:id])
+    raise Pundit::NotAuthorizedError unless @task_contribution.base == @task
+
+    authorize @task_contribution
+  end
+end

app/controllers/tasks_controller.rb

@@ -3,12 +3,14 @@
 require 'zip'
 
 class TasksController < ApplicationController # rubocop:disable Metrics/ClassLength
+  include TaskParameters
   before_action :load_and_authorize_task, except: %i[index new create import_start import_confirm import_uuid_check import_external]
   before_action :load_and_authorize_account_link, only: %i[export_external_start export_external_check export_external_confirm]
   before_action :only_authorize_action, only: %i[import_start import_confirm import_uuid_check import_external]
 
   before_action :handle_search_params, only: :index
   before_action :set_search, only: [:index]
+  before_action :redirect_task_contribution, only: %i[create update edit show]
   prepend_before_action :set_user_for_api_request, only: %i[import_uuid_check import_external]
   skip_before_action :verify_authenticity_token, only: %i[import_uuid_check import_external]
   skip_before_action :require_user!, only: %i[show download]
@@ -237,6 +239,12 @@ def only_authorize_action
     authorize Task
   end
 
+  def redirect_task_contribution
+    return unless @task.present? && @task.contribution?
+
+    redirect_to action: action_name, controller: 'task_contributions', task_id: @task.parent, id: @task.task_contribution
+  end
+
   def set_search # rubocop:disable Metrics/AbcSize
     search = params[:q]
     @req_labels = []
@@ -268,33 +276,6 @@ def handle_search_params
     save_search_params
   end
 
-  def file_params
-    %i[id content attachment path name internal_description mime_type use_attached_file used_by_grader visible usage_by_lms xml_id _destroy]
-  end
-
-  def test_params
-    [:id, :title, :testing_framework_id, :description, :internal_description, :test_type, :xml_id, :validity, :timeout, :_destroy,
-     {files_attributes: file_params}]
-  end
-
-  def model_solution_params
-    [:id, :description, :internal_description, :xml_id, :_destroy, {files_attributes: file_params}]
-  end
-
-  def task_params
-    params.require(:task).permit(:title, :description, :internal_description, :parent_uuid, :language, :license_id,
-      :programming_language_id, :access_level, files_attributes: file_params, tests_attributes: test_params,
-      model_solutions_attributes: model_solution_params, label_names: [])
-  end
-
-  def group_tasks_params
-    params.require(:group_tasks).permit(group_ids: [])
-  end
-
-  def import_confirm_params
-    params.permit(:import_id, :subfile_id, :import_type)
-  end
-
   def set_user_for_api_request
     authorization_header = request.headers['Authorization']
     api_key = authorization_header&.split&.second

app/mailers/access_request_mailer.rb

@@ -9,13 +9,4 @@ def send_access_request(user, admin, group)
       mail(to: @admin.email, subject: t('groups.access_request_mailer.subject', user: @user.name, group: @group.name))
     end
   end
-
-  def send_contribution_request(author, exercise, user)
-    @author = author
-    @exercise = exercise
-    @user = user
-    I18n.with_locale(@author.preferred_locale || I18n.default_locale) do
-      mail(to: @author.email, subject: t('tasks.access_request_mailer.subject', user: @user.name, task: @exercise.title))
-    end
-  end
 end

app/mailers/task_contribution_mailer.rb

@@ -0,0 +1,33 @@
+# frozen_string_literal: true
+
+class TaskContributionMailer < ApplicationMailer
+  def send_contribution_request(task_contrib)
+    @task_author = task_contrib.base.user
+    @task_contrib = task_contrib
+    @contrib_author = task_contrib.suggestion.user
+    I18n.with_locale(@task_author.preferred_locale || I18n.default_locale) do
+      mail(to: @task_author.email,
+        subject: t('task_contributions.mailer.contribution_request.subject_message', contrib_author: @contrib_author,
+          task: @task_contrib.base.title))
+    end
+  end
+
+  def send_approval_info(task_contrib)
+    @task_contrib = task_contrib
+    @contrib_author = task_contrib.suggestion.user
+    I18n.with_locale(@contrib_author.preferred_locale || I18n.default_locale) do
+      mail(to: @contrib_author.email,
+        subject: t('task_contributions.mailer.approval_info.subject_message', task: @task_contrib.base.title))
+    end
+  end
+
+  def send_rejection_info(task_contrib, duplicate)
+    @task_contrib = task_contrib
+    @contrib_author = task_contrib.suggestion.user
+    @duplicate = duplicate
+    I18n.with_locale(@contrib_author.preferred_locale || I18n.default_locale) do
+      mail(to: @contrib_author.email,
+        subject: t('task_contributions.mailer.rejection_info.subject_message', task: @task_contrib.base.title))
+    end
+  end
+end

app/models/concerns/file_concern.rb

@@ -18,4 +18,15 @@ def files
 
     association(:files).reader.records.filter {|file| file.fileable == self }
   end
+
+  # For the transfer of files between two records, we need to overwrite the getter and setter methods for the files association.
+  # We cannot use the `files` method defined above, since it would return a copy of the respective files as an array.
+  # Hence, we define a dedicated getter and setter method for the files association used in the context of task contributions.
+  def files_collection
+    association(:files).reader
+  end
+
+  def files_collection=(files)
+    association(:files).writer files
+  end
 end

app/models/concerns/parent_validation.rb

@@ -0,0 +1,12 @@
+# frozen_string_literal: true
+
+module ParentValidation
+  def parent_validation_check
+    return if parent_id.nil?
+
+    parent_entry = self.class.find_by(id: parent_id)
+    if parent_entry.blank? || !parent_entry.task.parent_of?(task)
+      errors.add(:parent_id, :invalid)
+    end
+  end
+end

app/models/concerns/transfer_values.rb

@@ -0,0 +1,44 @@
+# frozen_string_literal: true
+
+module TransferValues
+  def transfer_attributes(other) # rubocop:disable Metrics/AbcSize
+    assign_attributes(other.attributes.except(*excluded_attributes_from_transfer))
+
+    if is_a?(TaskFile) && other.attachment.attached?
+      attachment.attach(other.attachment.blob)
+      self.use_attached_file = 'true'
+    elsif !is_a?(TaskFile)
+      transfer_multiple_entities(files_collection, other.files_collection)
+    end
+  end
+
+  def transfer_multiple_entities(targets, others)
+    # Remove deleted elements
+    targets.each do |target|
+      unless others.exists?(parent_id: target.id)
+        targets.delete(target)
+      end
+    end
+    # Adding new or modified elements
+    others.to_a.each do |other|
+      if targets.exists?(other.parent_id)
+        old_entity = targets.find {|target| target.id == other.parent_id } # Required to force searching a copy in memory instead of db
+        old_entity.transfer_attributes(other)
+      else
+        targets.append(other.duplicate(set_parent_id: false))
+      end
+    end
+  end
+
+  private
+
+  def excluded_attributes_from_transfer
+    exclude = %w[id parent_id created_at task_id]
+    if is_a?(TaskFile)
+      exclude.append('fileable_id', 'fileable_type')
+    else
+      exclude.append('xml_id')
+    end
+    exclude
+  end
+end

app/models/model_solution.rb

@@ -2,13 +2,28 @@
 
 class ModelSolution < ApplicationRecord
   include FileConcern
+  include TransferValues
+  include ParentValidation
+
   belongs_to :task, autosave: true, inverse_of: :model_solutions
+  belongs_to :parent, class_name: 'ModelSolution', optional: true
+  has_many :files, as: :fileable, class_name: 'TaskFile', dependent: :destroy
+  accepts_nested_attributes_for :files, allow_destroy: true
   validates :xml_id, presence: true
+  validates :parent_id, uniqueness: {scope: :task}, if: -> { parent_id.present? }
+  validate :parent_validation_check
+  # TODO: For new tasks, this validation is currently useless, because the validation is performed
+  # before the task is saved (and thus the task_id is not yet known, i.e., is NULL). Therefore,
+  # one can create a **new task** with a test that has the same xml_id as another test of the same task.
+  # TODO: This validation is currently useless on new records, because the uuid is generated after validation
   validates :xml_id, uniqueness: {scope: :task_id}
 
-  def duplicate
-    dup.tap do |model_solutions|
-      model_solutions.files = files.map(&:duplicate)
+  def duplicate(set_parent_id: true)
+    dup.tap do |model_solution|
+      model_solution.files = files.map {|file| file.duplicate(set_parent_id:) }
+      if set_parent_id
+        model_solution.parent_id = id
+      end
     end
   end
 end