jro_apps/redmine Commit - r13058:15d5c331ebc1

Let the new time_entry form be submitted without project (#17954)....

Jean-Philippe Lang -

r13058:15d5c331ebc1

parent child

Context file:

r13058:15d5c331ebc1

Collapse all files

app/controllers/timelog_controller.rb +23 -40

              class TimelogController < ApplicationController
                menu_item :issues
-               before_filter :find_project_for_new_time_entry, :only => [:create]
                before_filter :find_time_entry, :only => [:show, :edit, :update]
                before_filter :find_time_entries, :only => [:bulk_edit, :bulk_update, :destroy]
-               before_filter :authorize, :except => [:new, :index, :report]
+               before_filter :authorize, :only => [:show, :edit, :update, :bulk_edit, :bulk_update, :destroy]
-               before_filter :find_optional_project, :only => [:index, :report]
-               before_filter :find_optional_project_for_new_time_entry, :only => [:new]
-               before_filter :authorize_global, :only => [:new, :index, :report]
+               before_filter :find_optional_project, :only => [:new, :create, :index, :report]
+               before_filter :authorize_global, :only => [:new, :create, :index, :report]
                accept_rss_auth :index
                accept_api_auth :index, :show, :create, :update, :destroy
                def create
                  @time_entry ||= TimeEntry.new(:project => @project, :issue => @issue, :user => User.current, :spent_on => User.current.today)
                  @time_entry.safe_attributes = params[:time_entry]
+                 if @time_entry.project && !User.current.allowed_to?(:log_time, @time_entry.project)
+                   render_403
+                   return
+                 end
                  call_hook(:controller_timelog_edit_before_save, { :params => params, :time_entry => @time_entry })
                      format.html {
                        flash[:notice] = l(:notice_successful_create)
                        if params[:continue]
-                         if params[:project_id]
-                           options = {
-                             :time_entry => {:issue_id => @time_entry.issue_id, :activity_id => @time_entry.activity_id},
-                             :back_url => params[:back_url]
+                           }
-                           if @time_entry.issue
-                             redirect_to new_project_issue_time_entry_path(@time_entry.project, @time_entry.issue, options)
-                           else
-                             redirect_to new_project_time_entry_path(@time_entry.project, options)
-                           end
+                         options = {
+                           :time_entry => {
+                             :project_id => params[:time_entry][:project_id],
+                             :issue_id => @time_entry.issue_id,
+                             :activity_id => @time_entry.activity_id
+                           },
+                           :back_url => params[:back_url]
+                         }
+                         if params[:project_id] && @time_entry.project
+                           redirect_to new_project_time_entry_path(@time_entry.project, options)
+                         elsif params[:issue_id] && @time_entry.issue
+                           redirect_to new_issue_time_entry_path(@time_entry.issue, options)
                          else
-                           options = {
-                             :time_entry => {:project_id => @time_entry.project_id, :issue_id => @time_entry.issue_id, :activity_id => @time_entry.activity_id},
-                             :back_url => params[:back_url]
+                           }
                            redirect_to new_time_entry_path(options)
                          end
                        else
                  end
                end
-               def find_optional_project_for_new_time_entry
-                 if (project_id = (params[:project_id] || params[:time_entry] && params[:time_entry][:project_id])).present?
-                   @project = Project.find(project_id)
-                 end
-                 if (issue_id = (params[:issue_id] || params[:time_entry] && params[:time_entry][:issue_id])).present?
-                   @issue = Issue.find(issue_id)
-                   @project ||= @issue.project
-                 end
-               rescue ActiveRecord::RecordNotFound
-                 render_404
-               end
-               def find_project_for_new_time_entry
-                 find_optional_project_for_new_time_entry
-                 if @project.nil?
-                   render_404
-                 end
-               end
                def find_optional_project
-                 if !params[:issue_id].blank?
+                 if params[:issue_id].present?
                    @issue = Issue.find(params[:issue_id])
                    @project = @issue.project
-                 elsif !params[:project_id].blank?
+                 elsif params[:project_id].present?
                    @project = Project.find(params[:project_id])
                  end
+               rescue ActiveRecord::RecordNotFound
+                 render_404
                end
                # Returns the TimeEntry scope for index and report actions

app/helpers/application_helper.rb +4 -1

                end
                def project_tree_options_for_select(projects, options = {})
-                 s = ''
+                 s = ''.html_safe
+                 if options[:include_blank]
+                   s << content_tag('option', '&nbsp;'.html_safe, :value => '')
+                 end
                  project_tree(projects) do |project, level|
                    name_prefix = (level > 0 ? '&nbsp;' * 2 * level + '&#187; ' : '').html_safe
                    tag_options = {:value => project.id}

app/models/time_entry.rb +8 -6

                belongs_to :user
                belongs_to :activity, :class_name => 'TimeEntryActivity', :foreign_key => 'activity_id'
-               attr_protected :project_id, :user_id, :tyear, :tmonth, :tweek
+               attr_protected :user_id, :tyear, :tmonth, :tweek
                acts_as_customizable
                acts_as_event :title => Proc.new {|o| "#{l_hours(o.hours)} (#{(o.issue || o.project).event_title})"},
                  end
                }
-               safe_attributes 'hours', 'comments', 'issue_id', 'activity_id', 'spent_on', 'custom_field_values', 'custom_fields'
+               safe_attributes 'hours', 'comments', 'project_id', 'issue_id', 'activity_id', 'spent_on', 'custom_field_values', 'custom_fields'
                def initialize(attributes=nil, *args)
                  super
                end
                def safe_attributes=(attrs, user=User.current)
-                 attrs = super
-                 if !new_record? && issue && issue.project_id != project_id
-                   if user.allowed_to?(:log_time, issue.project)
-                     self.project_id = issue.project_id
+                 if attrs
+                   attrs = super(attrs)
+                   if issue_id_changed? && attrs[:project_id].blank? && issue && issue.project_id != project_id
+                     if user.allowed_to?(:log_time, issue.project)
+                       self.project_id = issue.project_id
+                     end
                    end
                  end
                  attrs

app/views/issues/show.html.erb +1 -1

                  end
                end
                if User.current.allowed_to?(:view_time_entries, @project)
-                 rows.right l(:label_spent_time), (@issue.total_spent_hours > 0 ? link_to(l_hours(@issue.total_spent_hours), project_issue_time_entries_path(@project, @issue)) : "-"), :class => 'spent-time'
+                 rows.right l(:label_spent_time), (@issue.total_spent_hours > 0 ? link_to(l_hours(@issue.total_spent_hours), issue_time_entries_path(@issue)) : "-"), :class => 'spent-time'
                end
              end %>
              <%= render_custom_fields_rows(@issue) %>

app/views/timelog/_form.html.erb +5 -3

              <div class="box tabular">
                <% if @time_entry.new_record? %>
-                 <% if params[:project_id] || @time_entry.issue %>
-                   <%= f.hidden_field :project_id %>
+                 <% if params[:project_id] %>
+                   <%= hidden_field_tag 'project_id', params[:project_id] %>
+                 <% elsif params[:issue_id] %>
+                   <%= hidden_field_tag 'issue_id', params[:issue_id] %>
                  <% else %>
-                   <p><%= f.select :project_id, project_tree_options_for_select(Project.allowed_to(:log_time).all, :selected => @time_entry.project), :required => true %></p>
+                   <p><%= f.select :project_id, project_tree_options_for_select(Project.allowed_to(:log_time).all, :selected => @time_entry.project, :include_blank => true) %></p>
                  <% end %>
                <% end %>
                <p>

app/views/timelog/new.html.erb 0 -1

              <h2><%= l(:label_spent_time) %></h2>
              <%= labelled_form_for @time_entry, :url => time_entries_path do |f| %>
-               <%= hidden_field_tag 'project_id', params[:project_id] if params[:project_id] %>
                <%= render :partial => 'form', :locals => {:f => f} %>
                <%= submit_tag l(:button_create) %>
                <%= submit_tag l(:button_create_and_continue), :name => 'continue' %>

test/functional/timelog_controller_test.rb +83 -51

                include Redmine::I18n
-               def test_new_with_project_id
+               def test_new
                  @request.session[:user_id] = 3
-                 get :new, :project_id => 1
+                 get :new
                  assert_response :success
                  assert_template 'new'
-                 assert_select 'select[name=?]', 'time_entry[project_id]', 0
-                 assert_select 'input[name=?][value=1][type=hidden]', 'time_entry[project_id]'
+                 assert_select 'input[name=?][type=hidden]', 'project_id', 0
+                 assert_select 'input[name=?][type=hidden]', 'issue_id', 0
+                 assert_select 'select[name=?]', 'time_entry[project_id]' do
+                   # blank option for project
+                   assert_select 'option[value=]'
+                 end
                end
-               def test_new_with_issue_id
+               def test_new_with_project_id
                  @request.session[:user_id] = 3
-                 get :new, :issue_id => 2
+                 get :new, :project_id => 1
                  assert_response :success
                  assert_template 'new'
+                 assert_select 'input[name=?][type=hidden]', 'project_id'
+                 assert_select 'input[name=?][type=hidden]', 'issue_id', 0
                  assert_select 'select[name=?]', 'time_entry[project_id]', 0
-                 assert_select 'input[name=?][value=1][type=hidden]', 'time_entry[project_id]'
                end
-               def test_new_without_project
+               def test_new_with_issue_id
                  @request.session[:user_id] = 3
-                 get :new
+                 get :new, :issue_id => 2
                  assert_response :success
                  assert_template 'new'
-                 assert_select 'select[name=?]', 'time_entry[project_id]'
-                 assert_select 'input[name=?]', 'time_entry[project_id]', 0
+                 assert_select 'input[name=?][type=hidden]', 'project_id', 0
+                 assert_select 'input[name=?][type=hidden]', 'issue_id'
+                 assert_select 'select[name=?]', 'time_entry[project_id]', 0
                end
                def test_new_without_project_should_prefill_the_form
                  assert_select 'select[name=?]', 'time_entry[project_id]' do
                    assert_select 'option[value=1][selected=selected]'
                  end
-                 assert_select 'input[name=?]', 'time_entry[project_id]', 0
                end
                def test_new_without_project_should_deny_without_permission
                end
                def test_post_create
-                 # TODO: should POST to issues’ time log instead of project. change form
-                 # and routing
                  @request.session[:user_id] = 3
-                 post :create, :project_id => 1,
+                 assert_difference 'TimeEntry.count' do
+                   post :create, :project_id => 1,
                              :time_entry => {:comments => 'Some work on TimelogControllerTest',
                                              # Not the default activity
                                              :activity_id => '11',
                                              :spent_on => '2008-03-14',
                                              :issue_id => '1',
                                              :hours => '7.3'}
-                 assert_redirected_to :action => 'index', :project_id => 'ecookbook'
+                   assert_redirected_to '/projects/ecookbook/time_entries'
+                 end
-                 i = Issue.find(1)
-                 t = TimeEntry.find_by_comments('Some work on TimelogControllerTest')
+                 t = TimeEntry.order('id DESC').first
                  assert_not_nil t
+                 assert_equal 'Some work on TimelogControllerTest', t.comments
+                 assert_equal 1, t.project_id
+                 assert_equal 1, t.issue_id
                  assert_equal 11, t.activity_id
                  assert_equal 7.3, t.hours
                  assert_equal 3, t.user_id
-                 assert_equal i, t.issue
-                 assert_equal i.project, t.project
                end
                def test_post_create_with_blank_issue
-                 # TODO: should POST to issues’ time log instead of project. change form
-                 # and routing
                  @request.session[:user_id] = 3
-                 post :create, :project_id => 1,
+                 assert_difference 'TimeEntry.count' do
+                   post :create, :project_id => 1,
                              :time_entry => {:comments => 'Some work on TimelogControllerTest',
                                              # Not the default activity
                                              :activity_id => '11',
                                              :issue_id => '',
                                              :spent_on => '2008-03-14',
                                              :hours => '7.3'}
-                 assert_redirected_to :action => 'index', :project_id => 'ecookbook'
+                   assert_redirected_to '/projects/ecookbook/time_entries'
+                 end
-                 t = TimeEntry.find_by_comments('Some work on TimelogControllerTest')
+                 t = TimeEntry.order('id DESC').first
                  assert_not_nil t
+                 assert_equal 'Some work on TimelogControllerTest', t.comments
+                 assert_equal 1, t.project_id
+                 assert_nil t.issue_id
                  assert_equal 11, t.activity_id
                  assert_equal 7.3, t.hours
                  assert_equal 3, t.user_id
                end
-               def test_create_and_continue
+               def test_create_and_continue_at_project_level
                  @request.session[:user_id] = 2
-                 post :create, :project_id => 1,
-                             :time_entry => {:activity_id => '11',
-                                             :issue_id => '',
-                                             :spent_on => '2008-03-14',
-                                             :hours => '7.3'},
-                             :continue => '1'
-                 assert_redirected_to '/projects/ecookbook/time_entries/new?time_entry%5Bactivity_id%5D=11&time_entry%5Bissue_id%5D='
+                 assert_difference 'TimeEntry.count' do
+                   post :create, :time_entry => {:project_id => '1',
+                                                 :activity_id => '11',
+                                                 :issue_id => '',
+                                                 :spent_on => '2008-03-14',
+                                                 :hours => '7.3'},
+                                 :continue => '1'
+                   assert_redirected_to '/time_entries/new?time_entry%5Bactivity_id%5D=11&time_entry%5Bissue_id%5D=&time_entry%5Bproject_id%5D=1'
+                 end
                end
-               def test_create_and_continue_with_issue_id
+               def test_create_and_continue_at_issue_level
                  @request.session[:user_id] = 2
-                 post :create, :project_id => 1,
-                             :time_entry => {:activity_id => '11',
-                                             :issue_id => '1',
-                                             :spent_on => '2008-03-14',
-                                             :hours => '7.3'},
-                             :continue => '1'
-                 assert_redirected_to '/projects/ecookbook/issues/1/time_entries/new?time_entry%5Bactivity_id%5D=11&time_entry%5Bissue_id%5D=1'
+                 assert_difference 'TimeEntry.count' do
+                   post :create, :time_entry => {:project_id => '',
+                                                 :activity_id => '11',
+                                                 :issue_id => '1',
+                                                 :spent_on => '2008-03-14',
+                                                 :hours => '7.3'},
+                                 :continue => '1'
+                   assert_redirected_to '/time_entries/new?time_entry%5Bactivity_id%5D=11&time_entry%5Bissue_id%5D=1&time_entry%5Bproject_id%5D='
+                 end
                end
-               def test_create_and_continue_without_project
+               def test_create_and_continue_with_project_id
                  @request.session[:user_id] = 2
-                 post :create, :time_entry => {:project_id => '1',
-                                             :activity_id => '11',
-                                             :issue_id => '',
-                                             :spent_on => '2008-03-14',
-                                             :hours => '7.3'},
-                               :continue => '1'
+                 assert_difference 'TimeEntry.count' do
+                   post :create, :project_id => 1,
+                                 :time_entry => {:activity_id => '11',
+                                                 :issue_id => '',
+                                                 :spent_on => '2008-03-14',
+                                                 :hours => '7.3'},
+                                 :continue => '1'
+                   assert_redirected_to '/projects/ecookbook/time_entries/new?time_entry%5Bactivity_id%5D=11&time_entry%5Bissue_id%5D=&time_entry%5Bproject_id%5D='
+                 end
+               end
-                 assert_redirected_to '/time_entries/new?time_entry%5Bactivity_id%5D=11&time_entry%5Bissue_id%5D=&time_entry%5Bproject_id%5D=1'
+               def test_create_and_continue_with_issue_id
+                 @request.session[:user_id] = 2
+                 assert_difference 'TimeEntry.count' do
+                   post :create, :issue_id => 1,
+                                 :time_entry => {:activity_id => '11',
+                                                 :issue_id => '1',
+                                                 :spent_on => '2008-03-14',
+                                                 :hours => '7.3'},
+                                 :continue => '1'
+                   assert_redirected_to '/issues/1/time_entries/new?time_entry%5Bactivity_id%5D=11&time_entry%5Bissue_id%5D=1&time_entry%5Bproject_id%5D='
+                 end
                end
                def test_create_without_log_time_permission_should_be_denied
                  assert_response 403
                end
+               def test_create_without_project_and_issue_should_fail
+                 @request.session[:user_id] = 2
+                 post :create, :time_entry => {:issue_id => ''}
+                 assert_response :success
+                 assert_template 'new'
+               end
                def test_create_with_failure
                  @request.session[:user_id] = 2
                  post :create, :project_id => 1,
                  # display all time
                  assert_nil assigns(:from)
                  assert_nil assigns(:to)
-                 # TODO: remove /projects/:project_id/issues/:issue_id/time_entries routes
-                 # to use /issues/:issue_id/time_entries
                  assert_tag :form,
                    :attributes => {:action => "/projects/ecookbook/issues/1/time_entries", :id => 'query_form'}
                end

General Comments 0

Write
Preview

You need to be logged in to leave comments. Login now

No TODOs yet

	Site-wide shortcuts
/	Use quick search box
g h	Goto home page
g g	Goto my private gists page
g G	Goto my public gists page
g 0-9	Goto bookmarked items from 0-9
n r	New repository page
n g	New gist page

	Repositories
g s	Goto summary page
g c	Goto changelog page
g f	Goto files page
g F	Goto files page with file search activated
g p	Goto pull requests page
g o	Goto repository settings
g O	Goto repository access permissions settings
t s	Toggle sidebar on some pages