diff --git a/app/controllers/api/lessons_controller.rb b/app/controllers/api/lessons_controller.rb index 0d381e2ef..0f21d1260 100644 --- a/app/controllers/api/lessons_controller.rb +++ b/app/controllers/api/lessons_controller.rb @@ -2,6 +2,8 @@ module Api class LessonsController < ApiController + include RemixSelection + before_action :authorize_user, except: %i[index show] before_action :verify_school_class_belongs_to_school, only: :create load_and_authorize_resource :lesson @@ -82,24 +84,17 @@ def verify_school_class_belongs_to_school end def user_remixes(lessons) - lessons.map do |lesson| - next nil unless lesson&.project&.remixes&.any? - - user_remix(lesson) - end + lessons.map { |lesson| user_remix(lesson) } end def user_remix(lesson) - remixes = lesson&.project&.remixes + return nil unless lesson&.project&.remixes&.any? - remixes = remixes - .where(user_id: current_user.id) - .accessible_by(current_ability) - .order(created_at: :asc) - - remixes = remixes.includes(school_project: :feedback) if current_user&.school_student?(school) - - remixes.first + remix_for_user( + lesson.project, + current_user, + include_feedback: current_user&.school_student?(school) + ) end def lesson_params diff --git a/app/controllers/api/projects/remixes_controller.rb b/app/controllers/api/projects/remixes_controller.rb index 18bdb3867..e61a19667 100644 --- a/app/controllers/api/projects/remixes_controller.rb +++ b/app/controllers/api/projects/remixes_controller.rb @@ -3,6 +3,8 @@ module Api module Projects class RemixesController < ApiController + include RemixSelection + before_action :authorize_user load_and_authorize_resource :school, only: :index before_action :load_and_authorize_remix, only: %i[show show_identifier] @@ -44,7 +46,9 @@ def project end def load_and_authorize_remix - @project = Project.find_by!(remixed_from_id: project.id, user_id: current_user&.id) + @project = remix_for_user(project, current_user) + raise ActiveRecord::RecordNotFound unless @project + authorize! :show, @project end diff --git a/app/controllers/concerns/remix_selection.rb b/app/controllers/concerns/remix_selection.rb new file mode 100644 index 000000000..6375e63bf --- /dev/null +++ b/app/controllers/concerns/remix_selection.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +module RemixSelection + extend ActiveSupport::Concern + + def remix_for_user(project, user, include_feedback: false) + return nil if user.nil? + + query = Project.where(remixed_from_id: project.id, user_id: user.id) + .order(created_at: :asc) + .accessible_by(current_ability) + + query = query.includes(school_project: :feedback) if include_feedback + + query.first + end +end diff --git a/spec/requests/projects/remix_spec.rb b/spec/requests/projects/remix_spec.rb index 50edde286..7be77aaa0 100644 --- a/spec/requests/projects/remix_spec.rb +++ b/spec/requests/projects/remix_spec.rb @@ -71,6 +71,24 @@ expect(response).to have_http_status(:not_found) end + + context 'when multiple remixes exist for the same user and project' do + let!(:oldest_remix) do + create(:project, remixed_from_id: original_project.id, user_id: authenticated_user.id, + created_at: 2.days.ago, updated_at: 2.days.ago) + end + + before do + create(:project, remixed_from_id: original_project.id, user_id: authenticated_user.id, + created_at: 1.hour.from_now, updated_at: 1.hour.from_now) + end + + it 'returns the oldest created remix' do + get("/api/projects/#{original_project.identifier}/remix", headers:) + + expect(response.parsed_body['identifier']).to eq(oldest_remix.identifier) + end + end end describe('#show_identifier') do @@ -100,6 +118,23 @@ get("/api/projects/#{original_project.identifier}/remix/identifier", headers:) expect(response).to have_http_status(:not_found) end + + context 'when multiple remixes exist for the same user and project' do + let!(:oldest_remix) do + create(:project, remixed_from_id: original_project.id, user_id: authenticated_user.id, + created_at: 2.days.ago, updated_at: 2.days.ago) + end + + before do + create(:project, remixed_from_id: original_project.id, user_id: authenticated_user.id, + created_at: 1.hour.from_now, updated_at: 1.hour.from_now) + end + + it 'returns the identifier of the oldest created remix' do + get("/api/projects/#{original_project.identifier}/remix/identifier", headers:) + expect(response.parsed_body['identifier']).to eq(oldest_remix.identifier) + end + end end describe '#create' do