avoid sending journal aggregated event if journal was updated

Journals might get updated by a second change done to the journable within the aggregation time limit
carried out by the same user.

In that case, the journal should not be considered `completed`.

app/workers/journals/completed_job.rb

@@ -34,7 +34,7 @@ class Journals::CompletedJob < ApplicationJob
       return unless supported?(journal)
 
       set(wait_until: delivery_time)
-        .perform_later(journal.id, send_mails)
+        .perform_later(journal.id, journal.updated_at, send_mails)
     end
 
     def aggregated_event(journal)
@@ -61,12 +61,17 @@ class Journals::CompletedJob < ApplicationJob
     end
   end
 
-  def perform(journal_id, send_mails)
-    journal = Journal.find_by(id: journal_id)
-
-    # If the WP has been deleted the journal will have been deleted, too.
-    # Or the journal might have been replaced
-    return if journal.nil?
+  def perform(journal_id, journal_updated_at, send_mails)
+    # If the WP has been deleted, the journal will have been deleted, too.
+    # The journal might also have been updated in the meantime. This happens if
+    # the journable is updated a second time by the same user within the aggregation time.
+    # If aggregation happened, then the job scheduled when the journal was updated the second time
+    # will take care of notifying later.
+    # If another user were to update the journable even within aggregation time,
+    # the journal would not be altered. It is thus safe to consider the journal
+    # final.
+    journal = Journal.find_by(id: journal_id, updated_at: journal_updated_at)
+    return unless journal
 
     notify_journal_complete(journal, send_mails)
   end

spec/models/work_package/openproject_notifications_spec.rb

@@ -61,23 +61,23 @@ describe WorkPackage, type: :model, with_settings: { journal_aggregation_time_mi
       end
 
       it "are triggered" do
-        expect(journal_ids).to include(work_package.journals.last.id)
+        expect(journal_ids).to match [work_package.journals.last.id]
       end
     end
 
     describe 'when after update' do
       before do
         work_package
-
+        perform_enqueued_jobs
         journal_ids.clear
 
-        work_package.update subject: 'the wind of change'
+        work_package.update(subject: 'the wind of change')
 
         perform_enqueued_jobs
       end
 
       it "are triggered" do
-        expect(journal_ids).to include(work_package.journals.last.id)
+        expect(journal_ids).to match [work_package.journals.last.id]
       end
     end
   end

spec/workers/journals/completed_job_spec.rb

@@ -32,20 +32,7 @@ describe Journals::CompletedJob, type: :model do
   let(:send_mail) { true }
 
   let(:journal) do
-    build_stubbed(:journal, journable:).tap do |j|
-      allow(Journal)
-        .to receive(:find)
-              .with(j.id.to_s)
-              .and_return(j)
-      allow(Journal)
-        .to receive(:find_by)
-              .with(id: j.id)
-              .and_return(j)
-      allow(Journal)
-        .to receive(:exists?)
-              .with(id: j.id)
-              .and_return(true)
-    end
+    build_stubbed(:journal, journable:)
   end
 
   describe '.schedule' do
@@ -63,6 +50,7 @@ describe Journals::CompletedJob, type: :model do
           .to have_enqueued_job(described_class)
                 .at(Setting.journal_aggregation_time_minutes.to_i.minutes.from_now)
                 .with(journal.id,
+                      journal.updated_at,
                       send_mail)
       end
     end
@@ -94,7 +82,16 @@ describe Journals::CompletedJob, type: :model do
   end
 
   describe '#perform' do
-    subject { described_class.new.perform(journal.id, send_mail) }
+    subject { described_class.new.perform(journal.id, journal.updated_at, send_mail) }
+
+    let(:find_by_journal) { journal }
+
+    before do
+      allow(Journal)
+        .to receive(:find_by)
+              .with(id: journal.id, updated_at: journal.updated_at)
+              .and_return(find_by_journal)
+    end
 
     shared_examples_for 'sends a notification' do |event|
       it 'sends a notification' do
@@ -132,15 +129,9 @@ describe Journals::CompletedJob, type: :model do
                       OpenProject::Events::AGGREGATED_NEWS_JOURNAL_READY
     end
 
-    context 'with a non non-existant journal' do
+    context 'with a non non-existent journal (either because the journable was deleted or the journal updated)' do
       let(:journable) { build_stubbed(:work_package) }
-
-      before do
-        allow(Journal)
-          .to receive(:find_by)
-                .with(id: journal.id)
-                .and_return(nil)
-      end
+      let(:find_by_journal) { nil }
 
       it 'sends no notification' do
         allow(OpenProject::Notifications)