Redmine

I've attached a patch that takes a first step towards decoupling Journal from Issue .
it introduces a new acts_as_journalized act which allows journalized models to provide their own Journal-related behavior.

Issue now registers itself using:

acts_as_journalized activity_type: 'issues'

The activity provider registration is optional and journalized models can participate in the journaling infrastructure without exposing activity events.

The patch also removes the remaining assumption that a Journal is always associated with an Issue.
In particular, the temporary belongs_to :issue association used for eager loading is no longer required and Journal relies on the polymorphic journalized association instead.

Visibility, activity and permission handling are delegated to the journalized model through the new API.
This includes model-specific joins and scopes used by Journal.visible , allowing each journalized model to define its own visibility requirements.

The patch focuses on behaviors currently implemented directly in Journal, including:

• activity provider registration and activity-specific scopes
• event title, event type and event URL generation
• journal visibility and private note visibility handling
• notification delivery
• watcher processing
• relation visibility checks
• editability checks
• project lookup and journalized record visibility

The current behavior for issues is preserved.

I intentionally did not try to generalize routes, controllers or other Issue-specific UI flows at this stage.
The goal of this patch is solely to decouple Journal from Issue and provide the necessary extension points for future journalized models.

I'm interested in feedback on whether this direction makes sense and could serve as a foundation for further Journal decoupling work.

I've attached a revised version of the patch together with a benchmark, after reviewing and testing the first version on 7.0-stable.

0002-journal-decoupling-revised.patch replaces 0001-journal-decoupling.patch (complete diff against 7.0-stable).

Problems found in the first version¶

• Journal.visible combined the scopes of the journalized types with #or. ActiveRecord merges the joins of all #or branches, so the INNER JOIN issues was also applied to the journals of any other journalized type (a journal of another type was only visible if an issue with the same id existed).
• The defaults were permissive: a journalized type without overrides exposed all records and all private notes, also in notifications.
• IssueQuery#journals no longer joined issue_statuses (missing FROM-clause entry for table "issue_statuses" when sorting by status).
• Journalized types were registered when their class was loaded, so Journal.visible failed (nil.each) in development/test if Issue was not loaded yet.
• The entry title of the journals Atom feed changed.

Changes¶

• Visibility: when the journals are restricted to a single type (always the case in core: Issue, activity scope, issue query, JournalsController), Journal.visible joins the journalized table and uses the same conditions as before (journal_visible_join_condition). Only when journals of several types are queried, each type contributes a self-contained journalized_type = 'X' AND EXISTS (...) condition and the conditions are combined with OR.
• Configuration: acts_as_journalized accepts options, so most models need no overrides:
  

  acts_as_journalized :activity_type => 'issues',
                      :attributes => %w(subject description),  # unless the model defines journalized_attribute_names
                      :permissions => {
                        :edit_notes => :edit_issue_notes,
                        :edit_own_notes => :edit_own_issue_notes,
                        :add_watchers => :add_issue_watchers
                        # :view_private_notes and :set_notes_private default to the existing permissions
                      }
  

• Secure defaults: records are visible through the model's visible scope/visible? method, private notes are filtered with the :view_private_notes permission, a permission that is not configured is never granted. Every journal_* method can still be overridden.
• Registry: journalized types are registered by class name (Redmine::Acts::Journalized.register 'Issue' in Redmine::Preparation), so it works before the classes are loaded and after code reloading. Plugins register their models the same way.
• Journal.joins_journalized(klass) joins the journalized table and its project (used by the activity, Issue.load_visible_last_* and IssueQuery#journals).
• Journal handles a missing journalized record and journalized models without custom fields.
• Journal#issue is kept as a deprecated reader for plugins. joins(:issue)/preload(:issue) are not available anymore, Journal.joins_journalized(Issue) and preload(:journalized) can be used instead.

Example: journalized News¶

example-journalized-news.patch (on top of 0002) is not meant to be committed. It only shows what another model needs: News journalizes changes of title, summary, description (shown as an inline diff, since JournalsController#diff is still issue specific) and attachments, configures manage_news for editing notes, its journals are activity events of the existing news type and the news page shows the history. Visibility and private notes work without overrides.

Benchmark¶

journal-benchmark-script.patch adds extra/benchmarks/bench_journals.rb. It creates 20 projects (a quarter private), 20,000 issues (10% private) and 200,000 journals (10% private notes, status changes), keeps the data so that both variants run against the same data and statistics, and reports the median of each benchmark and a checksum of the result.

git checkout 7.0-stable && bin/rails r extra/benchmarks/bench_journals.rb
git checkout <patch>    && bin/rails r extra/benchmarks/bench_journals.rb
CLEANUP=1 bin/rails r extra/benchmarks/bench_journals.rb

It covers the queries touched by the patch: Journal.visible (count, latest 25, find), activity (project, all projects 30 days, last_by_project), IssueQuery#journals and Issue.load_visible_last_notes/load_visible_last_updated_by, each for an admin, a member with different roles and the anonymous user.

Results on GitHub Actions, median of 3 alternating runs of 7 iterations each on the same runner, ratio patch / 7.0-stable (< 1 is faster). Both variants returned the same results in all 72 cases.

Benchmark	User	PostgreSQL 14	MySQL 8.4	SQLite
visible_count	admin	1.13	1.07	1.05
visible_count	member	1.12	1.06	1.05
visible_count	anonymous	1.07	1.05	1.83
visible_latest_25	admin	0.97	1.08	0.92
visible_latest_25	member	1.04	1.06	0.92
visible_latest_25	anonymous	0.96	1.04	0.90
visible_find	admin	0.98	0.76	0.79
visible_find	member	1.01	0.90	0.95
visible_find	anonymous	0.96	0.89	0.90
activity_project	admin	0.97	0.99	0.81
activity_project	member	0.97	0.99	0.99
activity_project	anonymous	0.98	0.99	0.98
activity_all_30d	admin	0.99	0.98	0.99
activity_all_30d	member	1.01	0.98	0.96
activity_all_30d	anonymous	1.01	0.99	0.98
activity_last_by_project	admin	1.01	0.99	1.00
activity_last_by_project	member	1.02	0.99	1.00
activity_last_by_project	anonymous	0.94	0.99	1.01
issue_query_journals	admin	1.02	1.05	1.08
issue_query_journals	member	1.02	1.05	1.05
issue_query_journals	anonymous	1.03	1.04	1.07
issue_last_notes_25	admin	0.94	0.89	0.85
issue_last_notes_25	member	0.94	0.95	0.89
issue_last_notes_25	anonymous	0.96	0.93	0.95

Timings in msTimings in ms

Everything except visible_count is within ±10%, which is the noise between runs. visible_count is 5-13% slower: it counts all visible journals and the patch adds journals.journalized_type = 'Issue' to the condition (I measured the condition alone at about 5-10% on PostgreSQL). This condition is required as soon as journals of other types exist; the previous query only worked because all journals were issue journals. On SQLite, the anonymous count takes 57 ms instead of 31 ms. Counting all visible journals is not used in the UI as far as I can see.

The first version of the revision, with EXISTS conditions only, was up to 5-10 times slower for some queries (latest journals for a non admin user, issue query journals). That is the reason for joining the journalized table when a single type is queried.

Feedback welcome, especially on the API (acts_as_journalized options and the journal_* methods) and on the trade-off between the join and the EXISTS conditions.