Skip to content

Commit dac5345

Browse files
committed
Make load_and_authorize_remix deterministic.
This commit changes RemixesController#load_and_authorize_remix to deterministically always return the most recent Project record based on created_at and, secondarily, updated_at dates. While, in principle, there should not be multiple records where (remixed_from_id:, user_id:) are the same, in practice there are and there is no database constraint to prevent there being. The previous code would nondeterministically return one of possibly many records, causing downstream errors in Experience CS since each would have a different identifier, not matching the one stored in Experience CS. Overarching epic: RaspberryPiFoundation/experience-cs#1826
1 parent 062a116 commit dac5345

2 files changed

Lines changed: 38 additions & 1 deletion

File tree

app/controllers/api/projects/remixes_controller.rb

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,9 @@ def project
4444
end
4545

4646
def load_and_authorize_remix
47-
@project = Project.find_by!(remixed_from_id: project.id, user_id: current_user&.id)
47+
@project = Project.where(remixed_from_id: project.id, user_id: current_user&.id)
48+
.order(created_at: :desc, updated_at: :desc)
49+
.first!
4850
authorize! :show, @project
4951
end
5052

spec/requests/projects/remix_spec.rb

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,24 @@
6868

6969
expect(response).to have_http_status(:not_found)
7070
end
71+
72+
context 'when multiple remixes exist for the same user and project' do
73+
before do
74+
create(:project, remixed_from_id: original_project.id, user_id: authenticated_user.id,
75+
created_at: 2.days.ago, updated_at: 2.days.ago)
76+
end
77+
78+
let!(:newer_remix) do
79+
create(:project, remixed_from_id: original_project.id, user_id: authenticated_user.id,
80+
created_at: 1.hour.from_now, updated_at: 1.hour.from_now)
81+
end
82+
83+
it 'returns the most recently created remix' do
84+
get("/api/projects/#{original_project.identifier}/remix", headers:)
85+
86+
expect(response.parsed_body['identifier']).to eq(newer_remix.identifier)
87+
end
88+
end
7189
end
7290

7391
describe('#show_identifier') do
@@ -97,6 +115,23 @@
97115
get("/api/projects/#{original_project.identifier}/remix/identifier", headers:)
98116
expect(response).to have_http_status(:not_found)
99117
end
118+
119+
context 'when multiple remixes exist for the same user and project' do
120+
before do
121+
create(:project, remixed_from_id: original_project.id, user_id: authenticated_user.id,
122+
created_at: 2.days.ago, updated_at: 2.days.ago)
123+
end
124+
125+
let!(:newer_remix) do
126+
create(:project, remixed_from_id: original_project.id, user_id: authenticated_user.id,
127+
created_at: 1.hour.from_now, updated_at: 1.hour.from_now)
128+
end
129+
130+
it 'returns the identifier of the most recently created remix' do
131+
get("/api/projects/#{original_project.identifier}/remix/identifier", headers:)
132+
expect(response.parsed_body['identifier']).to eq(newer_remix.identifier)
133+
end
134+
end
100135
end
101136

102137
describe '#create' do

0 commit comments

Comments
 (0)