From 66ab19b68b5b638fced4e7986e74858117eafacc Mon Sep 17 00:00:00 2001
From: Philipp Tessenow <philipp@tessenow.org>
Date: Tue, 9 Jul 2013 10:47:38 +0200
Subject: [PATCH 01/16] refactor watchers helper to return html_safe html

---
 app/helpers/watchers_helper.rb | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/app/helpers/watchers_helper.rb b/app/helpers/watchers_helper.rb
index 91e58ef40b..d0b9c54a9d 100644
--- a/app/helpers/watchers_helper.rb
+++ b/app/helpers/watchers_helper.rb
@@ -53,21 +53,25 @@ module WatchersHelper
     link_to(label, path, html_options.merge(:remote => true, :method => method))
   end
 
-  # Returns a comma separated list of users watching the given object
+  # Returns HTML for a list of users watching the given object
   def watchers_list(object)
     remove_allowed = User.current.allowed_to?("delete_#{object.class.name.underscore}_watchers".to_sym, object.project)
-    lis = object.watchers.collect do |watch|
-      s = avatar(watch.user, :size => "16").to_s + link_to_user(watch.user, :class => 'user').to_s
-      if remove_allowed
-        s += ' ' + link_to(image_tag('red_x.png', :alt => l(:button_delete), :title => l(:button_delete)),
-                           watcher_path(watch),
-                           :method => :delete,
-                           :remote => true,
-                           :style => "vertical-align: middle",
-                           :class => "delete")
+    lis = object.watchers(true).collect do |watch|
+      content_tag :li do
+        avatar(watch.user, :size => "16") +
+          link_to_user(watch.user, :class => 'user') +
+          if remove_allowed
+            ' '.html_safe + link_to(image_tag('red_x.png', :alt => l(:button_delete), :title => l(:button_delete)),
+                             watcher_path(watch),
+                             :method => :delete,
+                             :remote => true,
+                             :style => "vertical-align: middle",
+                             :class => "delete")
+          else
+            ''.html_safe
+          end
       end
-      "<li>#{ s }</li>"
     end
-    lis.empty? ? "" : "<ul>#{ lis.join("\n") }</ul>".html_safe
+    lis.empty? ? ''.html_safe : content_tag(:ul, lis.reduce(:+))
   end
 end

From 4a932dae82f0e075d1347d8617708473645418cd Mon Sep 17 00:00:00 2001
From: Philipp Tessenow <philipp@tessenow.org>
Date: Tue, 9 Jul 2013 14:01:38 +0200
Subject: [PATCH 02/16] make gravatar helper html_safe when no gravatar is
 shown

---
 app/helpers/application_helper.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index ab549ed8a9..7e5dd85d64 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -1026,7 +1026,7 @@ module ApplicationHelper
       end
       return gravatar(email.to_s.downcase, options) unless email.blank? rescue nil
     else
-      ''
+      ''.html_safe
     end
   end
 

From 721ea01b22dba58f7c1254c0c819acde11b10ae4 Mon Sep 17 00:00:00 2001
From: Philipp Tessenow <philipp@tessenow.org>
Date: Tue, 9 Jul 2013 14:01:57 +0200
Subject: [PATCH 03/16] add cukes for adding/removing watchers in issue#show

---
 features/issues/issue_show.feature | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/features/issues/issue_show.feature b/features/issues/issue_show.feature
index 7d60018519..b78d6cc7ba 100644
--- a/features/issues/issue_show.feature
+++ b/features/issues/issue_show.feature
@@ -25,7 +25,10 @@ Feature: Watch issues
     And the role "member" may have the following rights:
       | view_work_packages |
     And there is 1 user with the following:
-      | login | bob|
+      | login     | bob    |
+      | firstname | Bob    |
+      | lastname  | Bobbit |
+      | admin     | true   |
     And the user "bob" is a "member" in the project "parent"
     Given the user "bob" has 1 issue with the following:
       | subject     | issue1              |
@@ -48,3 +51,23 @@ Feature: Watch issues
     When I click on "Unwatch" within "#content > .action_menu_main"
     Then I should see "Watch" within "#content > .action_menu_main"
     Then the issue "issue1" should have 0 watchers
+
+  @javascript
+  Scenario: Add a watcher to an issue
+    When I go to the page of the issue "issue1"
+    Then I should see "Add watcher" within "#content > .issue > #watchers"
+    When I click on "Add watcher" within "#content > .issue > #watchers"
+    And I select "Bob Bobbit" from "watcher_user_id" within "#content > .issue > #watchers"
+    And I press "Add" within "#content > .issue > #watchers"
+    Then I should see "Bob Bobbit" within "#content > .issue > #watchers > ul"
+    Then the issue "issue1" should have 1 watchers
+
+  @javascript
+  Scenario: Remove a watcher from an issue
+    Given the issue "issue1" is watched by:
+      | bob |
+    When I go to the page of the issue "issue1"
+    Then I should see "Bob Bobbit" within "#content > .issue > #watchers > ul"
+    When I click on "Delete" within "#content > .issue > #watchers > ul"
+    Then I should not see "Bob Bobbit" within "#content > .issue > #watchers"
+    Then the issue "issue1" should have 0 watchers

From fe18331cb17c825c844a945728d7d4c471983d0f Mon Sep 17 00:00:00 2001
From: Philipp Tessenow <philipp@tessenow.org>
Date: Tue, 9 Jul 2013 15:13:46 +0200
Subject: [PATCH 04/16] update changelog

---
 doc/CHANGELOG.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/doc/CHANGELOG.md b/doc/CHANGELOG.md
index 82b8d8ebb9..182f4a53ca 100644
--- a/doc/CHANGELOG.md
+++ b/doc/CHANGELOG.md
@@ -1,5 +1,6 @@
 # Changelog
 
+* `#1303` Watcherlist contains unescaped HTML
 * `#1315` Correct spelling mistakes in German translation
 * `#1299` Refactor user status
 * `#1301` Ajax call when logged out should open a popup window

From d6563b68199b750c1be0be03ecb2413bbc82e399 Mon Sep 17 00:00:00 2001
From: jwollert <johannes.wollert@gmail.com>
Date: Tue, 9 Jul 2013 18:07:33 +0200
Subject: [PATCH 05/16] changed resued password wording to one that is a little
 less complex and a little more polite

---
 config/locales/de.yml | 2 +-
 config/locales/en.yml | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/config/locales/de.yml b/config/locales/de.yml
index f97a141124..fff2f1feaf 100644
--- a/config/locales/de.yml
+++ b/config/locales/de.yml
@@ -209,7 +209,7 @@ de:
               uppercase: "Großbuchstaben (z.B. 'A')"
               numeric: "Ziffern (z.B. '1')"
               special: "Sonderzeichen (z.B. '%')"
-              reused: "darf nicht eines der %{count} zuletzt genutzten sein."
+              reused: "wurde bereits verwendet. Bitte wählen Sie eines, was sich von den letzten %{count} unterscheidet."
       template:
         body: "Bitte überprüfen Sie die folgenden Felder:"
         header:
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 9bb3405685..0bdc5747c4 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -213,7 +213,7 @@ en:
               uppercase: "uppercase (e.g. 'A')"
               numeric: "numeric (e.g. '1')"
               special: "special (e.g. '%')"
-              reused: "must not be one of the last %{count} used"
+              reused: "has been used before. Please choose one that is different from your last %{count}."
       template:
         body: "Please check the following fields:"
         header:

From 234ff1e50ef2e9c6e5869c9e7058574b8c109ca8 Mon Sep 17 00:00:00 2001
From: Michael Frister <m.frister@finn.de>
Date: Thu, 11 Jul 2013 08:47:16 +0200
Subject: [PATCH 06/16] Password reused warning: Improve singular

[skip ci]
---
 app/models/user.rb    | 2 +-
 config/locales/de.yml | 4 +++-
 config/locales/en.yml | 4 +++-
 3 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/app/models/user.rb b/app/models/user.rb
index 752bc32280..3ae4720f74 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -671,7 +671,7 @@ class User < Principal
         if former_passwords_include?(self.password)
           errors.add(:password,
                      I18n.t(:reused,
-                            :count => Setting[:password_count_former_banned],
+                            :count => Setting[:password_count_former_banned].to_i,
                             :scope => [:activerecord,
                                        :errors,
                                        :models,
diff --git a/config/locales/de.yml b/config/locales/de.yml
index fff2f1feaf..d86cc164b7 100644
--- a/config/locales/de.yml
+++ b/config/locales/de.yml
@@ -209,7 +209,9 @@ de:
               uppercase: "Großbuchstaben (z.B. 'A')"
               numeric: "Ziffern (z.B. '1')"
               special: "Sonderzeichen (z.B. '%')"
-              reused: "wurde bereits verwendet. Bitte wählen Sie eines, was sich von den letzten %{count} unterscheidet."
+              reused:
+                one: "wurde bereits verwendet. Bitte wählen Sie eines, das sich vom letzten unterscheidet."
+                other: "wurde bereits verwendet. Bitte wählen Sie eines, das sich von den letzten %{count} unterscheidet."
       template:
         body: "Bitte überprüfen Sie die folgenden Felder:"
         header:
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 0bdc5747c4..b187b1be72 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -213,7 +213,9 @@ en:
               uppercase: "uppercase (e.g. 'A')"
               numeric: "numeric (e.g. '1')"
               special: "special (e.g. '%')"
-              reused: "has been used before. Please choose one that is different from your last %{count}."
+              reused:
+                one: "has been used before. Please choose one that is different from your last one."
+                other: "has been used before. Please choose one that is different from your last %{count}."
       template:
         body: "Please check the following fields:"
         header:

From 101bf289e7476b222ba93cb0b3fd4d7f560eefdc Mon Sep 17 00:00:00 2001
From: Jens Ulferts <j.ulferts@finn.de>
Date: Thu, 11 Jul 2013 09:35:48 +0200
Subject: [PATCH 07/16] moves attribute rendering for work_package#show from
 controller to helper

---
 app/controllers/work_packages_controller.rb |  50 ---------
 app/helpers/work_packages_helper.rb         | 111 ++++++++++++++++++++
 app/views/work_packages/show.html.erb       |  27 +----
 3 files changed, 115 insertions(+), 73 deletions(-)

diff --git a/app/controllers/work_packages_controller.rb b/app/controllers/work_packages_controller.rb
index 8496ca6671..18302d026a 100644
--- a/app/controllers/work_packages_controller.rb
+++ b/app/controllers/work_packages_controller.rb
@@ -204,56 +204,6 @@ class WorkPackagesController < ApplicationController
     IssuePriority.all
   end
 
-  WorkPackageAttribute = Struct.new(:attribute, :html_class, :value)
-
-  WorkPackageUserAttribute = Struct.new(:attribute, :html_class, :value)
-
-  WorkPackageProgressAttribute = Struct.new(:attribute, :html_class, :value)
-
-  WorkPackagePlanningElementTypeAttribute = Struct.new(:attribute, :html_class, :value)
-
-  def work_package_attributes
-    if work_package.is_a? Issue
-      return [
-        WorkPackageAttribute.new(:status, 'status', work_package.status.name),
-        WorkPackageAttribute.new(:start_date, 'start-date', format_date(work_package.start_date)),
-        WorkPackageAttribute.new(:priority, 'priority', work_package.priority),
-        WorkPackageAttribute.new(:due_date, 'due-date', format_date(work_package.due_date)),
-        WorkPackageUserAttribute.new(:assigned_to, 'assigned-to', work_package.assigned_to),
-        WorkPackageProgressAttribute.new(:done_ratio, 'progress', work_package.done_ratio),
-        WorkPackageAttribute.new(:category,
-                                 'category',
-                                 (work_package.category.nil?) ? '' : work_package.category.name),
-        WorkPackageAttribute.new(:spent_time,
-                                 'spent-time',
-                                 work_package.spent_hours > 0 ? (view_context.link_to l_hours(work_package.spent_hours),
-                                                                 issue_time_entries_path(work_package)) : "-"),
-        WorkPackageAttribute.new(:fixed_version,
-                                 'fixed-version',
-                                 work_package.fixed_version ? view_context.link_to_version(work_package.fixed_version) : "-"),
-        WorkPackageAttribute.new(:estimated_hours, 'estimated_hours', l_hours(work_package.estimated_hours))
-      ]
-    elsif work_package.is_a? PlanningElement
-      format_date_options = {}
-      unless work_package.leaf?
-        format_date_options[:title] = l("timelines.dates_are_calculated_based_on_sub_elements")
-      end
-
-      return [
-        WorkPackageUserAttribute.new(:responsible, 'responsible', work_package.responsible),
-        WorkPackageAttribute.new(:start_date, 'start-date', format_date(work_package.start_date)),
-        WorkPackageAttribute.new(:parent_id,
-                                 'planning-element-parent-id',
-                                 work_package.parent ? (view_context.link_to_planning_element(work_package.parent,
-                                                                                              :include_id => false)) : ''),
-        WorkPackageAttribute.new(:due_date, 'due-date', format_date(work_package.end_date)),
-        WorkPackagePlanningElementTypeAttribute.new(:type,
-                                                    'planning-element-type',
-                                                    work_package.planning_element_type),
-      ]
-    end
-  end
-
   protected
 
   def assign_planning_elements
diff --git a/app/helpers/work_packages_helper.rb b/app/helpers/work_packages_helper.rb
index 6007f3e76b..47582369fc 100644
--- a/app/helpers/work_packages_helper.rb
+++ b/app/helpers/work_packages_helper.rb
@@ -88,6 +88,117 @@ module WorkPackagesHelper
     ].compact
   end
 
+  def work_package_show_attributes(work_package)
+    [
+      work_package_show_status_attribute(work_package),
+      work_package_show_start_date_attribute(work_package),
+      work_package_show_priority_attribute(work_package),
+      work_package_show_due_date_attribute(work_package),
+      work_package_show_assigned_to_attribute(work_package),
+      work_package_show_progress_attribute(work_package),
+      work_package_show_responsible_attribute(work_package),
+      work_package_show_category_attribute(work_package),
+      work_package_show_spent_time_attribute(work_package),
+      work_package_show_fixed_version_attribute(work_package),
+      work_package_show_estimated_hours_attribute(work_package)
+    ].compact
+  end
+
+  def work_package_show_table_row(attribute, klass = nil, &block)
+    klass = attribute.to_s.dasherize if klass.nil?
+
+    content = content_tag(:th, :class => klass) { "#{Issue.human_attribute_name(attribute)}:" }
+    content << content_tag(:td, :class => klass, &block)
+
+    WorkPackageAttribute.new(attribute, content)
+  end
+
+  def work_package_show_status_attribute(work_package)
+    work_package_show_table_row(:status) do
+      work_package.status ?
+        work_package.status.name :
+        "-"
+    end
+  end
+
+  def work_package_show_start_date_attribute(work_package)
+    work_package_show_table_row(:start_date, 'start-date') do
+      work_package.start_date ?
+        format_date(work_package.start_date) :
+        "-"
+    end
+  end
+
+  def work_package_show_priority_attribute(work_package)
+    work_package_show_table_row(:priority) do
+      work_package.priority ?
+        work_package.priority.name :
+        "-"
+    end
+  end
+
+  def work_package_show_due_date_attribute(work_package)
+    work_package_show_table_row(:due_date) do
+      work_package.due_date ?
+        format_date(work_package.due_date) :
+        "-"
+    end
+  end
+
+  def work_package_show_assigned_to_attribute(work_package)
+    work_package_show_table_row(:assigned_to) do
+      content = avatar(work_package.assigned_to, :size => "14").html_safe
+      content << (work_package.assigned_to ? link_to_user(work_package.assigned_to) : "-")
+      content
+    end
+  end
+
+  def work_package_show_responsible_attribute(work_package)
+    work_package_show_table_row(:responsible) do
+      content = avatar(work_package.responsible, :size => "14").html_safe
+      content << (work_package.responsible ? link_to_user(work_package.responsible) : "-")
+      content
+    end
+  end
+
+  def work_package_show_progress_attribute(work_package)
+    work_package_show_table_row(:progress, 'done-ratio') do
+      progress_bar work_package.done_ratio, :width => '80px', :legend => work_package.done_ratio.to_s
+    end
+  end
+
+  def work_package_show_category_attribute(work_package)
+    work_package_show_table_row(:category) do
+      work_package.category ?
+        work_package.category.name :
+        '-'
+    end
+  end
+
+  def work_package_show_spent_time_attribute(work_package)
+    work_package_show_table_row(:spent_time) do
+      work_package.spent_hours > 0 ?
+        link_to(l_hours(work_package.spent_hours), issue_time_entries_path(work_package)) :
+        "-"
+    end
+  end
+
+  def work_package_show_fixed_version_attribute(work_package)
+    work_package_show_table_row(:fixed_version) do
+      work_package.fixed_version ?
+        link_to_version(work_package.fixed_version) :
+        "-"
+    end
+  end
+
+  def work_package_show_estimated_hours_attribute(work_package)
+    work_package_show_table_row(:estimated_hours) do
+      work_package.estimated_hours ?
+        l_hours(work_package.estimated_hours) :
+        "-"
+    end
+  end
+
   def work_package_form_tracker_attribute(form, work_package, locals = {})
     if work_package.is_a?(Issue)
       field = form.select :tracker_id, locals[:project].trackers.collect {|t| [t.name, t.id]}, :required => true
diff --git a/app/views/work_packages/show.html.erb b/app/views/work_packages/show.html.erb
index 7d5f0f30bf..1f76bb29df 100644
--- a/app/views/work_packages/show.html.erb
+++ b/app/views/work_packages/show.html.erb
@@ -34,31 +34,12 @@ See doc/COPYRIGHT.rdoc for more details.
   <div class="issue details">
     <div class="meta">
       <table class="attributes">
-        <% controller.work_package_attributes.each_slice(2) do |attributes| %>
+        <% work_package_show_attributes(controller.work_package).each_slice(2) do |attributes| %>
           <tr>
           <% attributes.each do |attribute| %>
-            <th class="<%= attribute.html_class %>"><%= Issue.human_attribute_name(attribute.attribute)%>:</th>
-
-            <% if attribute.is_a? ::WorkPackagesController::WorkPackageAttribute %>
-              <td class="<%= attribute.html_class %>"><%= h(attribute.value) %></td>
-
-            <% elsif attribute.is_a? ::WorkPackagesController::WorkPackageUserAttribute %>
-              <td class="<%= attribute.html_class %>">
-                <%= avatar(attribute.value, :size => "14") %>
-                <%= attribute.value ? link_to_user(attribute.value) : "-" %>
-              </td>
-
-            <% elsif attribute.is_a? ::WorkPackagesController::WorkPackageProgressAttribute %>
-              <td class="<%= attribute.html_class %>">
-                <%= progress_bar attribute.value, :width => '80px', :legend => "#{attribute.value}%" %>
-              </td>
-
-            <% elsif attribute.is_a? ::WorkPackagesController::WorkPackagePlanningElementTypeAttribute %>
-              <td class="<%= attribute.html_class %>">
-                <%= icon_for_planning_element_type(attribute.value) %>
-                <%= h attribute.value.try(:name) %>
-              </td>
-            <% end %>
+
+              <%= attribute.field %>
+
           <% end %>
           </tr>
         <% end %>

From 81d42e32dff405dee066816dc42b9fa6649c175a Mon Sep 17 00:00:00 2001
From: Jens Ulferts <j.ulferts@finn.de>
Date: Thu, 11 Jul 2013 09:36:24 +0200
Subject: [PATCH 08/16] i18n for progress

---
 config/locales/de.yml | 1 +
 config/locales/en.yml | 1 +
 2 files changed, 2 insertions(+)

diff --git a/config/locales/de.yml b/config/locales/de.yml
index a7ec5f4a44..f3d44bb9f6 100644
--- a/config/locales/de.yml
+++ b/config/locales/de.yml
@@ -157,6 +157,7 @@ de:
         parent_title: "Übergeordnete Seite"
         redirect_existing_links: "Existierende Links umleiten"
       work_package:
+        progress: "% erledigt"
         responsible: "Planungsverantwortlicher"
         spent_time: "Aufgewendete Zeit"
 
diff --git a/config/locales/en.yml b/config/locales/en.yml
index be0262350b..5d508cfbed 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -162,6 +162,7 @@ en:
         reported_project_status: Project status
         created_at: Created
       work_package:
+        progress: "% done"
         responsible: "Responsible"
         spent_time: "Spent time"
 

From 8e149a2b3222a366b041e5f574e69fb11ba2b67d Mon Sep 17 00:00:00 2001
From: Jens Ulferts <j.ulferts@finn.de>
Date: Thu, 11 Jul 2013 09:38:23 +0200
Subject: [PATCH 09/16] redirect to work_package#show after issue#update

---
 app/controllers/issues_controller.rb         |  2 +-
 test/functional/issues_controller_test.rb    | 22 ++++++++++----------
 test/functional/user_mailer_test.rb          |  6 +++---
 test/integration/issues_test.rb              |  2 +-
 test/unit/helpers/application_helper_test.rb |  4 ++--
 5 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb
index 9a1f218223..af7a2a8fc5 100644
--- a/app/controllers/issues_controller.rb
+++ b/app/controllers/issues_controller.rb
@@ -203,7 +203,7 @@ class IssuesController < ApplicationController
       flash[:notice] = l(:notice_successful_update) unless @issue.current_journal == @journal
 
       respond_to do |format|
-        format.html { redirect_back_or_default({:action => 'show', :id => @issue}) }
+        format.html { redirect_back_or_default(work_package_path(@issue)) }
       end
     else
       render_attachment_warning_if_needed(@issue)
diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb
index 355c12ac5a..c7ccd6a832 100644
--- a/test/functional/issues_controller_test.rb
+++ b/test/functional/issues_controller_test.rb
@@ -318,7 +318,7 @@ class IssuesControllerTest < ActionController::TestCase
     get :show, :id => 1, :format => 'atom'
     assert_response :success
     assert_template 'journals/index'
-    assert_select 'content', :text => Regexp.new(Regexp.quote('http://test.host/issues/2'))
+    assert_select 'content', :text => Regexp.new(Regexp.quote('http://test.host/work_packages/2'))
   end
 
   def test_show_export_to_pdf
@@ -807,7 +807,7 @@ class IssuesControllerTest < ActionController::TestCase
     assert issue.current_journal.changed_data.has_key? "priority_id"
     assert !issue.current_journal.changed_data.has_key?("category_id")
 
-    assert_redirected_to :action => 'show', :id => '1'
+    assert_redirected_to work_package_path(1)
     issue.reload
     assert_equal new_subject, issue.subject
     # Make sure custom fields were not cleared
@@ -838,7 +838,7 @@ class IssuesControllerTest < ActionController::TestCase
     assert !issue.current_journal.changed_data.has_key?("category_id")
     assert issue.current_journal.changed_data.has_key? "custom_values2"
 
-    assert_redirected_to :action => 'show', :id => '1'
+    assert_redirected_to work_package_path(1)
     issue.reload
     assert_equal 'New custom value', issue.custom_value_for(2).value
 
@@ -858,7 +858,7 @@ class IssuesControllerTest < ActionController::TestCase
            :notes => 'Assigned to dlopper',
            :time_entry => { :hours => '', :comments => '', :activity_id => TimeEntryActivity.first }
     end
-    assert_redirected_to :action => 'show', :id => '1'
+    assert_redirected_to work_package_path(1)
     issue.reload
     assert_equal 2, issue.status_id
     j = WorkPackageJournal.find(:first, :order => 'id DESC')
@@ -877,7 +877,7 @@ class IssuesControllerTest < ActionController::TestCase
     put :update,
          :id => 1,
          :notes => notes
-    assert_redirected_to :action => 'show', :id => '1'
+    assert_redirected_to work_package_path(1)
     j = WorkPackageJournal.find(:first, :order => 'id DESC')
     assert_equal notes, j.notes
     assert_equal 0, j.details.size
@@ -896,7 +896,7 @@ class IssuesControllerTest < ActionController::TestCase
            :notes => '2.5 hours added',
            :time_entry => { :hours => '2.5', :comments => 'test_put_update_with_note_and_spent_time', :activity_id => TimeEntryActivity.first.id }
     end
-    assert_redirected_to :action => 'show', :id => '1'
+    assert_redirected_to work_package_path(1)
 
     issue = Issue.find(1)
 
@@ -921,7 +921,7 @@ class IssuesControllerTest < ActionController::TestCase
          :id => 1,
          :notes => '',
          :attachments => {'1' => {'file' => uploaded_test_file('testfile.txt', 'text/plain')}}
-    assert_redirected_to :action => 'show', :id => '1'
+    assert_redirected_to work_package_path(1)
     j = Issue.find(1).last_journal
     assert j.notes.blank?
     assert_equal 1, j.details.size
@@ -948,7 +948,7 @@ class IssuesControllerTest < ActionController::TestCase
          :id => 1,
          :notes => '',
          :attachments => {'1' => {'file' => uploaded_test_file('testfile.txt', 'text/plain')}}
-    assert_redirected_to :action => 'show', :id => '1'
+    assert_redirected_to work_package_path(1)
     assert_equal '1 file(s) could not be saved.', flash[:warning]
 
   end if Object.const_defined?(:Mocha)
@@ -962,7 +962,7 @@ class IssuesControllerTest < ActionController::TestCase
            :id => 1,
            :notes => ''
     end
-    assert_redirected_to :action => 'show', :id => '1'
+    assert_redirected_to work_package_path(1)
 
     issue.reload
     # No email should be sent
@@ -1067,7 +1067,7 @@ class IssuesControllerTest < ActionController::TestCase
     assert_redirected_to '/issues'
   end
 
-  def test_put_update_should_not_redirect_back_using_the_back_url_parameter_off_the_host
+  def test_put_update_should_redirect_back_using_the_back_url_parameter_off_the_host
     issue = Issue.find(2)
     @request.session[:user_id] = 2
 
@@ -1079,7 +1079,7 @@ class IssuesControllerTest < ActionController::TestCase
          :back_url => 'http://google.com'
 
     assert_response :redirect
-    assert_redirected_to :controller => 'issues', :action => 'show', :id => issue.id
+    assert_redirected_to work_package_path(issue.id)
   end
 
   def test_get_bulk_edit
diff --git a/test/functional/user_mailer_test.rb b/test/functional/user_mailer_test.rb
index d11ad319ad..8ced13ce31 100644
--- a/test/functional/user_mailer_test.rb
+++ b/test/functional/user_mailer_test.rb
@@ -84,7 +84,7 @@ class UserMailerTest < ActionMailer::TestCase
                     :text => "Details"
       # link to a referenced ticket
       assert_select 'a[href=?][title=?]',
-                    "https://mydomain.foo/issues/#{related_issue.id}",
+                    "https://mydomain.foo/work_packages/#{related_issue.id}",
                     "My related Ticket (#{related_issue.status})",
                     :text => "##{related_issue.id}"
       # link to a changeset
@@ -122,7 +122,7 @@ class UserMailerTest < ActionMailer::TestCase
                     :text => "Details"
       # link to a referenced ticket
       assert_select 'a[href=?][title=?]',
-                    "http://mydomain.foo/rdm/issues/#{related_issue.id}",
+                    "http://mydomain.foo/rdm/work_packages/#{related_issue.id}",
                     "My related Ticket (#{related_issue.status})",
                     :text => "##{related_issue.id}"
       # link to a changeset
@@ -163,7 +163,7 @@ class UserMailerTest < ActionMailer::TestCase
                     :text => "Details"
       # link to a referenced ticket
       assert_select 'a[href=?][title=?]',
-                    "http://mydomain.foo/rdm/issues/#{related_issue.id}",
+                    "http://mydomain.foo/rdm/work_packages/#{related_issue.id}",
                     "My related Ticket (#{related_issue.status})",
                     :text => "##{related_issue.id}"
       # link to a changeset
diff --git a/test/integration/issues_test.rb b/test/integration/issues_test.rb
index d9435e7522..2dc3ac638a 100644
--- a/test/integration/issues_test.rb
+++ b/test/integration/issues_test.rb
@@ -55,7 +55,7 @@ class IssuesTest < ActionDispatch::IntegrationTest
     put 'issues/1',
          :notes => 'Some notes',
          :attachments => {'1' => {'file' => uploaded_test_file('testfile.txt', 'text/plain'), 'description' => 'This is an attachment'}}
-    assert_redirected_to "/issues/1"
+    assert_redirected_to "/work_packages/1"
     follow_redirect!
 
     # make sure attachment was saved
diff --git a/test/unit/helpers/application_helper_test.rb b/test/unit/helpers/application_helper_test.rb
index fb699485b2..e3efd36c63 100644
--- a/test/unit/helpers/application_helper_test.rb
+++ b/test/unit/helpers/application_helper_test.rb
@@ -229,7 +229,7 @@ RAW
     identifier = @project.identifier
     @project.reload
 
-    issue_link = link_to("##{@issue.id}", {:controller => 'issues', :action => 'show', :id => @issue},
+    issue_link = link_to("##{@issue.id}", work_package_path(@issue),
                                :class => 'issue status-3 priority-1 created-by-me', :title => "#{@issue.subject} (#{@issue.status})")
 
     changeset_link = link_to("r#{changeset1.revision}", {:controller => 'repositories', :action => 'revision', :id => identifier, :rev => changeset1.revision},
@@ -498,7 +498,7 @@ RAW
 
     expected = <<-EXPECTED
 <p><a href="/projects/#{@project.identifier}/wiki/CookBook_documentation" class="wiki-page">CookBook documentation</a></p>
-<p><a href="/issues/#{@issue.id}" class="issue status-3 priority-1 created-by-me" title="#{@issue.subject} (#{@issue.status})">##{@issue.id}</a></p>
+<p><a href="/work_packages/#{@issue.id}" class="issue status-3 priority-1 created-by-me" title="#{@issue.subject} (#{@issue.status})">##{@issue.id}</a></p>
 <pre>
 [[CookBook documentation]]
 

From e37ab4b3579ff773e1306bad11d1bd16b62cd188 Mon Sep 17 00:00:00 2001
From: Jens Ulferts <j.ulferts@finn.de>
Date: Thu, 11 Jul 2013 09:39:10 +0200
Subject: [PATCH 10/16] uses work_package for work_package history

---
 app/views/work_packages/_history.html.erb | 4 ++--
 app/views/work_packages/show.html.erb     | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/app/views/work_packages/_history.html.erb b/app/views/work_packages/_history.html.erb
index c78b0e8d22..fa8e0b59b0 100644
--- a/app/views/work_packages/_history.html.erb
+++ b/app/views/work_packages/_history.html.erb
@@ -17,7 +17,7 @@ a journal as the cache could then not be used between all of an issue's journals
 <% @journal_cache = Acts::Journalized::JournalObjectCache.new %>
 
 <% for journal in journals %>
-  <%= render_journal issue,
+  <%= render_journal work_package,
                      journal,
                      :edit_permission => :edit_issue_notes,
                      :edit_own_permission => :edit_own_issue_notes,
@@ -25,4 +25,4 @@ a journal as the cache could then not be used between all of an issue's journals
   <%= call_hook(:view_issues_history_journal_bottom, { :journal => journal }) %>
 <% end %>
 
-<% heads_for_wiki_formatter if User.current.allowed_to?(:edit_issue_notes, issue.project) || User.current.allowed_to?(:edit_own_issue_notes, issue.project) %>
+<% heads_for_wiki_formatter if User.current.allowed_to?(:edit_issue_notes, work_package.project) || User.current.allowed_to?(:edit_own_issue_notes, work_package.project) %>
diff --git a/app/views/work_packages/show.html.erb b/app/views/work_packages/show.html.erb
index 1f76bb29df..cffb03c13f 100644
--- a/app/views/work_packages/show.html.erb
+++ b/app/views/work_packages/show.html.erb
@@ -104,7 +104,7 @@ See doc/COPYRIGHT.rdoc for more details.
 <% if controller.journals.present? %>
   <div id="history">
     <h3><%=l(:label_history)%></h3>
-    <%= render :partial => 'history', :locals => { :issue => controller.work_package, :journals => controller.journals } %>
+    <%= render :partial => 'history', :locals => { :work_package => controller.work_package, :journals => controller.journals } %>
   </div>
 <% end %>
 

From d7abdc50c7090b278ffb6ca5be0f07fe76753563 Mon Sep 17 00:00:00 2001
From: Jens Ulferts <j.ulferts@finn.de>
Date: Thu, 11 Jul 2013 09:42:11 +0200
Subject: [PATCH 11/16] specs rendering due date journal changes instead of end
 date

---
 spec/lib/journal_formatter/scenario_date_spec.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/spec/lib/journal_formatter/scenario_date_spec.rb b/spec/lib/journal_formatter/scenario_date_spec.rb
index e4bc989aa2..ea3399958e 100644
--- a/spec/lib/journal_formatter/scenario_date_spec.rb
+++ b/spec/lib/journal_formatter/scenario_date_spec.rb
@@ -46,11 +46,11 @@ describe OpenProject::JournalFormatter::ScenarioDate do
       it { instance.render(key, [nil, new_date]).should == expected }
     end
 
-    describe "WITH rendering the end date
+    describe "WITH rendering the due date
               WITH the first value beeing nil, and the second a date
               WITH the scenario existing" do
       let(:new_date) { Date.today }
-      let(:date_type) { "end" }
+      let(:date_type) { "due" }
       let(:expected) { I18n.t(:text_journal_set_to,
                               :label => "<strong>#{ I18n.t(:"label_scenario_#{date_type}_date", :title => scenario.name) }</strong>",
                               :value => "<i>#{format_date(new_date)}</i>") }

From c97cd466b3d6e316dc7f0ba632a0ab4056f0a81a Mon Sep 17 00:00:00 2001
From: Jens Ulferts <j.ulferts@finn.de>
Date: Thu, 11 Jul 2013 10:24:55 +0200
Subject: [PATCH 12/16] fixes issue creation cuke

---
 features/issues/issue_edit.feature | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/features/issues/issue_edit.feature b/features/issues/issue_edit.feature
index 3eb7b90b73..3aa48d391f 100644
--- a/features/issues/issue_edit.feature
+++ b/features/issues/issue_edit.feature
@@ -68,7 +68,7 @@ Feature: Issue edit
   Scenario: On an issue with children a User should not be able to change attributes which are overridden by children
     When I go to the page of the issue "issue1"
     And I click on "Add subtask"
-    Then I should see "New issue" within "#content"
+    Then I should be on the new work_package page of the project called "omicronpersei8"
     When I fill in "find the popplers" for "Subject"
     And I click on the first button matching "Create"
     Then I should see "Successful creation."

From eda5f61ddcc4a75b8269ca510927301d4a30e22a Mon Sep 17 00:00:00 2001
From: Jens Ulferts <j.ulferts@finn.de>
Date: Thu, 11 Jul 2013 10:26:15 +0200
Subject: [PATCH 13/16] disabling "create and continue" for now

---
 app/views/work_packages/new.html.erb | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/app/views/work_packages/new.html.erb b/app/views/work_packages/new.html.erb
index 4b4edacd4d..55bbd1ce79 100644
--- a/app/views/work_packages/new.html.erb
+++ b/app/views/work_packages/new.html.erb
@@ -34,7 +34,8 @@ See doc/COPYRIGHT.rdoc for more details.
   </div>
 
   <%= submit_tag l(:button_create) %>
-  <%= submit_tag l(:button_create_and_continue), :name => 'continue' %>
+  <!-- not supported by controller for now -->
+  <%#= submit_tag l(:button_create_and_continue), :name => 'continue' %>
   <%= link_to_issue_preview(controller.project) %>
 
   <%= javascript_tag "Form.Element.focus('#{f.object_name}_subject');" %>

From 69fc9db244d352e7c4da429e59048d97ee07ff08 Mon Sep 17 00:00:00 2001
From: Jens Ulferts <j.ulferts@finn.de>
Date: Thu, 11 Jul 2013 10:27:02 +0200
Subject: [PATCH 14/16] allows for extending permitted_params

---
 app/models/permitted_params.rb       | 128 +++++++++++++++++----------
 spec/models/permitted_params_spec.rb |  48 ++++++++--
 2 files changed, 123 insertions(+), 53 deletions(-)

diff --git a/app/models/permitted_params.rb b/app/models/permitted_params.rb
index 2e6b3c04c0..a1fa71f6f5 100644
--- a/app/models/permitted_params.rb
+++ b/app/models/permitted_params.rb
@@ -42,76 +42,112 @@ class PermittedParams < Struct.new(:params, :user)
   #
   # include ActiveModel::ForbiddenAttributesProtection
 
+
+  def self.permit(key, *params)
+    raise(ArgumentError, "no permitted params are configured for #{key}") unless permitted_attributes.has_key?(key)
+
+    permitted_attributes[key].concat(params)
+  end
+
   def project_type
-    params.require(:project_type).permit(:name,
-                                         :allows_association,
-                                         # have to check whether this is correct
-                                         # just copying over from model for now
-                                         :planning_element_type_ids => [],
-                                         :reported_project_status_ids => [])
+    params.require(:project_type).permit(*self.class.permitted_attributes[:project_type])
   end
 
   def project_type_move
-    params.require(:project_type).permit(:move_to)
+    params.require(:project_type).permit(*self.class.permitted_attributes[:project_type_move])
   end
 
   def color
-    params.require(:color).permit(:name,
-                                  :hexcode,
-                                  :move_to)
+    params.require(:color).permit(*self.class.permitted_attributes[:color])
   end
 
   def color_move
-    params.require(:color).permit(:move_to)
+    params.require(:color).permit(*self.class.permitted_attributes[:color_move])
   end
 
   def planning_element_type
-    params.require(:planning_element_type).permit(:name,
-                                                  :in_aggregation,
-                                                  :is_milestone,
-                                                  :is_default,
-                                                  :color_id)
+    params.require(:planning_element_type).permit(*self.class.permitted_attributes[:planning_element_type])
   end
 
   def planning_element_type_move
-    params.require(:planning_element_type).permit(:move_to)
+    params.require(:planning_element_type).permit(*self.class.permitted_attributes[:planning_element_type_move])
   end
 
   def scenario
-    params.require(:scenario).permit(:name,
-                                     :description)
+    params.require(:scenario).permit(*self.class.permitted_attributes[:scenario])
   end
 
   def planning_element
-    params.require(:planning_element).permit(:subject,
-                                             :description,
-                                             :start_date,
-                                             :due_date,
-                                             { scenarios: [:id, :start_date, :due_date] },
-                                             :note,
-                                             :planning_element_type_id,
-                                             :planning_element_status_comment,
-                                             :planning_element_status_id,
-                                             :parent_id,
-                                             :responsible_id)
+    params.require(:planning_element).permit(*self.class.permitted_attributes[:planning_element])
   end
 
   def new_work_package
-    params[:work_package].permit(:subject,
-                                         :description,
-                                         :start_date,
-                                         :due_date,
-                                         :planning_element_type_id,
-                                         :parent_id,
-                                         :parent_issue_id,
-                                         :assigned_to_id,
-                                         :responsible_id,
-                                         :tracker_id,
-                                         :fixed_version_id,
-                                         :estimated_hours,
-                                         :done_ratio,
-                                         :priority_id,
-                                         :category_id,
-                                         :status_id)
+    params[:work_package].permit(*self.class.permitted_attributes[:new_work_package])
+  end
+
+  protected
+
+  def self.permitted_attributes
+    @whitelisted_params ||= {
+                              :new_work_package => [
+                                                     :subject,
+                                                     :description,
+                                                     :start_date,
+                                                     :due_date,
+                                                     :planning_element_type_id,
+                                                     :parent_id,
+                                                     :parent_issue_id,
+                                                     :assigned_to_id,
+                                                     :responsible_id,
+                                                     :tracker_id,
+                                                     :fixed_version_id,
+                                                     :estimated_hours,
+                                                     :done_ratio,
+                                                     :priority_id,
+                                                     :category_id,
+                                                     :status_id
+                                                   ],
+                               :color_move => [:move_to],
+                               :color => [
+                                           :name,
+                                           :hexcode,
+                                           :move_to
+                                         ],
+                               :planning_element => [
+                                                      :subject,
+                                                      :description,
+                                                      :start_date,
+                                                      :due_date,
+                                                      { scenarios: [:id, :start_date, :due_date] },
+                                                      :note,
+                                                      :planning_element_type_id,
+                                                      :planning_element_status_comment,
+                                                      :planning_element_status_id,
+                                                      :parent_id,
+                                                      :responsible_id
+                                                    ],
+                               :planning_element_type => [
+                                                           :name,
+                                                           :in_aggregation,
+                                                           :is_milestone,
+                                                           :is_default,
+                                                           :color_id
+                                                         ],
+                               :planning_element_type_move => [:move_to],
+                               :project_type_move => [:move_to],
+                               :project_type => [
+                                                  :name,
+                                                  :allows_association,
+                                                  # have to check whether this is correct
+                                                  # just copying over from model for now
+                                                  :planning_element_type_ids => [],
+                                                  :reported_project_status_ids => []
+                                                ],
+                               :scenario => [
+                                              :name,
+                                              :description
+                                            ]
+
+                            }
   end
 end
diff --git a/spec/models/permitted_params_spec.rb b/spec/models/permitted_params_spec.rb
index e76c618658..72c0de4acf 100644
--- a/spec/models/permitted_params_spec.rb
+++ b/spec/models/permitted_params_spec.rb
@@ -14,6 +14,31 @@ require File.expand_path('../../spec_helper', __FILE__)
 describe PermittedParams do
   let(:user) { FactoryGirl.build(:user) }
 
+  describe :permit do
+    it "adds an attribute to be permitted later" do
+      # just taking project_type here as an example, could be anything
+
+      # taking the originally whitelisted params to be restored later
+      original_whitelisted = PermittedParams.instance_variable_get(:@whitelisted_params)
+
+
+      params = ActionController::Parameters.new(:project_type => { "blubs1" => "blubs" } )
+
+      PermittedParams.new(params, user).project_type.should == {}
+
+      PermittedParams.permit(:project_type, :blubs1)
+
+      PermittedParams.new(params, user).project_type.should == { "blubs1" => "blubs" }
+
+
+      PermittedParams.instance_variable_set(:@whitelisted_params, original_whitelisted)
+    end
+
+    it "raises an argument error if key does not exist" do
+      expect{ PermittedParams.permit(:bogus_key) }.to raise_error ArgumentError
+    end
+  end
+
   describe :project_type do
     it "should return name" do
       params = ActionController::Parameters.new(:project_type => { "name" => "blubs" } )
@@ -221,13 +246,6 @@ describe PermittedParams do
       PermittedParams.new(params, user).planning_element.should == hash
     end
 
-    it "should permit parent_issue_id" do
-      hash = { "parent_issue_id" => "1" }
-
-      params = ActionController::Parameters.new(:planning_element => hash)
-
-      PermittedParams.new(params, user).planning_element.should == hash
-    end
 
     it "should permit responsible_id" do
       hash = { "responsible_id" => "1" }
@@ -311,6 +329,22 @@ describe PermittedParams do
       PermittedParams.new(params, user).new_work_package.should == hash
     end
 
+    it "should permit parent_issue_id" do
+      hash = { "parent_id" => "1" }
+
+      params = ActionController::Parameters.new(:work_package => hash)
+
+      PermittedParams.new(params, user).new_work_package.should == hash
+    end
+
+    it "should permit parent_issue_id" do
+      hash = { "parent_issue_id" => "1" }
+
+      params = ActionController::Parameters.new(:work_package => hash)
+
+      PermittedParams.new(params, user).new_work_package.should == hash
+    end
+
     it "should permit fixed_version_id" do
       hash = { "fixed_version_id" => "1" }
 

From ec74cb3edb899a8faf420357baf0200a817ac893 Mon Sep 17 00:00:00 2001
From: Jens Ulferts <j.ulferts@finn.de>
Date: Thu, 11 Jul 2013 11:47:08 +0200
Subject: [PATCH 15/16] updates changelog

---
 doc/CHANGELOG.md | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/doc/CHANGELOG.md b/doc/CHANGELOG.md
index 82b8d8ebb9..20a1c84c74 100644
--- a/doc/CHANGELOG.md
+++ b/doc/CHANGELOG.md
@@ -1,3 +1,6 @@
+
+* `#1119` Creates a unified view for work_package show, new and create
+
 # Changelog
 
 * `#1315` Correct spelling mistakes in German translation

From 37ee532c030bb9bf4598bd1f67177ae86243ed3b Mon Sep 17 00:00:00 2001
From: Nils Kenneweg <n.kenneweg@finn.de>
Date: Thu, 11 Jul 2013 13:49:19 +0200
Subject: [PATCH 16/16] makes PE show work again.

PEs do not have a spent_hours method so we need to check for its existance.
As soon as the tree model is the same we can move spent_hours up into work_package.
---
 app/helpers/work_packages_helper.rb | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/app/helpers/work_packages_helper.rb b/app/helpers/work_packages_helper.rb
index 47582369fc..53e35eba7f 100644
--- a/app/helpers/work_packages_helper.rb
+++ b/app/helpers/work_packages_helper.rb
@@ -177,7 +177,8 @@ module WorkPackagesHelper
 
   def work_package_show_spent_time_attribute(work_package)
     work_package_show_table_row(:spent_time) do
-      work_package.spent_hours > 0 ?
+      #This check can be removed as soon as spent_hours is part of work_package and not Issue
+      work_package.respond_to?(:spent_hours) && work_package.spent_hours > 0 ?
         link_to(l_hours(work_package.spent_hours), issue_time_entries_path(work_package)) :
         "-"
     end