From f9051bba3eb039820ad6b7977db96344c251d873 Mon Sep 17 00:00:00 2001 From: Jan Sandbrink Date: Tue, 4 Aug 2015 15:52:53 +0200 Subject: [PATCH] display diff of containing aggregated journal ... instead of displaying the diff for the requested journal only - this should provide the best forward compatibility of links - even when aggregation timeframes are changed intermittently --- app/controllers/journals_controller.rb | 8 ++++--- app/models/journal/aggregated_journal.rb | 6 ++++++ .../models/journal/aggregated_journal_spec.rb | 21 +++++++++++++++++++ 3 files changed, 32 insertions(+), 3 deletions(-) diff --git a/app/controllers/journals_controller.rb b/app/controllers/journals_controller.rb index 271d0b220b..064f9438ca 100644 --- a/app/controllers/journals_controller.rb +++ b/app/controllers/journals_controller.rb @@ -88,13 +88,15 @@ class JournalsController < ApplicationController end def diff + journal = Journal::AggregatedJournal.for_journal(@journal) + if valid_diff? field = params[:field].parameterize.underscore.to_sym - from = @journal.details[field][0] - to = @journal.details[field][1] + from = journal.details[field][0] + to = journal.details[field][1] @diff = Redmine::Helpers::Diff.new(to, from) - @journable = @journal.journable + @journable = journal.journable respond_to do |format| format.html {} format.js { render partial: 'diff', locals: { diff: @diff } } diff --git a/app/models/journal/aggregated_journal.rb b/app/models/journal/aggregated_journal.rb index 3e0bbfcc2a..48ce08e88a 100644 --- a/app/models/journal/aggregated_journal.rb +++ b/app/models/journal/aggregated_journal.rb @@ -41,6 +41,12 @@ # be dropped class Journal::AggregatedJournal class << self + # Returns the aggregated journal that contains the specified (vanilla) journal. + def for_journal(vanilla_journal) + Journal::AggregatedJournal.aggregated_journals(journable: vanilla_journal.journable) + .detect { |journal| journal.version >= vanilla_journal.version } + end + def with_notes_id(notes_id) raw_journal = query_aggregated_journals .where("#{table_name}.id = ?", notes_id) diff --git a/spec/models/journal/aggregated_journal_spec.rb b/spec/models/journal/aggregated_journal_spec.rb index fed09fb969..41d4e57e47 100644 --- a/spec/models/journal/aggregated_journal_spec.rb +++ b/spec/models/journal/aggregated_journal_spec.rb @@ -120,6 +120,14 @@ describe Journal::AggregatedJournal, type: :model do expect(subject.first.successor).to be_nil end + it 'returns the single journal for both original journals' do + expect(described_class.for_journal work_package.journals.first) + .to be_equivalent_to_journal subject.first + + expect(described_class.for_journal work_package.journals.second) + .to be_equivalent_to_journal subject.first + end + context 'with a comment' do let(:notes) { 'This is why I changed it.' } @@ -153,6 +161,19 @@ describe Journal::AggregatedJournal, type: :model do it 'has the second as successor of the first journal' do expect(subject.first.successor).to be_equivalent_to_journal subject.second end + + it 'returns the same aggregated journal for the first two originals' do + expect(described_class.for_journal work_package.journals.first) + .to be_equivalent_to_journal subject.first + + expect(described_class.for_journal work_package.journals.second) + .to be_equivalent_to_journal subject.first + end + + it 'returns a different aggregated journal for the last original' do + expect(described_class.for_journal work_package.journals.last) + .to be_equivalent_to_journal subject.second + end end context 'adding another change without comment' do