From 121a373e5dd1290df650fa635348838dc51aa8de Mon Sep 17 00:00:00 2001
From: Sebastian Serth <Sebastian.Serth@hpi.de>
Date: Sun, 1 Sep 2024 19:47:41 +0200
Subject: [PATCH 01/14] Correctly render task form during validation errors
 when a new file has been added

---
 app/views/tasks/_editor.html.slim | 8 +++++---
 app/views/tasks/_form.html.slim   | 2 +-
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/app/views/tasks/_editor.html.slim b/app/views/tasks/_editor.html.slim
index 3085b380e..dbcb7c509 100644
--- a/app/views/tasks/_editor.html.slim
+++ b/app/views/tasks/_editor.html.slim
@@ -13,13 +13,15 @@ a.btn.btn-main.btn-sm.toggle-input href='#'
     = file.hidden_field :use_attached_file, value: show_attachment, class: 'use-attached-file'
     - if file_attached
       .btn-group.attachment_present
-        - if file.object.attachment.image?
+        - if file.object.persisted? && file.object.attachment.image?
           = image_tag file.object.attachment.variant(resize_to_limit: [600, 600])
-        - else
+        - elsif file.object.persisted?
           = link_to file.object.attachment.filename, download_attachment_task_file_path(file.object), class: 'btn btn-light border-right-0', 'data-turbolinks' => false
+        - else
+          .btn.btn-light.border-right-0.disabled = file.object.attachment.filename
         = link_to '#', class: 'btn btn-light reupload-attachment border-left-0 me-1', title: t('.button.upload_a_new_file') do
           i.fa-solid.fa-upload.text-secondary
-        - if file.object.text_data?
+        - if file.object.persisted? && file.object.text_data?
           .extract-text.btn.btn-main data-file-id=file.object.id
             = t('.button.extract_text')
     .alternative class="#{file_attached ? 'hide' : ''}"
diff --git a/app/views/tasks/_form.html.slim b/app/views/tasks/_form.html.slim
index 9c052a44d..3a5c7c889 100644
--- a/app/views/tasks/_form.html.slim
+++ b/app/views/tasks/_form.html.slim
@@ -8,7 +8,7 @@
 .row
   .col-md-12.my-4
     - if @task.contribution?
-      - if action_name == 'show' || action_name == 'new'
+      - if action_name.in?(%w[new create show])
         - url = task_task_contributions_path(@task.parent)
       - else
         - url = task_task_contribution_path(@task.parent, @task.task_contribution)

From 5a028550d355c3f1b148301fbd0d2b40c308fb8e Mon Sep 17 00:00:00 2001
From: Sebastian Serth <Sebastian.Serth@hpi.de>
Date: Sun, 1 Sep 2024 19:43:26 +0200
Subject: [PATCH 02/14] Fix behavior for invalid task contributions

Previously, both the flash message was incomplete (since it didn't reveal the errors) nor was the action correct (it showed the original task rather than the form to fix problems).
---
 app/controllers/task_contributions_controller.rb       | 3 ++-
 spec/controllers/task_contributions_controller_spec.rb | 9 ++++-----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/app/controllers/task_contributions_controller.rb b/app/controllers/task_contributions_controller.rb
index 039b577fe..76026e6b8 100644
--- a/app/controllers/task_contributions_controller.rb
+++ b/app/controllers/task_contributions_controller.rb
@@ -83,7 +83,8 @@ def create # rubocop:disable Metrics/AbcSize
       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
-      redirect_to @task, alert: t('common.errors.model_not_saved', model: TaskContribution.model_name.human)
+      @task = @task_contribution.suggestion
+      render 'tasks/new'
     end
   end
 
diff --git a/spec/controllers/task_contributions_controller_spec.rb b/spec/controllers/task_contributions_controller_spec.rb
index 59f69d0ca..686cc0ff0 100644
--- a/spec/controllers/task_contributions_controller_spec.rb
+++ b/spec/controllers/task_contributions_controller_spec.rb
@@ -58,15 +58,14 @@
     context 'when new task is invalid' do
       let(:task_params) { {title: ''} }
 
-      it 'shows a flash message' do
+      it 'shows validation errors' do
         post_request
-        expect(flash[:alert]).to eq(I18n.t('common.errors.model_not_saved', model: 'Task Contribution'))
+        expect(assigns(:task).errors.messages).to include(:title)
       end
 
-      it 'redirects to the original task' do
+      it 'renders the form again' do
         post_request
-        expect(response).to have_http_status(:redirect)
-        expect(response).to redirect_to(task)
+        expect(response).to render_template('tasks/new')
       end
     end
   end

From ce7c37696dafc70431904216dc0bb384a57e7638 Mon Sep 17 00:00:00 2001
From: Sebastian Serth <Sebastian.Serth@hpi.de>
Date: Sun, 1 Sep 2024 19:52:45 +0200
Subject: [PATCH 03/14] Allow editing an existing contribution

---
 app/views/tasks/show.html.slim | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/views/tasks/show.html.slim b/app/views/tasks/show.html.slim
index 1b8aa733c..0c41bbaf1 100644
--- a/app/views/tasks/show.html.slim
+++ b/app/views/tasks/show.html.slim
@@ -581,7 +581,7 @@
           - edit_url = edit_task_task_contribution_path(@task.parent, @task.task_contribution)
         - else
           - edit_url = edit_task_path(@task)
-        - unless @task.contribution?
+        - if !@task.contribution? || @task.pending_contribution?
           = link_to edit_url, class: 'btn btn-important' do
             i.fa-solid.fa-pen-to-square
             =< t('common.button.edit')

From 3cf17c6a583877788f3ea7fdde8a9c2db8718e00 Mon Sep 17 00:00:00 2001
From: Sebastian Serth <Sebastian.Serth@hpi.de>
Date: Sun, 1 Sep 2024 16:31:11 +0200
Subject: [PATCH 04/14] Allow admins to perform any action on contributions

---
 app/policies/task_contribution_policy.rb | 22 ++++++++++++++--------
 app/policies/task_policy.rb              |  2 +-
 spec/policies/task_policy_spec.rb        |  4 ++--
 3 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/app/policies/task_contribution_policy.rb b/app/policies/task_contribution_policy.rb
index eb27974d0..e9b7f43a4 100644
--- a/app/policies/task_contribution_policy.rb
+++ b/app/policies/task_contribution_policy.rb
@@ -19,27 +19,33 @@ def base
     end
   end
 
-  # While discard_changes? and show? look similar, the difference in the position of parentheses is relevant
-  # discard_changes? can only be executed while the task_contribution is pending by either the owner of the base task or the contrib author
-  # show? allows the owner of the base task to view it anytime.
-  # Simultaneously, the contributor can view it only while the contrib is still pending
   def discard_changes?
-    task_contribution.pending? && record_owner?
+    return no_one unless task_contribution.pending?
+
+    record_owner? || admin?
   end
 
   def show?
-    (task_contribution.pending? && record_owner?) || Pundit.policy(@user, base).edit?
+    # show? allows the owner of the base task to view it anytime.
+    # Simultaneously, the contributor can view it only while the contrib is still pending
+    return true if Pundit.policy(@user, base).edit?
+
+    task_contribution.pending? && record_owner?
   end
 
   %i[approve_changes? reject_changes?].each do |action|
     define_method(action) do
-      Pundit.policy(@user, base).edit? && task_contribution.pending?
+      return no_one unless task_contribution.pending?
+
+      Pundit.policy(@user, base).edit?
     end
   end
 
   %i[edit? update?].each do |action|
     define_method(action) do
-      record_owner? && task_contribution.pending?
+      return no_one unless task_contribution.pending?
+
+      record_owner? || admin?
     end
   end
 
diff --git a/app/policies/task_policy.rb b/app/policies/task_policy.rb
index a2b1fbccb..8883eca84 100644
--- a/app/policies/task_policy.rb
+++ b/app/policies/task_policy.rb
@@ -60,7 +60,7 @@ def generate_test?
 
   def contribute?(check_other_contrib: true)
     return false if @user.blank? # Disallow contributing without an active session (anonymous user).
-    return false unless show? && !edit? # Contributing is only allowed if a user cannot make direct changes.
+    return false unless (show? && !edit?) || admin? # Contributing is only allowed if a user cannot make direct changes and is not an admin.
     return true unless check_other_contrib # Allow action, if existing contributions are irrelevant.
 
     # Contributing is only allowed if no other pending contributions exists.
diff --git a/spec/policies/task_policy_spec.rb b/spec/policies/task_policy_spec.rb
index c536b351a..c86776db3 100644
--- a/spec/policies/task_policy_spec.rb
+++ b/spec/policies/task_policy_spec.rb
@@ -40,7 +40,7 @@
       context 'without gpt access token' do
         let(:openai_api_key) { nil }
 
-        it { is_expected.to forbid_only_actions %i[generate_test contribute] }
+        it { is_expected.to forbid_only_actions %i[generate_test] }
         it { is_expected.to permit_actions(generic_user_permissions) }
       end
 
@@ -53,7 +53,7 @@
           Settings.open_ai.access_token = nil
         end
 
-        it { is_expected.to forbid_only_actions %i[generate_test contribute] }
+        it { is_expected.to permit_all_actions }
       end
     end
 

From 3386d8b48f69ccebc450e9250080cd9cbb5b7ae8 Mon Sep 17 00:00:00 2001
From: Sebastian Serth <Sebastian.Serth@hpi.de>
Date: Sun, 1 Sep 2024 19:54:09 +0200
Subject: [PATCH 05/14] Allow new task contributions without changes to
 attached files

---
 app/controllers/concerns/task_parameters.rb |  2 +-
 app/models/task_file.rb                     | 12 +++++++++++-
 app/views/tasks/_editor.html.slim           |  6 ++++--
 3 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/app/controllers/concerns/task_parameters.rb b/app/controllers/concerns/task_parameters.rb
index 0a4e39d88..9fe175adf 100644
--- a/app/controllers/concerns/task_parameters.rb
+++ b/app/controllers/concerns/task_parameters.rb
@@ -5,7 +5,7 @@ module TaskParameters
 
   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_id parent_blob_id]
   end
 
   def test_params
diff --git a/app/models/task_file.rb b/app/models/task_file.rb
index 7314c188b..08320ab2b 100644
--- a/app/models/task_file.rb
+++ b/app/models/task_file.rb
@@ -15,11 +15,14 @@ class TaskFile < ApplicationRecord
   validates :parent_id, uniqueness: {scope: %i[fileable_id fileable_type]}, if: -> { parent_id.present? }
   validate :unique_xml_id, if: -> { !fileable.nil? }
   validate :parent_validation_check
-  attr_accessor :use_attached_file, :file_marked_for_deletion
+  attr_accessor :use_attached_file, :file_marked_for_deletion, :parent_blob_id
 
+  before_validation :attach_parent_blob, if: -> { attachment.blank? && task&.contribution? && parent.present? && parent_blob_id.present? }
   before_save :remove_attachment
 
   def task
+    return nil unless fileable
+
     if fileable.is_a?(Task)
       # This file is directly attached to a task
       fileable
@@ -67,6 +70,13 @@ def extract_text_data
 
   private
 
+  def attach_parent_blob
+    # The comparison of the blob ID is used as a safeguard to ensure updated files in a task contribution are not overwritten unintended.
+    return unless parent_blob_id.to_i == parent.attachment.blob.id
+
+    attachment.attach(parent.attachment.blob)
+  end
+
   def remove_attachment
     attachment.purge if use_attached_file != 'true' && attachment.present?
   end
diff --git a/app/views/tasks/_editor.html.slim b/app/views/tasks/_editor.html.slim
index dbcb7c509..9934063a1 100644
--- a/app/views/tasks/_editor.html.slim
+++ b/app/views/tasks/_editor.html.slim
@@ -1,3 +1,4 @@
+- parent_blob_id = file.object.attachment&.blob&.id
 - file_attached = file.object.attachment.attached?
 - show_attachment = file_attached || file.object.use_attached_file
 
@@ -11,7 +12,8 @@ a.btn.btn-main.btn-sm.toggle-input href='#'
       = file.text_area :content
   .attachment class="#{show_attachment ? '' : 'hide'}"
     = file.hidden_field :use_attached_file, value: show_attachment, class: 'use-attached-file'
-    - if file_attached
+    = file.hidden_field :parent_blob_id, value: parent_blob_id
+    - if file_attached && parent_blob_id
       .btn-group.attachment_present
         - if file.object.persisted? && file.object.attachment.image?
           = image_tag file.object.attachment.variant(resize_to_limit: [600, 600])
@@ -24,5 +26,5 @@ a.btn.btn-main.btn-sm.toggle-input href='#'
         - if file.object.persisted? && file.object.text_data?
           .extract-text.btn.btn-main data-file-id=file.object.id
             = t('.button.extract_text')
-    .alternative class="#{file_attached ? 'hide' : ''}"
+    .alternative class="#{file_attached && parent_blob_id ? 'hide' : ''}"
       = file.file_field :attachment, class: 'alternative-input form-control'

From 0247f2a70b60344837ef53d8e83dfcc9f2f4ab9a Mon Sep 17 00:00:00 2001
From: Sebastian Serth <Sebastian.Serth@hpi.de>
Date: Sun, 1 Sep 2024 20:37:17 +0200
Subject: [PATCH 06/14] Correctly apply changes to attached files when
 accepting a contribution

---
 app/models/concerns/transfer_values.rb | 9 ++++++---
 app/models/task.rb                     | 2 +-
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/app/models/concerns/transfer_values.rb b/app/models/concerns/transfer_values.rb
index 93b15b514..bb9f75cb0 100644
--- a/app/models/concerns/transfer_values.rb
+++ b/app/models/concerns/transfer_values.rb
@@ -11,9 +11,12 @@ def excluded_attributes(entity_type)
     exclude
   end
 
-  def transfer_attributes(other, excluded_attributes = [], has_files: true)
+  def transfer_attributes(other, excluded_attributes = [])
     assign_attributes(other.attributes.except(*excluded_attributes))
-    if has_files
+    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, other.files, 'task_file')
     end
   end
@@ -29,7 +32,7 @@ def transfer_multiple_entities(targets, others, entity_type) # rubocop:disable M
     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, excluded_attributes(entity_type), has_files: entity_type != 'task_file')
+        old_entity.transfer_attributes(other, excluded_attributes(entity_type))
       else
         targets.append(other.duplicate(set_parent_id: false))
       end
diff --git a/app/models/task.rb b/app/models/task.rb
index 17055ba67..1bc5ca9e1 100644
--- a/app/models/task.rb
+++ b/app/models/task.rb
@@ -138,7 +138,7 @@ def pending_contribution?
   end
 
   def apply_contribution(contrib)
-    transfer_attributes(contrib.suggestion, %w[id parent_uuid access_level user_id uuid created_at], has_files: true)
+    transfer_attributes(contrib.suggestion, %w[id parent_uuid access_level user_id uuid created_at])
     transfer_multiple_entities(model_solutions, contrib.suggestion.model_solutions, 'model_solution')
     transfer_multiple_entities(tests, contrib.suggestion.tests, 'test')
     contrib.status = :merged

From ef3bcc238116ee47a79fec8a856e35d4766fc9b7 Mon Sep 17 00:00:00 2001
From: Sebastian Serth <Sebastian.Serth@hpi.de>
Date: Sun, 1 Sep 2024 20:37:45 +0200
Subject: [PATCH 07/14] Disallow changes to the polymorphic file_type when
 transferring values

---
 app/models/concerns/transfer_values.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/models/concerns/transfer_values.rb b/app/models/concerns/transfer_values.rb
index bb9f75cb0..72ebb76b4 100644
--- a/app/models/concerns/transfer_values.rb
+++ b/app/models/concerns/transfer_values.rb
@@ -4,7 +4,7 @@ module TransferValues
   def excluded_attributes(entity_type)
     exclude = %w[id parent_id created_at task_id]
     if entity_type == 'task_file'
-      exclude.append('fileable_id')
+      exclude.append('fileable_id', 'fileable_type')
     else
       exclude.append('xml_id')
     end

From aed3f63424b59df7b4c19b009474b18bc79fa050 Mon Sep 17 00:00:00 2001
From: Sebastian Serth <Sebastian.Serth@hpi.de>
Date: Sun, 1 Sep 2024 20:47:58 +0200
Subject: [PATCH 08/14] Refactor exclusion of attributes during transfer

* Make method private
* Reduce parameters and avoid passing strings of class names
* Overwrite for file specifically
---
 app/models/concerns/transfer_values.rb        | 31 ++++++++++---------
 app/models/task.rb                            | 10 ++++--
 .../support/shared_examples/transfer_files.rb |  2 +-
 .../transfer_multiple_entities.rb             |  2 +-
 4 files changed, 26 insertions(+), 19 deletions(-)

diff --git a/app/models/concerns/transfer_values.rb b/app/models/concerns/transfer_values.rb
index 72ebb76b4..fc82d28f0 100644
--- a/app/models/concerns/transfer_values.rb
+++ b/app/models/concerns/transfer_values.rb
@@ -1,27 +1,18 @@
 # frozen_string_literal: true
 
 module TransferValues
-  def excluded_attributes(entity_type)
-    exclude = %w[id parent_id created_at task_id]
-    if entity_type == 'task_file'
-      exclude.append('fileable_id', 'fileable_type')
-    else
-      exclude.append('xml_id')
-    end
-    exclude
-  end
+  def transfer_attributes(other) # rubocop:disable Metrics/AbcSize
+    assign_attributes(other.attributes.except(*excluded_attributes_from_transfer))
 
-  def transfer_attributes(other, excluded_attributes = [])
-    assign_attributes(other.attributes.except(*excluded_attributes))
     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, other.files, 'task_file')
+      transfer_multiple_entities(files, other.files)
     end
   end
 
-  def transfer_multiple_entities(targets, others, entity_type) # rubocop:disable Metrics/AbcSize
+  def transfer_multiple_entities(targets, others)
     # Remove deleted elements
     targets.each do |target|
       unless others.exists?(parent_id: target.id)
@@ -32,10 +23,22 @@ def transfer_multiple_entities(targets, others, entity_type) # rubocop:disable M
     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, excluded_attributes(entity_type))
+        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
diff --git a/app/models/task.rb b/app/models/task.rb
index 1bc5ca9e1..7bad0b9b2 100644
--- a/app/models/task.rb
+++ b/app/models/task.rb
@@ -138,9 +138,9 @@ def pending_contribution?
   end
 
   def apply_contribution(contrib)
-    transfer_attributes(contrib.suggestion, %w[id parent_uuid access_level user_id uuid created_at])
-    transfer_multiple_entities(model_solutions, contrib.suggestion.model_solutions, 'model_solution')
-    transfer_multiple_entities(tests, contrib.suggestion.tests, 'test')
+    transfer_attributes(contrib.suggestion)
+    transfer_multiple_entities(model_solutions, contrib.suggestion.model_solutions)
+    transfer_multiple_entities(tests, contrib.suggestion.tests)
     contrib.status = :merged
     self.label_names = contrib.suggestion.label_names # TODO: Write a test that changes in labels are correctly applied.
     save && contrib.save
@@ -298,4 +298,8 @@ def no_license_change_on_duplicate
       errors.add(:license, :cannot_change_on_duplicate)
     end
   end
+
+  def excluded_attributes_from_transfer
+    %w[id parent_uuid access_level user_id uuid created_at]
+  end
 end
diff --git a/spec/support/shared_examples/transfer_files.rb b/spec/support/shared_examples/transfer_files.rb
index 04d55eecc..467b5cd50 100644
--- a/spec/support/shared_examples/transfer_files.rb
+++ b/spec/support/shared_examples/transfer_files.rb
@@ -6,7 +6,7 @@ def entities(model)
     model.to_s.pluralize
   end
 
-  subject(:transfer_files) { task.transfer_multiple_entities(task.send(entities(model)), other_task.send(entities(model)), model.to_s) }
+  subject(:transfer_files) { task.transfer_multiple_entities(task.send(entities(model)), other_task.send(entities(model))) }
 
   let(:task) { create(:task) }
   let(:other_task) { create(:task, parent_uuid: task.uuid) }
diff --git a/spec/support/shared_examples/transfer_multiple_entities.rb b/spec/support/shared_examples/transfer_multiple_entities.rb
index 0e5b08a34..40838dde3 100644
--- a/spec/support/shared_examples/transfer_multiple_entities.rb
+++ b/spec/support/shared_examples/transfer_multiple_entities.rb
@@ -5,7 +5,7 @@ def entities(model)
     model.to_s == 'task_file' ? 'files' : model.to_s.pluralize
   end
 
-  subject(:transfer_multiple) { task.transfer_multiple_entities(task.send(entities(model)), other_task.send(entities(model)), model.to_s) }
+  subject(:transfer_multiple) { task.transfer_multiple_entities(task.send(entities(model)), other_task.send(entities(model))) }
 
   let(:task) { create(:task) }
   let(:other_task) { create(:task, parent_uuid: task.uuid) }

From f09ea3dc9585b6202e9200b9224c21638e3707ab Mon Sep 17 00:00:00 2001
From: Sebastian Serth <Sebastian.Serth@hpi.de>
Date: Sun, 1 Sep 2024 21:21:16 +0200
Subject: [PATCH 09/14] Refactor transfer specs to pass class names rather than
 symbols

---
 spec/models/model_solution_spec.rb            |  4 +-
 spec/models/task_file_spec.rb                 |  2 +-
 spec/models/test_spec.rb                      |  4 +-
 .../support/shared_examples/transfer_files.rb |  8 ++--
 .../transfer_multiple_entities.rb             | 37 ++++++++++---------
 5 files changed, 30 insertions(+), 25 deletions(-)

diff --git a/spec/models/model_solution_spec.rb b/spec/models/model_solution_spec.rb
index 095c64648..9f8a9a9ce 100644
--- a/spec/models/model_solution_spec.rb
+++ b/spec/models/model_solution_spec.rb
@@ -38,7 +38,7 @@
   end
 
   describe '#transfer_multiple_entities' do
-    it_behaves_like 'transfer multiple entities', :model_solution
-    it_behaves_like 'transfer files', :model_solution
+    it_behaves_like 'transfer multiple entities', described_class
+    it_behaves_like 'transfer files', described_class
   end
 end
diff --git a/spec/models/task_file_spec.rb b/spec/models/task_file_spec.rb
index 073f1de96..9be1cbf17 100644
--- a/spec/models/task_file_spec.rb
+++ b/spec/models/task_file_spec.rb
@@ -219,6 +219,6 @@
   end
 
   describe '#transfer_multiple_entities' do
-    it_behaves_like 'transfer multiple entities', :task_file
+    it_behaves_like 'transfer multiple entities', described_class
   end
 end
diff --git a/spec/models/test_spec.rb b/spec/models/test_spec.rb
index 775eb5be5..f15d1bade 100644
--- a/spec/models/test_spec.rb
+++ b/spec/models/test_spec.rb
@@ -37,7 +37,7 @@
   end
 
   describe '#transfer_multiple_entities' do
-    it_behaves_like 'transfer multiple entities', :test
-    it_behaves_like 'transfer files', :test
+    it_behaves_like 'transfer multiple entities', described_class
+    it_behaves_like 'transfer files', described_class
   end
 end
diff --git a/spec/support/shared_examples/transfer_files.rb b/spec/support/shared_examples/transfer_files.rb
index 467b5cd50..9fc56acfd 100644
--- a/spec/support/shared_examples/transfer_files.rb
+++ b/spec/support/shared_examples/transfer_files.rb
@@ -3,15 +3,17 @@
 # The primary logic is already covered by 'transfer multiple entities' this is just used to test the recursive call for files
 RSpec.shared_examples 'transfer files' do |model|
   def entities(model)
-    model.to_s.pluralize
+    model.name.underscore.pluralize.to_sym
   end
 
+  let(:model_factory) { model.name.underscore.to_sym }
+
   subject(:transfer_files) { task.transfer_multiple_entities(task.send(entities(model)), other_task.send(entities(model))) }
 
   let(:task) { create(:task) }
   let(:other_task) { create(:task, parent_uuid: task.uuid) }
-  let(:instance1) { build(model, internal_description: 'Instance 1') }
-  let(:instance2) { build(model, internal_description: 'Instance 2') }
+  let(:instance1) { build(model_factory, internal_description: 'Instance 1') }
+  let(:instance2) { build(model_factory, internal_description: 'Instance 2') }
 
   context 'when the entity exists in both tasks' do
     let(:file1) { build(:task_file, name: 'File 1') }
diff --git a/spec/support/shared_examples/transfer_multiple_entities.rb b/spec/support/shared_examples/transfer_multiple_entities.rb
index 40838dde3..9a7cdd2dd 100644
--- a/spec/support/shared_examples/transfer_multiple_entities.rb
+++ b/spec/support/shared_examples/transfer_multiple_entities.rb
@@ -5,60 +5,63 @@ def entities(model)
     model.to_s == 'task_file' ? 'files' : model.to_s.pluralize
   end
 
-  subject(:transfer_multiple) { task.transfer_multiple_entities(task.send(entities(model)), other_task.send(entities(model))) }
+  let(:entities) { model == TaskFile ? :files : model.name.underscore.pluralize }
+  let(:model_factory) { model.name.underscore.to_sym }
+
+  subject(:transfer_multiple) { task.transfer_multiple_entities(task.send(entities), other_task.send(entities)) }
 
   let(:task) { create(:task) }
   let(:other_task) { create(:task, parent_uuid: task.uuid) }
-  let(:instance1) { build(model, internal_description: 'Instance 1') }
-  let(:instance2) { build(model, internal_description: 'Instance 2') }
-  let(:instance3) { build(model, internal_description: 'Instance 3') }
+  let(:instance1) { build(model_factory, internal_description: 'Instance 1') }
+  let(:instance2) { build(model_factory, internal_description: 'Instance 2') }
+  let(:instance3) { build(model_factory, internal_description: 'Instance 3') }
 
   context 'when the entity only exists in the original task' do
     before do
       instance3.parent = instance1
-      task.send(:"#{entities(model)}=", [instance1, instance2])
-      other_task.send(:"#{entities(model)}=", [instance3])
+      task.send(:"#{entities}=", [instance1, instance2])
+      other_task.send(:"#{entities}=", [instance3])
     end
 
     it 'deletes the other instance' do
       transfer_multiple
-      expect(task.send(entities(model))).to contain_exactly(instance1)
+      expect(task.send(entities)).to contain_exactly(instance1)
     end
   end
 
   context 'when the entity exists in both tasks' do
-    let(:instance4) { build(model, internal_description: 'Instance 4') }
+    let(:instance4) { build(model_factory, internal_description: 'Instance 4') }
 
     before do
       instance2.parent = instance1
       instance4.parent = instance3
-      task.send(:"#{entities(model)}=", [instance1, instance3])
-      other_task.send(:"#{entities(model)}=", [instance2, instance4])
+      task.send(:"#{entities}=", [instance1, instance3])
+      other_task.send(:"#{entities}=", [instance2, instance4])
     end
 
     it 'contains the correct instances' do
       transfer_multiple
-      expect(task.send(entities(model))).to contain_exactly(instance1, instance3)
+      expect(task.send(entities)).to contain_exactly(instance1, instance3)
     end
 
     it 'contains the modified version' do
       transfer_multiple
-      expect(task.send(entities(model)).find {|i| i.internal_description == 'Instance 2' }).to be_present
-      expect(task.send(entities(model)).find {|i| i.internal_description == 'Instance 4' }).to be_present
+      expect(task.send(entities).find {|i| i.internal_description == 'Instance 2' }).to be_present
+      expect(task.send(entities).find {|i| i.internal_description == 'Instance 4' }).to be_present
     end
   end
 
   context 'when the entity only exists in the other task' do
     before do
       instance3.parent = instance1
-      task.send(:"#{entities(model)}=", [instance1])
-      other_task.send(:"#{entities(model)}=", [instance2, instance3])
+      task.send(:"#{entities}=", [instance1])
+      other_task.send(:"#{entities}=", [instance2, instance3])
     end
 
     it 'contains the correct instances' do
       transfer_multiple
-      expect(task.send(entities(model)).find {|i| i.internal_description == 'Instance 2' }).to be_present
-      expect(task.send(entities(model)).find {|i| i.internal_description == 'Instance 3' }).to be_present
+      expect(task.send(entities).find {|i| i.internal_description == 'Instance 2' }).to be_present
+      expect(task.send(entities).find {|i| i.internal_description == 'Instance 3' }).to be_present
     end
   end
 end

From be56c69ea1f6d8d1c4fd295a9bd580c5c291851f Mon Sep 17 00:00:00 2001
From: Sebastian Serth <Sebastian.Serth@hpi.de>
Date: Sun, 1 Sep 2024 19:57:11 +0200
Subject: [PATCH 10/14] Remove inaccessible task contributions from tasks index
 page

For now, we decided to keep the current behavior. Further investigations should happen in the future, for which we will create a new GitHub issue.
---
 app/models/task.rb                             | 3 ++-
 spec/policies/task_contribution_policy_spec.rb | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/app/models/task.rb b/app/models/task.rb
index 7bad0b9b2..b83971c77 100644
--- a/app/models/task.rb
+++ b/app/models/task.rb
@@ -79,7 +79,8 @@ class Task < ApplicationRecord # rubocop:disable Metrics/ClassLength
     where.missing(:task_contribution)
   }
   scope :pending_contribution, lambda {|user|
-    joins(:task_contribution).where(user:, task_contribution: {status: :pending})
+    # Since accessing any contribution that belongs to a non-public task doesn't work, we filter for the parent visibility.
+    joins(:task_contribution, :parent).where(user:, task_contribution: {status: :pending}, parent: access_level_public)
   }
   scope :created_before_days, ->(days) { where(created_at: days.to_i.days.ago.beginning_of_day..) if days.to_i.positive? }
   scope :min_stars, lambda {|stars|
diff --git a/spec/policies/task_contribution_policy_spec.rb b/spec/policies/task_contribution_policy_spec.rb
index 69890f370..b6ee070a2 100644
--- a/spec/policies/task_contribution_policy_spec.rb
+++ b/spec/policies/task_contribution_policy_spec.rb
@@ -24,7 +24,7 @@
       context 'when the original task is private' do
         let(:access_level) { 'private' }
 
-        it { is_expected.to permit_only_actions %i[discard_changes show update edit] } # TODO: This currently tests the case where a public task with a contrib becomes private. We should evaluate in the future what should happen in this case.
+        it { is_expected.to permit_only_actions %i[discard_changes show update edit] }
       end
 
       context 'when the original task is public' do

From ed9def355772de80c5afa3b173c4f551296a54e9 Mon Sep 17 00:00:00 2001
From: Sebastian Serth <Sebastian.Serth@hpi.de>
Date: Sun, 1 Sep 2024 20:02:47 +0200
Subject: [PATCH 11/14] Include task name in task contributions index page

---
 app/models/task_contribution.rb | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/app/models/task_contribution.rb b/app/models/task_contribution.rb
index 9654db5ac..04b62752a 100644
--- a/app/models/task_contribution.rb
+++ b/app/models/task_contribution.rb
@@ -33,6 +33,10 @@ def decouple
     end
   end
 
+  def self.parent_resource
+    Task
+  end
+
   def to_s
     I18n.t('task_contributions.model.contribution_title', task_title: base.title)
   end

From d2d46edc64e5aa388f3fcaa30c2aaab97b792de9 Mon Sep 17 00:00:00 2001
From: Sebastian Serth <Sebastian.Serth@hpi.de>
Date: Sun, 1 Sep 2024 20:49:34 +0200
Subject: [PATCH 12/14] Reverse ordering of task contributions to show newest
 first

---
 app/controllers/task_contributions_controller.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/controllers/task_contributions_controller.rb b/app/controllers/task_contributions_controller.rb
index 76026e6b8..a671188a3 100644
--- a/app/controllers/task_contributions_controller.rb
+++ b/app/controllers/task_contributions_controller.rb
@@ -36,7 +36,7 @@ def reject_changes
 
   def index
     authorize @task, :edit?
-    @task_contributions = @task.contributions
+    @task_contributions = @task.contributions.order(created_at: :desc)
   end
 
   def show

From f2dfd401c707e529e59a9b76b182015e65c23f76 Mon Sep 17 00:00:00 2001
From: Sebastian Serth <Sebastian.Serth@hpi.de>
Date: Sun, 1 Sep 2024 19:56:28 +0200
Subject: [PATCH 13/14] Fix capitalization of action headers for task show

---
 config/locales/en/views/tasks.yml | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/config/locales/en/views/tasks.yml b/config/locales/en/views/tasks.yml
index 27498a03b..fca06a10d 100644
--- a/config/locales/en/views/tasks.yml
+++ b/config/locales/en/views/tasks.yml
@@ -76,29 +76,29 @@ en:
       contribution_info: Contribution Information
       new_comment: New Comment
     show:
-      add_to_collection_hint: Save Tasks for later by adding them to a collection.
+      add_to_collection_hint: Save tasks for later by adding them to a collection.
       button:
         add_to_collection: Add to Collection
         api_key_required: OpenAI API key is required to generate a test
         contribution_list: View Related Contributions
         create_collection: Create new Collection
-        download_as_zip: Download this Task as a ZIP file.
+        download_as_zip: Download this task as a ZIP file.
         export: Export
         export_tasks: Export Tasks
         generate_test: Generate Unit Tests
         rate: Rate
         show_comments: Show Comments
-      contribution_list_hint: View Contributions linked to this task.
+      contribution_list_hint: View contributions linked to this task.
       create_collection_placeholder: Collection name
       define_account_link: Please define an account link first
       export_to: Export to
       guest:
         disabled:
           tooltip: You need to be logged in to use this.
-      no_files: No files included
+      no_files: No Files included
       no_license: None
       no_model_solution_present: No Model Solutions present
-      no_tests: No tests included
+      no_tests: No Tests included
       owner_required_tooltip: This feature can only be used by the owner of the task.
       remove_task_from_collection_warning: Are you sure you want to remove this Task from the Collection?
       task_contribution:

From baaabb9f5658357d8465191bf2d18bdaa555fe81 Mon Sep 17 00:00:00 2001
From: Sebastian Serth <Sebastian.Serth@hpi.de>
Date: Sun, 1 Sep 2024 20:09:25 +0200
Subject: [PATCH 14/14] Used prefixed enum for contribution status

Also, use the newer syntax, so that the code is already compatible with Rails 7.2 and higher.
---
 app/models/task.rb                       | 2 +-
 app/models/task_contribution.rb          | 3 +--
 app/policies/task_contribution_policy.rb | 8 ++++----
 3 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/app/models/task.rb b/app/models/task.rb
index b83971c77..ab08cad5a 100644
--- a/app/models/task.rb
+++ b/app/models/task.rb
@@ -135,7 +135,7 @@ def contribution?
   end
 
   def pending_contribution?
-    contribution? && task_contribution.status == 'pending'
+    contribution? && task_contribution.status_pending?
   end
 
   def apply_contribution(contrib)
diff --git a/app/models/task_contribution.rb b/app/models/task_contribution.rb
index 04b62752a..12ed7e4b0 100644
--- a/app/models/task_contribution.rb
+++ b/app/models/task_contribution.rb
@@ -12,8 +12,7 @@ class TaskContribution < ApplicationRecord
 
   scope :for_task_uuid, ->(uuid) { joins(:suggestion).where(suggestion: {parent_uuid: uuid}) }
 
-  # TODO: Use `_prefix: true`
-  enum status: {pending: 0, closed: 1, merged: 2}, _default: :pending
+  enum :status, {pending: 0, closed: 1, merged: 2}, default: :pending, prefix: true
 
   def self.new_for(task, user)
     suggestion = task.clean_duplicate(user, change_title: false)
diff --git a/app/policies/task_contribution_policy.rb b/app/policies/task_contribution_policy.rb
index e9b7f43a4..07cb20034 100644
--- a/app/policies/task_contribution_policy.rb
+++ b/app/policies/task_contribution_policy.rb
@@ -20,7 +20,7 @@ def base
   end
 
   def discard_changes?
-    return no_one unless task_contribution.pending?
+    return no_one unless task_contribution.status_pending?
 
     record_owner? || admin?
   end
@@ -30,12 +30,12 @@ def show?
     # Simultaneously, the contributor can view it only while the contrib is still pending
     return true if Pundit.policy(@user, base).edit?
 
-    task_contribution.pending? && record_owner?
+    task_contribution.status_pending? && record_owner?
   end
 
   %i[approve_changes? reject_changes?].each do |action|
     define_method(action) do
-      return no_one unless task_contribution.pending?
+      return no_one unless task_contribution.status_pending?
 
       Pundit.policy(@user, base).edit?
     end
@@ -43,7 +43,7 @@ def show?
 
   %i[edit? update?].each do |action|
     define_method(action) do
-      return no_one unless task_contribution.pending?
+      return no_one unless task_contribution.status_pending?
 
       record_owner? || admin?
     end