Merge branch 'ticket/unstable/808-issue-description-diff' into unstable

app/controllers/journals_controller.rb

@@ -12,6 +12,8 @@
 # See doc/COPYRIGHT.rdoc for more details.
 #++
 
+require 'diff'
+
 class JournalsController < ApplicationController
   before_filter :find_journal, :only => [:edit, :diff]
   before_filter :find_issue, :only => [:new]
@@ -84,6 +86,22 @@ class JournalsController < ApplicationController
     end
   end
 
+  def diff
+    if valid_field?(params[:field])
+      from = @journal.changes[params[:field]][0]
+      to = @journal.changes[params[:field]][1]
+
+      @diff = Redmine::Helpers::Diff.new(to, from)
+      @issue = @journal.journaled
+      respond_to do |format|
+        format.html { }
+        format.js { render :layout => false }
+      end
+    else
+      render_404
+    end
+  end
+  
   private
 
   def find_journal
@@ -100,4 +118,9 @@ class JournalsController < ApplicationController
   rescue ActiveRecord::RecordNotFound
     render_404
   end
+
+  # Is this a valid field for diff'ing?
+  def valid_field?(field)
+    field.to_s.strip == "description"
+  end
 end

app/views/journals/diff.html.erb

@@ -0,0 +1,9 @@
+<p><%= authoring @journal.created_at, @journal.user, :label => :label_updated_time_by %></p>
+
+<div class="text-diff">
+<%= simple_format_without_paragraph @diff.to_html %>
+</div>
+
+<p><%= link_to(l(:button_back), issue_path(@issue)) %></p>
+
+<% html_title "#{@issue.tracker.name} ##{@issue.id}: #{@issue.subject}" %>

app/views/layouts/base.rhtml

@@ -145,6 +145,7 @@
   </div>
 
   <div id="ajax-indicator" style="display:none;"><span><%= l(:label_loading) %></span></div>
+  <div id="dialog-window" style="display:none;"></div>
 
 </div>
 <%= call_hook :view_layouts_base_body_bottom %>

config/routes.rb

@@ -95,7 +95,8 @@ ActionController::Routing::Routes.draw do |map|
   map.bulk_update_issue 'issues/bulk_edit', :controller => 'issues', :action => 'bulk_update', :conditions => { :method => :post }
   map.quoted_issue '/issues/:id/quoted', :controller => 'journals', :action => 'new', :id => /\d+/, :conditions => { :method => :post }
   map.connect '/issues/:id/destroy', :controller => 'issues', :action => 'destroy', :conditions => { :method => :post } # legacy
-
+  map.journal_diff '/journals/:id/diff/:field', :controller => 'journals', :action => 'diff', :conditions => { :method => :get }
+  
   map.resource :gantt, :path_prefix => '/issues', :controller => 'gantts', :only => [:show, :update]
   map.resource :gantt, :path_prefix => '/projects/:project_id/issues', :controller => 'gantts', :only => [:show, :update]
   map.resource :calendar, :path_prefix => '/issues', :controller => 'calendars', :only => [:show, :update]

public/javascripts/application.js

@@ -465,6 +465,15 @@ jQuery.viewportHeight = function() {
         document.body.clientHeight;
 };
 
+// Automatically use format.js for jQuery Ajax
+jQuery.ajaxSetup({
+  'beforeSend': function(xhr) {xhr.setRequestHeader("Accept", "text/javascript")}
+})
+
+// Show/hide the ajax indicators
+jQuery("#ajax-indicator").ajaxStart(function(){ jQuery(this).show().css('z-index', '9999');  });
+jQuery("#ajax-indicator").ajaxStop(function(){ jQuery(this).hide();  });
+
 /* TODO: integrate with existing code and/or refactor */
 jQuery(document).ready(function($) {
 
@@ -608,4 +617,31 @@ jQuery(document).ready(function($) {
   $('#nav-login-content').click(function(event){
     event.stopPropagation();
   });
+
+  $('.lightbox-ajax').click(function(event) {
+    $('#dialog-window').
+      html('').
+      load(this.href, function() {
+        // Set width to the content width
+        var width = $('#content').width();
+        $(this).dialog("option", "width", width).dialog('open');
+      });
+
+    event.preventDefault();
+    return false;
+
+  });
+
+  // Configures the default dialog window
+  function setUpDialogWindow() {
+    $('#dialog-window').
+      dialog({
+        autoOpen: false,
+        minWidth: 400,
+        width: 800
+      });
+
+  }
+
+  setUpDialogWindow();
 });

test/functional/journals_controller_test.rb

@@ -81,4 +81,68 @@ class JournalsControllerTest < ActionController::TestCase
     assert_select_rjs :show, "update"
   end
 
+  context "#diff" do
+    setup do
+      @request.session[:user_id] = 1
+      @issue = Issue.find(6)
+      @previous_description = @issue.description
+      @new_description = "New description"
+      
+      assert_difference("Journal.count") do
+        @issue.description = @new_description
+        assert @issue.save
+      end
+      @last_journal = @issue.last_journal
+    end
+    
+    context "without a valid journal" do
+      should "return a 404" do
+        get :diff, :id => '0'
+        assert_response :not_found
+      end
+    end
+
+
+    context "with no field parameter" do
+      should "return a 404" do
+        get :diff, :id => @last_journal.id
+        assert_response :not_found
+      end
+    end
+
+    context "for an invalid field" do
+      should "return a 404" do
+        get :diff, :id => @last_journal.id, :field => 'id'
+        assert_response :not_found
+      end
+    end
+
+    context "without permission to view_issues" do
+      should "return a 403" do
+        @request.session[:user_id] = 7
+        get :diff, :id => @last_journal.id, :field => 'description'
+
+        assert_response :forbidden
+      end
+      
+    end
+
+    context "with permission to view_issues" do
+      setup do
+        get :diff, :id => @last_journal.id, :field => 'description'
+      end
+      
+      should "create a diff" do
+        assert_not_nil assigns(:diff)
+        assert assigns(:diff).is_a?(Redmine::Helpers::Diff)
+      end
+
+      should "render an inline diff" do
+        assert_select "#content .text-diff"
+      end
+      
+    end
+      
+  end
+  
 end

test/integration/journals_test.rb

@@ -0,0 +1,47 @@
+#-- encoding: UTF-8
+#-- copyright
+# ChiliProject is a project management system.
+#
+# Copyright (C) 2010-2011 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# See doc/COPYRIGHT.rdoc for more details.
+#++
+require File.expand_path('../../test_helper', __FILE__)
+require 'capybara/rails'
+
+class JournalsTest < ActionController::IntegrationTest
+  fixtures :all
+
+  include IntegrationTestHelpers::CapybaraHelpers
+  include Capybara::DSL
+  
+  test "showing issue description changes as a diff" do
+    # Description change
+    @issue = Issue.find(1)
+    @issue.recreate_initial_journal!
+    @issue.reload
+    assert_difference("Journal.count") do
+      @issue.journal_user = User.find_by_login('jsmith')
+      @issue.description = "A new description"
+      assert @issue.save
+    end
+    
+    log_user('jsmith', 'jsmith')
+
+    # Issue page
+    visit_issue_page(@issue)
+    assert has_selector?("#history .journal-attributes li i", :text => 'A new description')
+    within("#history .journal-attributes li") do
+      find_link("More").click
+    end
+
+    # Diff page
+    assert_response :success
+    assert has_selector?("#content .text-diff", :text => /A new description/)
+  end
+end

test/integration/routing_test.rb

@@ -109,6 +109,10 @@ class RoutingTest < ActionController::IntegrationTest
     should_route :post, "/issues/bulk_edit", :controller => 'issues', :action => 'bulk_update'
   end
 
+  context "journals" do
+    should_route :get, "/journals/100/diff/description", :controller => 'journals', :action => 'diff', :id => '100', :field => 'description'
+  end
+  
   context "issue categories" do
     should_route :get, "/projects/test/issue_categories/new", :controller => 'issue_categories', :action => 'new', :project_id => 'test'
 

test/integration_test_helpers.rb

@@ -51,7 +51,7 @@ module IntegrationTestHelpers
       visit "/login"
       fill_in 'Login', :with => user
       fill_in 'Password', :with => password
-      click_button 'login'
+      click_button 'Login'
       assert_response :success
       assert User.current.logged?
     end

vendor/plugins/acts_as_journalized/lib/journal_formatter.rb

@@ -28,6 +28,7 @@ module JournalFormatter
   include CustomFieldsHelper
   include ActionView::Helpers::TagHelper
   include ActionView::Helpers::UrlHelper
+  include ActionView::Helpers::TextHelper
   include ActionController::UrlWriter
   extend Redmine::I18n
 
@@ -122,6 +123,16 @@ module JournalFormatter
     [label, old_value, value]
   end
 
+  # Formats a detail to be used with a Journal diff
+  #
+  # Truncates the content. Adds a link to view a diff.
+  def format_html_diff_detail(key, label, old_value, value)
+    link = link_to(l(:label_more), {:controller => 'journals', :action => 'diff', :id => id, :field => key.to_s}, :class => 'lightbox-ajax')
+    old_value = truncate(old_value, :length => 80)
+    value = truncate(value, :length => 80) + " " + link
+    [old_value, value]
+  end
+  
   def property(detail)
     key = prop_key(detail)
     if key.start_with? "custom_values"
@@ -186,6 +197,9 @@ module JournalFormatter
     unless no_html
       label, old_value, value = *format_html_detail(label, old_value, value)
       value = format_html_attachment_detail(key.sub("attachments", ""), value) if attachment_detail
+      if property(detail) == :attribute && key == "description"
+        old_value, value = *format_html_diff_detail(key, label, old_value, value)
+      end
     end
 
     unless value.blank?