Skip to content

Commit 907d2d6

Browse files
authored
AO3-7224 Eliminate chilled string warnings in our own code (#5503)
* AO3-7224 Eliminate chilled string warnings in our own code * AO3-7224 Fix unsanitized wildcards in fandom_string
1 parent 3a882c3 commit 907d2d6

File tree

6 files changed

+29
-47
lines changed

6 files changed

+29
-47
lines changed

app/controllers/archive_faqs_controller.rb

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,13 +42,12 @@ def show
4242
protected
4343

4444
def build_questions
45-
notice = +""
4645
num_to_build = params["num_questions"] ? params["num_questions"].to_i : @archive_faq.questions.count
4746
if num_to_build < @archive_faq.questions.count
48-
notice += ts("There are currently %{num} questions. You can only submit a number equal to or greater than %{num}. ", num: @archive_faq.questions.count)
47+
notice = ts("There are currently %{num} questions. You can only submit a number equal to or greater than %{num}. ", num: @archive_faq.questions.count)
4948
num_to_build = @archive_faq.questions.count
5049
elsif params["num_questions"]
51-
notice += ts("Set up %{num} questions. ", num: num_to_build)
50+
notice = ts("Set up %{num} questions. ", num: num_to_build)
5251
end
5352
num_existing = @archive_faq.questions.count
5453
num_existing.upto(num_to_build-1) do

app/controllers/fandoms_controller.rb

Lines changed: 8 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -33,24 +33,15 @@ def show
3333
end
3434

3535
def unassigned
36-
join_string = "LEFT JOIN wrangling_assignments
37-
ON (wrangling_assignments.fandom_id = tags.id)
38-
LEFT JOIN users
39-
ON (users.id = wrangling_assignments.user_id)"
40-
conditions = "canonical = 1 AND users.id IS NULL"
41-
unless params[:media_id].blank?
36+
@fandoms = Fandom.joins("LEFT JOIN wrangling_assignments ON (wrangling_assignments.fandom_id = tags.id)
37+
LEFT JOIN users ON (users.id = wrangling_assignments.user_id)").where(canonical: true, users: { id: nil })
38+
39+
if params[:media_id].present?
4240
@media = Media.find_by_name(params[:media_id])
43-
if @media
44-
join_string << " INNER JOIN common_taggings
45-
ON (tags.id = common_taggings.common_tag_id)"
46-
conditions << " AND common_taggings.filterable_id = #{@media.id}
47-
AND common_taggings.filterable_type = 'Tag'"
48-
end
41+
@fandoms = @fandoms.joins(:common_taggings).where(common_taggings: { filterable: @media }) if @media
4942
end
50-
@fandoms = Fandom.joins(join_string).
51-
where(conditions).
52-
order(params[:sort] == 'count' ? "count DESC" : "sortable_name ASC").
53-
with_count.
54-
paginate(page: params[:page], per_page: 250)
43+
44+
order = params[:sort] == "count" ? "count DESC" : "sortable_name ASC"
45+
@fandoms = @fandoms.order(order).with_count.paginate(page: params[:page], per_page: 250)
5546
end
5647
end

app/controllers/tag_wranglers_controller.rb

Lines changed: 14 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -9,35 +9,27 @@ def index
99
authorize :wrangling, :full_access? if logged_in_as_admin?
1010

1111
@wranglers = Role.find_by(name: "tag_wrangler").users.alphabetical
12-
conditions = ["canonical = 1"]
13-
joins = "LEFT JOIN wrangling_assignments ON (wrangling_assignments.fandom_id = tags.id)
14-
LEFT JOIN users ON (users.id = wrangling_assignments.user_id)"
15-
unless params[:fandom_string].blank?
16-
conditions.first << " AND name LIKE ?"
17-
conditions << params[:fandom_string] + "%"
18-
end
19-
unless params[:wrangler_id].blank?
12+
13+
@assignments = Fandom.in_use.joins("LEFT JOIN wrangling_assignments ON (wrangling_assignments.fandom_id = tags.id)
14+
LEFT JOIN users ON (users.id = wrangling_assignments.user_id)").where(canonical: true)
15+
16+
@assignments = @assignments.where("name LIKE ?", "#{Fandom.sanitize_sql_like(params[:fandom_string])}%") if params[:fandom_string].present?
17+
18+
if params[:wrangler_id].present?
2019
if params[:wrangler_id] == "No Wrangler"
21-
conditions.first << " AND users.id IS NULL"
20+
@assignments = @assignments.where(users: { id: nil })
2221
else
2322
@wrangler = User.find_by(login: params[:wrangler_id])
24-
if @wrangler
25-
conditions.first << " AND users.id = #{@wrangler.id}"
26-
end
23+
@assignments = @assignments.where(users: { id: @wrangler.id }) if @wrangler
2724
end
2825
end
29-
unless params[:media_id].blank?
26+
27+
if params[:media_id].present?
3028
@media = Media.find_by_name(params[:media_id])
31-
if @media
32-
joins << " INNER JOIN common_taggings ON (tags.id = common_taggings.common_tag_id)"
33-
conditions.first << " AND common_taggings.filterable_id = #{@media.id} AND common_taggings.filterable_type = 'Tag'"
34-
end
29+
@assignments = @assignments.joins(:common_taggings).where(common_taggings: { filterable: @media }) if @media
3530
end
36-
@assignments = Fandom.in_use.joins(joins)
37-
.select('tags.*, users.login AS wrangler')
38-
.where(conditions)
39-
.order(:name)
40-
.paginate(page: params[:page], per_page: 50)
31+
32+
@assignments = @assignments.select("tags.*, users.login AS wrangler").order(:name).paginate(page: params[:page], per_page: 50)
4133
end
4234

4335
def show

app/helpers/tags_helper.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ def remove_tag_association_label(tag)
8888

8989
# Adds the "tag" classname to links (for tag links)
9090
def link_to_with_tag_class(path, text, options)
91-
options[:class] ? options[:class] << " tag" : options[:class] = "tag"
91+
options[:class] ? options[:class].dup << " tag" : options[:class] = "tag"
9292
link_to text, path, options
9393
end
9494

spec/models/story_parser_spec.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ class StoryParser
201201

202202
it "does NOT convert raw anchor links to absolute links" do
203203
location = "http://external_site"
204-
story_in = "<html><body><p><a href=#local>local href</p></body></html>"
204+
story_in = +"<html><body><p><a href=#local>local href</p></body></html>"
205205
result = @sp.parse_common(story_in, location)
206206
expect(result[:chapter_attributes][:content]).not_to include(location)
207207
expect(result[:chapter_attributes][:content]).to include("#local")

spec/models/tag_spec.rb

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -295,14 +295,14 @@ def expect_tag_update_flag_in_redis_to_be(flag)
295295
tag_character = FactoryBot.create(:character, canonical: true, name: "kirk")
296296
tag_fandom = FactoryBot.create(:fandom, name: "Star Trek", canonical: true)
297297
tag_fandom.add_to_autocomplete
298-
results = Tag.autocomplete_fandom_lookup(term: "ki", fandom: "Star Trek")
298+
results = Tag.autocomplete_fandom_lookup(term: +"ki", fandom: "Star Trek")
299299
expect(results.include?("#{tag_character.id}: #{tag_character.name}")).to be_truthy
300300
expect(results.include?("brave_sire_robin")).to be_falsey
301301
end
302302

303303
it "old tag maker still works" do
304-
tag_adult = Rating.create_canonical("adult", true)
305-
tag_normal = ArchiveWarning.create_canonical("other")
304+
tag_adult = Rating.create_canonical(+"adult", true)
305+
tag_normal = ArchiveWarning.create_canonical(+"other")
306306
expect(tag_adult.name).to eq("adult")
307307
expect(tag_normal.name).to eq("other")
308308
expect(tag_adult.adult).to be_truthy

0 commit comments

Comments
 (0)