Skip to content

Commit ab9aa69

Browse files
JasonBarnabeJasonBarnabe
committed
Ensure mentions are not duplicated on new script versions #884 
1 parent e4aaf2e commit ab9aa69

File tree

3 files changed

+95
-14
lines changed

3 files changed

+95
-14
lines changed

‎app/controllers/reports_controller.rb

+7-7
Original file line numberDiff line numberDiff line change
@@ -37,21 +37,21 @@ def create
3737

3838
def index
3939
scope = Report
40-
.includes(:item)
41-
.order(:created_at)
40+
.includes(:item)
41+
.order(:created_at)
4242
if params[:user_id].present?
4343
scope = scope.where(reporter_id: params[:user_id])
4444
else
4545
@show_separator = true
4646
scope = scope.unresolved
4747
end
4848
report_ids = scope
49-
.sort_by { |r| [r.awaiting_response? ? 1 : 0, r.created_at] }
50-
.reject {|r| r.item.nil? }
51-
.map(&:id)
49+
.sort_by { |r| [r.awaiting_response? ? 1 : 0, r.created_at] }
50+
.reject { |r| r.item.nil? }
51+
.map(&:id)
5252
@reports = Report
53-
.where(id: report_ids)
54-
.includes(:item, :reference_script, :reporter, :rebuttal_by_user)
53+
.where(id: report_ids)
54+
.includes(:item, :reference_script, :reporter, :rebuttal_by_user)
5555
@reports = @reports.order(Arel.sql("FIELD(id, #{report_ids.join(',')})")) if report_ids.any?
5656
@reports = @reports.paginate(page: params[:page], per_page: per_page)
5757
end

‎app/models/script.rb

+15-7
Original file line numberDiff line numberDiff line change
@@ -234,19 +234,27 @@ def apply_from_script_version(script_version)
234234
# Copy additional_info from script versions. Retain syncing info.
235235
original_script_las = localized_attributes_for('additional_info').to_a
236236
# Try to retain the records - search by locale
237-
script_version.localized_attributes_for('additional_info').each do |la|
238-
matching_osla = original_script_las.find { |osla| osla.locale_id == la.locale_id }
237+
script_version.localized_attributes_for('additional_info').each do |sv_la|
238+
matching_osla = original_script_las.find { |osla| osla.locale_id == sv_la.locale_id }
239239
if matching_osla.nil?
240240
# New
241-
build_localized_attribute(la)
241+
build_localized_attribute(sv_la)
242242
else
243-
matching_osla.value_markup = la.value_markup
244-
matching_osla.attribute_value = la.attribute_value
243+
matching_osla.value_markup = sv_la.value_markup
244+
matching_osla.attribute_value = sv_la.attribute_value
245245
# We've found this one, don't search for it any more.
246246
original_script_las.delete(matching_osla)
247-
la.mentions.each do |mention|
248-
matching_osla.mentions.build(user_id: mention.user_id, text: mention.text)
247+
248+
# Add any missing mentions
249+
sv_la.mentions.each do |mention|
250+
matching_osla.mentions.build(user_id: mention.user_id, text: mention.text) if matching_osla.mentions.none? { |matching_mention| matching_mention.user_id == mention.user_id && matching_mention.text == mention.text }
249251
end
252+
253+
# Remove any unneeded mentions
254+
matching_osla.mentions
255+
.reject(&:new_record?)
256+
.reject { |mention| sv_la.mentions.any? { |matching_mention| matching_mention.user_id == mention.user_id && matching_mention.text == mention.text } }
257+
.each(&:mark_for_destruction)
250258
end
251259
end
252260
# Delete any that are gone

‎test/system/scripts/additional_info_test.rb

+73
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,79 @@ class AdditionalInfoTest < ApplicationSystemTestCase
3939
assert_link '@Gordon J. Canada', href: user_path(mentioned_user2, locale: :en)
4040
end
4141

42+
test 'mention retention and deletion' do
43+
user = User.first
44+
login_as(user, scope: :user)
45+
46+
visit new_script_version_url
47+
code = <<~JS
48+
// ==UserScript==
49+
// @name A Test!
50+
// @description Unit test.
51+
// @version 1.1
52+
// @namespace http://greasyfork.local/users/1
53+
// @include *
54+
// ==/UserScript==
55+
var foo = 1;
56+
JS
57+
fill_in 'Code', with: code
58+
fill_in 'Additional info', with: 'Hey @Geoffrey this is for you!'
59+
click_button 'Post script'
60+
assert_selector 'h2', text: 'A Test!'
61+
script = Script.last
62+
script_version = script.script_versions.last
63+
[script, script_version].each do |o|
64+
assert_equal 1, o.localized_attribute_for('additional_info').mentions.count
65+
assert_equal '@Geoffrey', o.localized_attribute_for('additional_info').mentions.first.text
66+
end
67+
68+
# No update
69+
visit new_script_script_version_url(script_id: script.id)
70+
click_button 'Post new version'
71+
assert_selector 'h2', text: 'A Test!'
72+
script.reload
73+
script_version = script.script_versions.last
74+
[script, script_version].each do |o|
75+
assert_equal 1, o.localized_attribute_for('additional_info').mentions.count
76+
assert_equal '@Geoffrey', o.localized_attribute_for('additional_info').mentions.first.text
77+
end
78+
79+
# Update the additional info, but retain the mention
80+
visit new_script_script_version_url(script_id: script.id)
81+
fill_in 'Additional info', with: 'Hey @Geoffrey this is for you!!!!!!'
82+
click_button 'Post new version'
83+
assert_selector 'h2', text: 'A Test!'
84+
script.reload
85+
script_version = script.script_versions.last
86+
[script, script_version].each do |o|
87+
assert_equal 1, o.localized_attribute_for('additional_info').mentions.count
88+
assert_equal '@Geoffrey', o.localized_attribute_for('additional_info').mentions.first.text
89+
end
90+
91+
# Update the additional info, mentioning a different user
92+
visit new_script_script_version_url(script_id: script.id)
93+
fill_in 'Additional info', with: 'Hey @"Gordon J. Canada" this is for you!'
94+
click_button 'Post new version'
95+
assert_selector 'h2', text: 'A Test!'
96+
script.reload
97+
script_version = script.script_versions.last
98+
[script, script_version].each do |o|
99+
assert_equal 1, o.localized_attribute_for('additional_info').mentions.count
100+
assert_equal '@"Gordon J. Canada"', o.localized_attribute_for('additional_info').mentions.first.text
101+
end
102+
103+
# No longer mention anyone
104+
visit new_script_script_version_url(script_id: script.id)
105+
fill_in 'Additional info', with: 'This is for no one'
106+
click_button 'Post new version'
107+
assert_selector 'h2', text: 'A Test!'
108+
script.reload
109+
script_version = script.script_versions.last
110+
[script, script_version].each do |o|
111+
assert_empty o.localized_attribute_for('additional_info').mentions
112+
end
113+
end
114+
42115
test 'changing just additional info reindexes' do
43116
script = Script.find(11)
44117
login_as(script.users.first, scope: :user)

0 commit comments

Comments
 (0)