Skip to content

Commit

Permalink
Refactor: extract TimelogController#new from #edit
Browse files Browse the repository at this point in the history
git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@4239 e93f8b46-1217-0410-a6f0-8f06a7374b81
  • Loading branch information
edavis10 committed Oct 7, 2010
1 parent e59156b commit 068771e
Show file tree
Hide file tree
Showing 7 changed files with 31 additions and 22 deletions.
10 changes: 9 additions & 1 deletion app/controllers/timelog_controller.rb
Expand Up @@ -17,7 +17,7 @@

class TimelogController < ApplicationController
menu_item :issues
before_filter :find_project, :authorize, :only => [:edit, :destroy]
before_filter :find_project, :authorize, :only => [:new, :edit, :destroy]
before_filter :find_optional_project, :only => [:index]

verify :method => :post, :only => :destroy, :redirect_to => { :action => :index }
Expand Down Expand Up @@ -85,6 +85,14 @@ def index
end
end
end

def new
@time_entry ||= TimeEntry.new(:project => @project, :issue => @issue, :user => User.current, :spent_on => User.current.today)
@time_entry.attributes = params[:time_entry]

call_hook(:controller_timelog_edit_before_save, { :params => params, :time_entry => @time_entry })
render :action => 'edit'
end

def edit
(render_403; return) if @time_entry && !@time_entry.editable_by?(User.current)
Expand Down
2 changes: 1 addition & 1 deletion app/views/time_entry_reports/report.rhtml
@@ -1,5 +1,5 @@
<div class="contextual">
<%= link_to_if_authorized l(:button_log_time), {:controller => 'timelog', :action => 'edit', :project_id => @project, :issue_id => @issue}, :class => 'icon icon-time-add' %>
<%= link_to_if_authorized l(:button_log_time), {:controller => 'timelog', :action => 'new', :project_id => @project, :issue_id => @issue}, :class => 'icon icon-time-add' %>
</div>

<%= render_timelog_breadcrumb %>
Expand Down
2 changes: 1 addition & 1 deletion app/views/timelog/index.html.erb
@@ -1,5 +1,5 @@
<div class="contextual">
<%= link_to_if_authorized l(:button_log_time), {:controller => 'timelog', :action => 'edit', :project_id => @project, :issue_id => @issue}, :class => 'icon icon-time-add' %>
<%= link_to_if_authorized l(:button_log_time), {:controller => 'timelog', :action => 'new', :project_id => @project, :issue_id => @issue}, :class => 'icon icon-time-add' %>
</div>

<%= render_timelog_breadcrumb %>
Expand Down
6 changes: 3 additions & 3 deletions config/routes.rb
Expand Up @@ -15,8 +15,8 @@
map.connect 'help/:ctrl/:page', :controller => 'help'

map.connect 'time_entries/:id/edit', :action => 'edit', :controller => 'timelog'
map.connect 'projects/:project_id/time_entries/new', :action => 'edit', :controller => 'timelog'
map.connect 'projects/:project_id/issues/:issue_id/time_entries/new', :action => 'edit', :controller => 'timelog'
map.connect 'projects/:project_id/time_entries/new', :action => 'new', :controller => 'timelog'
map.connect 'projects/:project_id/issues/:issue_id/time_entries/new', :action => 'new', :controller => 'timelog'

map.with_options :controller => 'timelog' do |timelog|
timelog.connect 'projects/:project_id/time_entries', :action => 'index'
Expand All @@ -37,7 +37,7 @@
time_report.connect 'projects/:project_id/time_entries/report.:format'
end

timelog.with_options :action => 'edit', :conditions => {:method => :get} do |time_edit|
timelog.with_options :action => 'new', :conditions => {:method => :get} do |time_edit|
time_edit.connect 'issues/:issue_id/time_entries/new'
end

Expand Down
4 changes: 2 additions & 2 deletions lib/redmine.rb
Expand Up @@ -86,8 +86,8 @@
map.project_module :time_tracking do |map|
map.permission :log_time, {:timelog => :edit}, :require => :loggedin
map.permission :view_time_entries, :timelog => [:index], :time_entry_reports => [:report]
map.permission :edit_time_entries, {:timelog => [:edit, :destroy]}, :require => :member
map.permission :edit_own_time_entries, {:timelog => [:edit, :destroy]}, :require => :loggedin
map.permission :edit_time_entries, {:timelog => [:new, :edit, :destroy]}, :require => :member
map.permission :edit_own_time_entries, {:timelog => [:new, :edit, :destroy]}, :require => :loggedin
map.permission :manage_project_activities, {:project_enumerations => [:update, :destroy]}, :require => :member
end

Expand Down
22 changes: 11 additions & 11 deletions test/functional/timelog_controller_test.rb
Expand Up @@ -31,16 +31,25 @@ def setup
@response = ActionController::TestResponse.new
end

def test_get_edit
def test_get_new
@request.session[:user_id] = 3
get :edit, :project_id => 1
get :new, :project_id => 1
assert_response :success
assert_template 'edit'
# Default activity selected
assert_tag :tag => 'option', :attributes => { :selected => 'selected' },
:content => 'Development'
end

def test_get_new_should_only_show_active_time_entry_activities
@request.session[:user_id] = 3
get :new, :project_id => 1
assert_response :success
assert_template 'edit'
assert_no_tag :tag => 'option', :content => 'Inactive Activity'

end

def test_get_edit_existing_time
@request.session[:user_id] = 2
get :edit, :id => 2, :project_id => nil
Expand All @@ -50,15 +59,6 @@ def test_get_edit_existing_time
assert_tag :tag => 'form', :attributes => { :action => '/projects/ecookbook/timelog/edit/2' }
end

def test_get_edit_should_only_show_active_time_entry_activities
@request.session[:user_id] = 3
get :edit, :project_id => 1
assert_response :success
assert_template 'edit'
assert_no_tag :tag => 'option', :content => 'Inactive Activity'

end

def test_get_edit_with_an_existing_time_entry_with_inactive_activity
te = TimeEntry.find(1)
te.activity = TimeEntryActivity.find_by_name("Inactive Activity")
Expand Down
7 changes: 4 additions & 3 deletions test/integration/routing_test.rb
Expand Up @@ -233,11 +233,12 @@ class RoutingTest < ActionController::IntegrationTest
should_route :get, "/issues/234/time_entries.atom", :controller => 'timelog', :action => 'index', :issue_id => '234', :format => 'atom'
should_route :get, "/projects/ecookbook/issues/123/time_entries", :controller => 'timelog', :action => 'index', :project_id => 'ecookbook', :issue_id => '123'

should_route :get, "/issues/567/time_entries/new", :controller => 'timelog', :action => 'edit', :issue_id => '567'
should_route :get, "/projects/ecookbook/time_entries/new", :controller => 'timelog', :action => 'edit', :project_id => 'ecookbook'
should_route :get, "/projects/ecookbook/issues/567/time_entries/new", :controller => 'timelog', :action => 'edit', :project_id => 'ecookbook', :issue_id => '567'
should_route :get, "/issues/567/time_entries/new", :controller => 'timelog', :action => 'new', :issue_id => '567'
should_route :get, "/projects/ecookbook/time_entries/new", :controller => 'timelog', :action => 'new', :project_id => 'ecookbook'
should_route :get, "/projects/ecookbook/issues/567/time_entries/new", :controller => 'timelog', :action => 'new', :project_id => 'ecookbook', :issue_id => '567'
should_route :get, "/time_entries/22/edit", :controller => 'timelog', :action => 'edit', :id => '22'

should_route :post, "/time_entries/22/edit", :controller => 'timelog', :action => 'edit', :id => '22'
should_route :post, "/time_entries/55/destroy", :controller => 'timelog', :action => 'destroy', :id => '55'
end

Expand Down

4 comments on commit 068771e

@gravis
Copy link

@gravis gravis commented on 068771e Oct 8, 2010

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Timelog is broken for me since this commit. The "log time" link is display an edit page, with a pre-filled value.
I think you forgot to update app/views/issues/_action_menu.rhtml ?

<%= link_to_if_authorized l(:button_log_time), {:controller => 'timelog', :action => 'edit', :issue_id => @issue}, :class => 'icon icon-time-add' %>

to
<%= link_to_if_authorized l(:button_log_time), {:controller => 'timelog', :action => 'new', :issue_id => @issue}, :class => 'icon icon-time-add' %>

@edavis10
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, thanks for catching that.

@gravis
Copy link

@gravis gravis commented on 068771e Oct 8, 2010

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you're welcome. Thank you for your great work on redmine, it's really appreciated :)

@edavis10
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 84ebd78

Please sign in to comment.