Skip to content

Commit 58f281f

Browse files
committed
Only select the "latest email with patches" based on the extension
Otherwise we'll pick up emails not intendeded to be consumed by commitfest, and try to download a txt or something else instead of the latest patch.
1 parent c58dad6 commit 58f281f

4 files changed

Lines changed: 22 additions & 3 deletions

File tree

app/controllers/topics_controller.rb

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,7 @@ def show
6363
load_star_state
6464
end
6565

66-
# Check if topic has any patches (for sidebar button)
67-
@has_patches = @messages.any? { |msg| msg.attachments.any?(&:patch?) }
66+
@has_patches = @messages.any? { |msg| msg.attachments.any?(&:patch_extension?) }
6867
end
6968

7069
def aware
@@ -141,7 +140,7 @@ def latest_patchset
141140
.where(id: Attachment.where(message_id: @topic.messages.select(:id))
142141
.select(:message_id))
143142
.order(created_at: :desc)
144-
.find { |msg| msg.attachments.any?(&:patch?) }
143+
.find { |msg| msg.attachments.any?(&:patch_extension?) }
145144

146145
return head :not_found unless latest_message
147146

app/models/attachment.rb

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,10 @@ class Attachment < ApplicationRecord
88
def patch?
99
file_name&.ends_with?('.patch') || file_name&.ends_with?('.diff') || patch_content?
1010
end
11+
12+
def patch_extension?
13+
file_name&.ends_with?('.patch') || file_name&.ends_with?('.diff')
14+
end
1115

1216
def decoded_body
1317
Base64.decode64(body) if body.present?

spec/factories/attachments.rb

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,5 +24,11 @@
2424
content_type { "text/plain" }
2525
body { Base64.encode64("diff --git a/src/backend/test.c b/src/backend/test.c\nindex 123..456\n--- a/src/backend/test.c\n+++ b/src/backend/test.c\n@@ -1,3 +1,4 @@\n code\n+new line\n more code") }
2626
end
27+
28+
trait :content_based_patch do
29+
file_name { "changes.txt" }
30+
content_type { "text/plain" }
31+
body { Base64.encode64("diff --git a/src/backend/test.c b/src/backend/test.c\nindex 123..456\n--- a/src/backend/test.c\n+++ b/src/backend/test.c\n@@ -1,3 +1,4 @@\n code\n+new line\n more code") }
32+
end
2733
end
2834
end

spec/requests/topics_spec.rb

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,16 @@ def attach_verified_alias(user, email:, primary: true)
292292
end
293293
end
294294

295+
context "with content-based patches only (no .diff or .patch extension)" do
296+
let!(:message) { create(:message, topic: topic, sender: creator) }
297+
let!(:attachment) { create(:attachment, :content_based_patch, message: message) }
298+
299+
it "returns 404" do
300+
get latest_patchset_topic_path(topic)
301+
expect(response).to have_http_status(:not_found)
302+
end
303+
end
304+
295305
context "with nonexistent topic" do
296306
it "returns 404" do
297307
get latest_patchset_topic_path(id: 99999)

0 commit comments

Comments
 (0)