Skip to content

Commit 21cf468

Browse files
Fix deface_simulate_override_in_place to use actual source_element instead of placeholder text for accurate hash calculation
1 parent 58ae141 commit 21cf468

File tree

1 file changed

+62
-16
lines changed

1 file changed

+62
-16
lines changed

test/global_test_helper.rb

Lines changed: 62 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,8 @@ def count_sql_queries
222222
end
223223

224224
# Validates that all Deface overrides matching the given pattern have correct hashes.
225-
# This ensures the overrides still match their target elements in Redmine templates.
225+
# This simulates Deface's runtime behavior by applying overrides in sequence order,
226+
# so it can detect hash conflicts caused by earlier overrides modifying the template.
226227
#
227228
# @param partial_patterns [Array<String>, String] patterns to match against partial paths
228229
# e.g. 'wiki_guide' matches partials like 'wiki/wiki_guide_edit_link'
@@ -238,29 +239,47 @@ def assert_deface_overrides_valid(partial_patterns:)
238239

239240
# rubocop:disable Rails/FindEach -- Deface::Override.all returns a Hash, not ActiveRecord::Relation
240241
Deface::Override.all.each do |virtual_path, overrides_hash|
241-
overrides_hash.each do |name, override|
242-
next unless override.respond_to? :args
243-
next if override.args[:original].blank?
242+
# Collect overrides we care about for this template
243+
relevant_overrides = overrides_hash.select do |_name, override|
244+
next false unless override.respond_to? :args
245+
next false if override.args[:original].blank?
244246

245247
partial = override.args[:partial].to_s
246-
next unless patterns.any? { |pattern| partial.include? pattern }
248+
patterns.any? { |pattern| partial.include? pattern }
249+
end
250+
251+
next if relevant_overrides.empty?
252+
253+
template_path = deface_resolve_template_path virtual_path
254+
next unless template_path && File.exist?(template_path)
255+
256+
# Get ALL overrides for this template, sorted by sequence (like Deface does at runtime)
257+
all_overrides_sorted = overrides_hash.values.sort_by(&:sequence)
247258

248-
template_path = deface_resolve_template_path virtual_path
249-
next unless template_path && File.exist?(template_path)
259+
# Parse template once and reuse the doc object to avoid re-parsing artifacts
260+
source = File.read template_path
261+
doc = Deface::Parser.convert source
250262

251-
source = File.read template_path
252-
doc = Deface::Parser.convert source
263+
# Apply overrides in sequence order, validating our overrides as we go
264+
all_overrides_sorted.each do |override|
253265
elements = doc.css override.selector
254266

255-
if elements.empty?
256-
invalid_overrides << "#{name}: selector '#{override.selector}' finds no elements"
257-
next
267+
# If this is one of our overrides, validate it
268+
if relevant_overrides.value? override
269+
if elements.empty?
270+
invalid_overrides << "#{override.name}: selector '#{override.selector}' finds no elements"
271+
else
272+
actual_hash = Digest::SHA1.hexdigest(elements.first.to_s.gsub(/\s/, ''))
273+
expected_hash = override.args[:original]
274+
275+
if actual_hash != expected_hash
276+
invalid_overrides << "#{override.name}: expected hash '#{expected_hash}', got '#{actual_hash}'"
277+
end
278+
end
258279
end
259280

260-
actual_hash = Digest::SHA1.hexdigest(elements.first.to_s.gsub(/\s/, ''))
261-
expected_hash = override.args[:original]
262-
263-
invalid_overrides << "#{name}: expected hash '#{expected_hash}', got '#{actual_hash}'" unless actual_hash == expected_hash
281+
# Simulate applying this override by modifying the doc in place
282+
deface_simulate_override_in_place doc, override if elements.any?
264283
end
265284
end
266285
# rubocop:enable Rails/FindEach
@@ -287,6 +306,33 @@ def WikiPage.generate!(**options)
287306

288307
private
289308

309+
# Simulates applying a Deface override by modifying the doc in place.
310+
# Uses actual source_element content to match runtime hash calculations.
311+
def deface_simulate_override_in_place(doc, override)
312+
target = doc.css(override.selector).first
313+
return unless target
314+
315+
action = override.action.to_sym
316+
317+
case action
318+
when :insert_bottom
319+
target.add_child override.source_element.to_s
320+
when :insert_top
321+
target.prepend_child override.source_element.to_s
322+
when :insert_after
323+
target.after override.source_element.to_s
324+
when :insert_before
325+
target.before override.source_element.to_s
326+
when :replace, :replace_contents
327+
target.inner_html = override.source_element.to_s
328+
when :remove
329+
target.remove
330+
when :set_attributes, :add_to_attributes, :remove_from_attributes
331+
# Attribute actions don't affect element content/hash, skip
332+
nil
333+
end
334+
end
335+
290336
def deface_resolve_template_path(virtual_path)
291337
possible_paths = [
292338
Rails.root.join('app', 'views', "#{virtual_path}.html.erb"),

0 commit comments

Comments
 (0)