Bug/66552 Fix the activities tab’s :only_changes filtration for lazy loading

[akabiru]

Follows #20733

https://community.openproject.org/wp/66552

Correct the :only_changes filter added in #20733 such that it checks for at least one journal change and NOT presence as it did before. The latter would evaluate to false positives that further materialized the original bug.

Test WP:

• https://qa.openproject-stage.com/projects/cecile-demo-project/work_packages/3647/activity
• https://qa.openproject-edge.com/projects/test-project-cecile/work_packages/10423/activity

The red borders demarcate page nodes. (Click to see highlight code change)

diff --git a/app/components/work_packages/activities_tab/journals/page_component.html.erb b/app/components/work_packages/activities_tab/journals/page_component.html.erb
index cc02170f5fc..621bc2eca2f 100644
--- a/app/components/work_packages/activities_tab/journals/page_component.html.erb
+++ b/app/components/work_packages/activities_tab/journals/page_component.html.erb
@@ -1,6 +1,6 @@
 <%=
   component_wrapper do
-    flex_layout do |flex_container|
+    flex_layout(style: "border: 2px dashed red;") do |flex_container|
       journals.each do |record|
         flex_container.with_row do
           if record.is_a?(Changeset)

Before:

After:

---

Sample resulting query

SELECT *
FROM "journals"
JOIN LATERAL
  (SELECT journals_rank.id,
          ROW_NUMBER() OVER (
                             ORDER BY VERSION ASC) sequence_version
   FROM journals journals_rank
   WHERE journals_rank.journable_id = journals.journable_id
     AND journals_rank.journable_type = journals.journable_type) ranked ON ranked.id = journals.id
WHERE "journals"."journable_id" = 40
  AND "journals"."journable_type" = 'WorkPackage'
  AND "journals"."restricted" = FALSE
  AND (VERSION = 1
       OR (cause IS NOT NULL
           AND cause != '{}')
       OR EXISTS
         (SELECT 1
          FROM journals predecessor
          INNER JOIN work_package_journals pred_data ON predecessor.data_id = pred_data.id
          INNER JOIN work_package_journals curr_data ON journals.data_id = curr_data.id
          WHERE predecessor.journable_id = journals.journable_id
            AND predecessor.journable_type = journals.journable_type
            AND predecessor.version =
              (SELECT MAX(VERSION)
               FROM journals p2
               WHERE p2.journable_id = journals.journable_id
                 AND p2.journable_type = journals.journable_type
                 AND p2.version < journals.version)
            AND (pred_data.type_id IS DISTINCT
                 FROM curr_data.type_id
                 OR pred_data.project_id IS DISTINCT
                 FROM curr_data.project_id
                 OR pred_data.subject IS DISTINCT
                 FROM curr_data.subject
                 OR pred_data.description IS DISTINCT
                 FROM curr_data.description
                 OR pred_data.due_date IS DISTINCT
                 FROM curr_data.due_date
                 OR pred_data.category_id IS DISTINCT
                 FROM curr_data.category_id
                 OR pred_data.status_id IS DISTINCT
                 FROM curr_data.status_id
                 OR pred_data.assigned_to_id IS DISTINCT
                 FROM curr_data.assigned_to_id
                 OR pred_data.priority_id IS DISTINCT
                 FROM curr_data.priority_id
                 OR pred_data.version_id IS DISTINCT
                 FROM curr_data.version_id
                 OR pred_data.author_id IS DISTINCT
                 FROM curr_data.author_id
                 OR pred_data.done_ratio IS DISTINCT
                 FROM curr_data.done_ratio
                 OR pred_data.estimated_hours IS DISTINCT
                 FROM curr_data.estimated_hours
                 OR pred_data.start_date IS DISTINCT
                 FROM curr_data.start_date
                 OR pred_data.parent_id IS DISTINCT
                 FROM curr_data.parent_id
                 OR pred_data.responsible_id IS DISTINCT
                 FROM curr_data.responsible_id
                 OR pred_data.derived_estimated_hours IS DISTINCT
                 FROM curr_data.derived_estimated_hours
                 OR pred_data.schedule_manually IS DISTINCT
                 FROM curr_data.schedule_manually
                 OR pred_data.duration IS DISTINCT
                 FROM curr_data.duration
                 OR pred_data.ignore_non_working_days IS DISTINCT
                 FROM curr_data.ignore_non_working_days
                 OR pred_data.derived_remaining_hours IS DISTINCT
                 FROM curr_data.derived_remaining_hours
                 OR pred_data.derived_done_ratio IS DISTINCT
                 FROM curr_data.derived_done_ratio
                 OR pred_data.story_points IS DISTINCT
                 FROM curr_data.story_points
                 OR pred_data.remaining_hours IS DISTINCT
                 FROM curr_data.remaining_hours
                 OR pred_data.budget_id IS DISTINCT
                 FROM curr_data.budget_id
                 OR pred_data.project_phase_definition_id IS DISTINCT
                 FROM curr_data.project_phase_definition_id))
       OR EXISTS
         (SELECT 1
          FROM attachable_journals curr
          LEFT JOIN journals predecessor ON predecessor.journable_id = journals.journable_id
          AND predecessor.journable_type = journals.journable_type
          AND predecessor.version =
            (SELECT MAX(VERSION)
             FROM journals p2
             WHERE p2.journable_id = journals.journable_id
               AND p2.journable_type = journals.journable_type
               AND p2.version < journals.version)
          LEFT JOIN attachable_journals pred ON pred.journal_id = predecessor.id
          AND pred.attachment_id = curr.attachment_id
          WHERE curr.journal_id = journals.id
            AND (pred.id IS NULL
                 OR (pred.filename IS DISTINCT
                     FROM curr.filename))
          UNION ALL SELECT 1
          FROM journals predecessor
          INNER JOIN attachable_journals pred ON pred.journal_id = predecessor.id
          LEFT JOIN attachable_journals curr ON curr.journal_id = journals.id
          AND curr.attachment_id = pred.attachment_id
          WHERE predecessor.journable_id = journals.journable_id
            AND predecessor.journable_type = journals.journable_type
            AND predecessor.version =
              (SELECT MAX(VERSION)
               FROM journals p2
               WHERE p2.journable_id = journals.journable_id
                 AND p2.journable_type = journals.journable_type
                 AND p2.version < journals.version)
            AND curr.id IS NULL)
       OR EXISTS
         (SELECT 1
          FROM customizable_journals curr
          LEFT JOIN journals predecessor ON predecessor.journable_id = journals.journable_id
          AND predecessor.journable_type = journals.journable_type
          AND predecessor.version =
            (SELECT MAX(VERSION)
             FROM journals p2
             WHERE p2.journable_id = journals.journable_id
               AND p2.journable_type = journals.journable_type
               AND p2.version < journals.version)
          LEFT JOIN customizable_journals pred ON pred.journal_id = predecessor.id
          AND pred.custom_field_id = curr.custom_field_id
          WHERE curr.journal_id = journals.id
            AND (pred.id IS NULL
                 OR (pred.value IS DISTINCT
                     FROM curr.value))
          UNION ALL SELECT 1
          FROM journals predecessor
          INNER JOIN customizable_journals pred ON pred.journal_id = predecessor.id
          LEFT JOIN customizable_journals curr ON curr.journal_id = journals.id
          AND curr.custom_field_id = pred.custom_field_id
          WHERE predecessor.journable_id = journals.journable_id
            AND predecessor.journable_type = journals.journable_type
            AND predecessor.version =
              (SELECT MAX(VERSION)
               FROM journals p2
               WHERE p2.journable_id = journals.journable_id
                 AND p2.journable_type = journals.journable_type
                 AND p2.version < journals.version)
            AND curr.id IS NULL)
       OR EXISTS
         (SELECT 1
          FROM storages_file_links_journals curr
          LEFT JOIN journals predecessor ON predecessor.journable_id = journals.journable_id
          AND predecessor.journable_type = journals.journable_type
          AND predecessor.version =
            (SELECT MAX(VERSION)
             FROM journals p2
             WHERE p2.journable_id = journals.journable_id
               AND p2.journable_type = journals.journable_type
               AND p2.version < journals.version)
          LEFT JOIN storages_file_links_journals pred ON pred.journal_id = predecessor.id
          AND pred.file_link_id = curr.file_link_id
          WHERE curr.journal_id = journals.id
            AND (pred.id IS NULL
                 OR (pred.link_name IS DISTINCT
                     FROM curr.link_name
                     OR pred.storage_name IS DISTINCT
                     FROM curr.storage_name))
          UNION ALL SELECT 1
          FROM journals predecessor
          INNER JOIN storages_file_links_journals pred ON pred.journal_id = predecessor.id
          LEFT JOIN storages_file_links_journals curr ON curr.journal_id = journals.id
          AND curr.file_link_id = pred.file_link_id
          WHERE predecessor.journable_id = journals.journable_id
            AND predecessor.journable_type = journals.journable_type
            AND predecessor.version =
              (SELECT MAX(VERSION)
               FROM journals p2
               WHERE p2.journable_id = journals.journable_id
                 AND p2.journable_type = journals.journable_type
                 AND p2.version < journals.version)
            AND curr.id IS NULL))
ORDER BY "journals"."version" DESC

Performance Analysis Report ✨ (AI Assisted)

Benchmark Results Summary (Work Package #10423, 684 journals)

Scenario	Average Time	Min	Max	Notes
filter=:all	335.68ms	237.65ms	471.76ms	Baseline - loads all journals
filter=:only_comments	247.12ms	233.16ms	255.78ms	26% faster than :all
filter=:only_changes	19.48ms	17.14ms	24.38ms	94% faster than :all! ✅
Anchor navigation	256.53ms	232.08ms	312.85ms	Similar to :only_comments

Benchmark script

# frozen_string_literal: true

# Benchmark script for WorkPackages::ActivitiesTab::Paginator
# Run with: bundle exec rails runner tmp/paginator_benchmark.rb

require 'benchmark'

wp_id = 10423
work_package = WorkPackage.find(wp_id)

puts "=" * 80
puts "Paginator Performance Benchmark"
puts "=" * 80
puts "Work Package: ##{work_package.id}"
puts "Journals count: #{work_package.journals.count}"
puts "=" * 80
puts

# Helper to run and report
def benchmark_scenario(name, &block)
  puts "\n#{name}"
  puts "-" * 80

  result = nil
  times = []

  # Warm up
  block.call

  # Run 5 times
  5.times do
    time = Benchmark.realtime { result = block.call }
    times << time
  end

  avg_time = times.sum / times.size
  min_time = times.min
  max_time = times.max

  puts "Average: #{(avg_time * 1000).round(2)}ms"
  puts "Min:     #{(min_time * 1000).round(2)}ms"
  puts "Max:     #{(max_time * 1000).round(2)}ms"

  if result.is_a?(Array) && result[1].is_a?(Array)
    puts "Results: #{result[1].size} records"
  end

  result
end

# Scenario 1: Filter :all (baseline)
benchmark_scenario("Scenario 1: filter=:all (baseline)") do
  WorkPackages::ActivitiesTab::Paginator.paginate(work_package, filter: :all, page: 1)
end

# Scenario 2: Filter :only_comments
benchmark_scenario("Scenario 2: filter=:only_comments") do
  WorkPackages::ActivitiesTab::Paginator.paginate(work_package, filter: :only_comments, page: 1)
end

# Scenario 3: Filter :only_changes (with new heuristic)
benchmark_scenario("Scenario 3: filter=:only_changes (with change detection)") do
  WorkPackages::ActivitiesTab::Paginator.paginate(work_package, filter: :only_changes, page: 1)
end

# Scenario 4: Anchor navigation (find_index performance)
benchmark_scenario("Scenario 4: Anchor navigation to middle journal") do
  middle_journal = work_package.journals.order(:version).offset(work_package.journals.count / 2).first
  WorkPackages::ActivitiesTab::Paginator.paginate(work_package, anchor: "comment-#{middle_journal.id}", page: 1)
end

# Deep dive: SQL query analysis
puts "\n" + "=" * 80
puts "SQL Query Analysis (with EXPLAIN)"
puts "=" * 80

ActiveRecord::Base.logger = Logger.new($stdout)
ActiveRecord::Base.logger.level = Logger::DEBUG

puts "\nQuery for :only_changes filter:"
puts "-" * 80
WorkPackages::ActivitiesTab::Paginator.paginate(work_package, filter: :only_changes, page: 1)

ActiveRecord::Base.logger = nil

puts "\n" + "=" * 80
puts "Benchmark Complete"
puts "=" * 80

[github-actions]

Caution

The provided work package version does not match the core version

Details:

• Work package URL: https://community.openproject.org/wp/66552
• Work package version: 17.0.0
• Core version: 16.6.0

Please make sure that:

• The work package version OR your pull request target branch is correct

[cbliard]

Nice one 👏

I reviewed the code and sql queries. It looks solid, even if I'm sure I don't get all the subtleties of it 😅
I'm amazed that it's possible to create such nested and complicated SQL queries and it's still fast to process it.

[cbliard]

Given the entire UNION query is inside of an EXISTS (...) clause, deduplication is not necessary as EXISTS short-circuits on the first match. Hence, no need to have the db engine waste cycles checking for duplicates.

That is the kind of subtleties that I missed. Nice! I was actually wondering why it was not using UNION instead of OR. This gives the answer.

[brunopagno]

Let's go!! 🚢🚢🚢

[akabiru]

Nice one 👏

I reviewed the code and sql queries. It looks solid, even if I'm sure I don't get all the subtleties of it 😅 I'm amazed that it's possible to create such nested and complicated SQL queries and it's still fast to process it.

Thanks! I reckon that using subqueries and the EXISTS (…) clause with the SELECT 1 flags allows for faster speeds and optimization of subqueries by PostgreSQL. I’ll still keep an eye on it. I admit the complexity is seemingly overwhelming, but I hope that the encapsulation reduces the perceived nature of it. 😅