From acd42316934ce76782db64acb4c7d01dd83f9c4f Mon Sep 17 00:00:00 2001 From: Timothy High Date: Mon, 25 Jan 2010 21:08:46 -0200 Subject: [PATCH] #595 - Added watch list form to AD forms; rolled back changes from #593 and #594 --- README | 1 - .../arch_decision_discussions_controller.rb | 20 ----------- app/controllers/arch_decisions_controller.rb | 14 -------- app/helpers/arch_decisions_helper.rb | 4 --- .../arch_decisions/.tmp__form.html.erb.32347~ | 0 .../.tmp__arch_decisions.html.erb.60982~ | 19 ----------- app/views/settings/_arch_decisions.html.erb | 29 ---------------- init.rb | 13 -------- .../arch_decisions_controller_test.rb | 23 +++++++------ ...h_decisions_discussions_controller_test.rb | 33 ------------------- 10 files changed, 11 insertions(+), 145 deletions(-) create mode 100644 app/views/arch_decisions/.tmp__form.html.erb.32347~ delete mode 100644 app/views/settings/.tmp__arch_decisions.html.erb.60982~ delete mode 100644 app/views/settings/_arch_decisions.html.erb diff --git a/README b/README index 94e161f..d9d5de3 100644 --- a/README +++ b/README @@ -11,7 +11,6 @@ RELEASE NOTES * Feature #582 - Enable security controls on the Arch Decisions features - Basic permissions are now required in order to perform actions other than view the entries * Feature #592 - Users should have view permissions in order to see ADs and related data - Permissions are now required to view AD data * Feature #581 - Factors should have a "scope" attribute - Factors can now be restricted to a single project, or single AD. Existing Factors will be updated according to current assignments. -* Feature #594 - Users automatically added to watch list on creation, assignment or comments * Bug #557 - Fixed problem with unwanted link activation when dragging a Factor for reprioritization CURRENT FEATURES diff --git a/app/controllers/arch_decision_discussions_controller.rb b/app/controllers/arch_decision_discussions_controller.rb index ce1265f..217eb2d 100644 --- a/app/controllers/arch_decision_discussions_controller.rb +++ b/app/controllers/arch_decision_discussions_controller.rb @@ -23,13 +23,6 @@ def new @discussion.created_by = User.current if @discussion.save attach_files(@discussion, params[:attachments]) - if automatically_add_watchers - wad = ad_for_watch_list @discussion - unless wad.nil? - wad.set_watcher(User.current, true) - wad.save - end - end Mailer.deliver_arch_decision_discussion_add(@discussion) flash[:notice] = l(:notice_successful_create) else @@ -125,17 +118,4 @@ def get_id_from_params(type) params[type + "_id"] ? params[type + "_id"] : (params[:discussion] ? params[:discussion][type + "_id"] : nil) end - def ad_for_watch_list(discussion) - case discussion.parent_type - when "Arch Decision" then discussion.arch_decision - when "Strategy" then discussion.parent.arch_decision - when "Factor" then - if discussion.factor.scope == Factor::SCOPE_ARCH_DECISION - discussion.factor.arch_decisions[0] - else - nil - end - else nil - end - end end diff --git a/app/controllers/arch_decisions_controller.rb b/app/controllers/arch_decisions_controller.rb index a90a4f9..1f43254 100755 --- a/app/controllers/arch_decisions_controller.rb +++ b/app/controllers/arch_decisions_controller.rb @@ -50,15 +50,10 @@ def new @arch_decision = ArchDecision.new(params[:arch_decision]) @arch_decision.project = @project @arch_decision_statuses = ArchDecisionStatus.find(:all) - @arch_decision.watcher_user_ids = [User.current.id] if request.post? @arch_decision.created_by = User.current @arch_decision.updated_by = User.current update_watch_list -# @arch_decision.set_watcher @arch_decision.created_by, true #TODO: make this optional by putting it in the form -# if (@arch_decision.assigned_to) -# @arch_decision.set_watcher(@arch_decision.assigned_to, true) -# end if @arch_decision.save Mailer.deliver_arch_decision_add(@arch_decision) flash[:notice] = l(:notice_successful_create) @@ -71,11 +66,6 @@ def edit if request.post? update_watch_list if @arch_decision.update_attributes(params[:arch_decision]) -# if (@arch_decision.watchers.select{|w| w.user == @arch_decision.assigned_to}.empty?) -# #Add new assignee to watch list -# @arch_decision.set_watcher(@arch_decision.assigned_to, true) -# @arch_decision.save! -# end Mailer.deliver_arch_decision_edit(@arch_decision) flash[:notice] = l(:notice_successful_update) redirect_to :action => 'show', :project_id => @project, :id => @arch_decision @@ -184,10 +174,6 @@ def refresh_factors_table def update_watch_list wids = params[:arch_decision]['watcher_user_ids'] wids = wids.nil? ? [] : wids - if automatically_add_watchers - wids = wids << params[:arch_decision]['assigned_to_id'].to_i unless params[:arch_decision]['assigned_to_id'].to_i == 0 - wids.uniq! - end @arch_decision.watcher_user_ids = wids end end diff --git a/app/helpers/arch_decisions_helper.rb b/app/helpers/arch_decisions_helper.rb index 6bec193..6a0ec3b 100755 --- a/app/helpers/arch_decisions_helper.rb +++ b/app/helpers/arch_decisions_helper.rb @@ -28,8 +28,4 @@ def text_for_discussion_count(item) return "(" + l(label, item.arch_decision_discussions.size.to_s) + ")" end - def automatically_add_watchers - Setting["plugin_redmine_arch_decisions"]["automatically_add_watchers"] - end - end diff --git a/app/views/arch_decisions/.tmp__form.html.erb.32347~ b/app/views/arch_decisions/.tmp__form.html.erb.32347~ new file mode 100644 index 0000000..e69de29 diff --git a/app/views/settings/.tmp__arch_decisions.html.erb.60982~ b/app/views/settings/.tmp__arch_decisions.html.erb.60982~ deleted file mode 100644 index 8547377..0000000 --- a/app/views/settings/.tmp__arch_decisions.html.erb.60982~ +++ /dev/null @@ -1,19 +0,0 @@ -<% form_tag({:action => 'plugin'}) do %> - -
<%=l(:text_select_mail_notifications)%> - - <%= check_box_tag "settings[notify_on_new_ad]", Setting["plugin_redmine_arch_decisions"]["notify_on_new_ad"], Setting["plugin_redmine_arch_decisions"]["notify_on_new_ad"] %> - Creation of a new Arch Decision -
-<% ArchDecisionStatus.all.each do |status| %> - - <% param_name = "notify_on_#{status.name.downcase.gsub(' ', '_')}" %> - <%= check_box_tag "settings[#{param_name}]", Setting["plugin_redmine_arch_decisions"][param_name], Setting["plugin_redmine_arch_decisions"][param_name] %> - <%= status.name %> -
-<% end %> -<%= hidden_field_tag 'settings[notified_events][]', '' %> -

<%= check_all_links('notified_events') %>

-
- -<% end %> diff --git a/app/views/settings/_arch_decisions.html.erb b/app/views/settings/_arch_decisions.html.erb deleted file mode 100644 index b725cb2..0000000 --- a/app/views/settings/_arch_decisions.html.erb +++ /dev/null @@ -1,29 +0,0 @@ -<% form_tag({:action => 'plugin'}) do %> - -
<%=l(:text_select_mail_notifications)%> - - <%= check_box_tag "settings[notify_on_new_ad]", Setting["plugin_redmine_arch_decisions"]["notify_on_new_ad"], Setting["plugin_redmine_arch_decisions"]["notify_on_new_ad"] %> - Creation of a new Arch Decision -
-
- -
Status changes which should trigger a broadcast notification -<% ArchDecisionStatus.all.each do |status| %> - - <% param_name = "notify_on_#{status.name.downcase.gsub(' ', '_')}" %> - <%= check_box_tag "settings[#{param_name}]", Setting["plugin_redmine_arch_decisions"][param_name], Setting["plugin_redmine_arch_decisions"][param_name] %> - <%= status.name %> -
-<% end %> -<%= hidden_field_tag 'settings[notified_events][]', '' %> -

<%= check_all_links('notified_events') %>

-
- -
Other configurations - - <%= check_box_tag "settings[automatically_add_watchers]", Setting["plugin_redmine_arch_decisions"]["automatically_add_watchers"], Setting["plugin_redmine_arch_decisions"]["automatically_add_watchers"] %> - Automatically add creators, assignees and commenters to watch list -
-
- -<% end %> diff --git a/init.rb b/init.rb index 7f972e1..1e84500 100755 --- a/init.rb +++ b/init.rb @@ -32,17 +32,4 @@ menu :project_menu, :arch_decisions, { :controller => 'arch_decisions', :action => 'index' }, :caption => :label_arch_decision_plural, :param => :project_id menu :project_menu, :factors, { :controller => 'factors', :action => 'index' }, :caption => :label_factor_plural, :param => :project_id - # This plugin contains settings - settings :default => { - 'automatically_add_watchers' => true, - 'notify_on_new_ad' => true, - 'notify_on_not_started' => false, - 'notify_on_under_discussion' => false, - 'notify_on_decision_made' => true, - 'notify_on_work_scheduled' => false, - 'notify_on_implemented' => false, - 'notify_on_canceled' => false, - 'notify_on_deprecated' => false - }, :partial => 'settings/arch_decisions' - end diff --git a/test/functional/arch_decisions_controller_test.rb b/test/functional/arch_decisions_controller_test.rb index a8424a2..e291834 100755 --- a/test/functional/arch_decisions_controller_test.rb +++ b/test/functional/arch_decisions_controller_test.rb @@ -260,6 +260,7 @@ def test_new assert_select 'select#arch_decision_status_id' assert_select 'select#arch_decision_assigned_to_id' end + # TODO: test watchers form end def test_new_post @@ -271,7 +272,7 @@ def test_new_post :resolution => "test_new_post resolution", :status_id => arch_decision_statuses(:valid).id, :assigned_to_id => users(:users_001).id, - :watcher_user_ids => [User.current.id] + :watcher_user_ids => [User.current.id, users(:users_001).id] } assert_equal ad_count+1, ArchDecision.count(:all) ad = assigns(:arch_decision) @@ -280,7 +281,7 @@ def test_new_post assert_equal "test_new_post resolution", ad.resolution assert_equal arch_decision_statuses(:valid), ad.status assert_equal users(:users_001), ad.assigned_to - # Make sure watchers are automatically assigned + # Make sure watchers are assigned assert_equal 2, ad.watchers.size assert ad.watchers.collect{ |w| w.user }.include?(ad.created_by) assert ad.watchers.collect{ |w| w.user }.include?(ad.assigned_to) @@ -294,19 +295,20 @@ def test_new_post_one_watcher :problem_description => "test_new_post problem_description", :resolution => "test_new_post resolution", :status_id => arch_decision_statuses(:valid).id, - :assigned_to_id => User.current.id + :assigned_to_id => User.current.id, + :watcher_user_ids => [users(:users_001).id] } assert_equal ad_count+1, ArchDecision.count(:all) ad = assigns(:arch_decision) assert_equal ad.created_by, ad.assigned_to # There should only be one watcher assert_equal 1, ad.watchers.size - assert ad.watchers.collect{ |w| w.user }.include?(ad.created_by) - assert ad.watchers.collect{ |w| w.user }.include?(ad.assigned_to) + assert !ad.watchers.collect{ |w| w.user }.include?(ad.created_by) + assert !ad.watchers.collect{ |w| w.user }.include?(ad.assigned_to) + assert ad.watchers.collect{ |w| w.user }.include?(users(:users_001)) end def test_new_post_dont_add_watchers - Setting["plugin_redmine_arch_decisions"]["automatically_add_watchers"] = false ad_count = ArchDecision.count(:all) post :new, :project_id => @ad.project.id, :arch_decision => { @@ -321,7 +323,6 @@ def test_new_post_dont_add_watchers assert_equal ad.created_by, ad.assigned_to # There should be no watchers despite assigned to value assert_equal 0, ad.watchers.size - Setting["plugin_redmine_arch_decisions"]["automatically_add_watchers"] = true end def test_new_no_perms @@ -341,6 +342,7 @@ def test_edit assert_select 'select#arch_decision_status_id' assert_select 'select#arch_decision_assigned_to_id' end + # TODO: assert watchers form end def test_edit_post @@ -363,11 +365,10 @@ def test_edit_post assert_equal "test_edit_post resolution", ad.resolution assert_equal arch_decision_statuses(:valid_name_max_length), ad.status assert_equal assignee, ad.assigned_to - # Make sure new assignee is added to watch list - assert_equal 3, ad.watchers.size + assert_equal 2, ad.watchers.size watching_users = ad.watchers.collect{ |w| w.user } assert watching_users.include?(ad.created_by) - assert watching_users.include?(ad.assigned_to) + assert !watching_users.include?(ad.assigned_to) assert watching_users.include?(old_assignee) end @@ -393,7 +394,6 @@ def test_edit_post_assigned_to_unchanged end def test_edit_post_dont_add_watchers - Setting["plugin_redmine_arch_decisions"]["automatically_add_watchers"] = false assignee = users(:users_002) post :edit, :project_id => @ad.project.id, :id => @ad.id, @@ -412,7 +412,6 @@ def test_edit_post_dont_add_watchers watching_users = ad.watchers.collect{ |w| w.user } assert watching_users.include?(ad.created_by) assert !watching_users.include?(ad.assigned_to) - Setting["plugin_redmine_arch_decisions"]["automatically_add_watchers"] = true end diff --git a/test/functional/arch_decisions_discussions_controller_test.rb b/test/functional/arch_decisions_discussions_controller_test.rb index 7e59c4d..4a5ef68 100644 --- a/test/functional/arch_decisions_discussions_controller_test.rb +++ b/test/functional/arch_decisions_discussions_controller_test.rb @@ -27,7 +27,6 @@ def setup def test_new_ad_discussion expected_counts = get_counts - assert !@ad.watchers.collect{|w| w.user}.include?(User.current), "User shouldn't be on AD watch list yet" post :new, :project_id => 1, :arch_decision_id => @ad.id, @@ -37,32 +36,11 @@ def test_new_ad_discussion assert_not_nil assigns(:arch_decision) add_counts expected_counts, :arch_decisions, 1 assert_counts expected_counts - @ad.reload - assert @ad.watchers.collect{|w| w.user}.include?(User.current), "After comment, user should be on watch list" - end - - def test_new_ad_discussion_dont_add_watchers - Setting["plugin_redmine_arch_decisions"]["automatically_add_watchers"] = false - expected_counts = get_counts - assert !@ad.watchers.collect{|w| w.user}.include?(User.current), "User shouldn't be on AD watch list yet" - post :new, - :project_id => 1, - :arch_decision_id => @ad.id, - :discussion => {:subject => "New discussion", :content => "New content"} - assert_redirected_to :controller => "arch_decisions", :action => "show" - #assert_equal 'Post was successfully created.', flash[:notice] assert_not_nil assigns(:project) - assert_not_nil assigns(:arch_decision) - add_counts expected_counts, :arch_decisions, 1 - assert_counts expected_counts - @ad.reload - assert !@ad.watchers.collect{|w| w.user}.include?(User.current), "After comment, user still shouldn't be on watch list" - Setting["plugin_redmine_arch_decisions"]["automatically_add_watchers"] = true end def test_new_factor_discussion expected_counts = get_counts factor = factors(:valid) - assert !@ad.watchers.collect{|w| w.user}.include?(User.current), "User shouldn't be on AD watch list yet" post :new, :project_id => 1, :factor_id => factor.id, @@ -72,14 +50,11 @@ def test_new_factor_discussion assert_not_nil assigns(:factor) add_counts expected_counts, :factors, 1 assert_counts expected_counts - @ad.reload - assert !@ad.watchers.collect{|w| w.user}.include?(User.current), "After comment, user still shouldn't be on watch list" end def test_new_ad_factor_discussion expected_counts = get_counts factor = factors(:valid_2) - assert !@ad.watchers.collect{|w| w.user}.include?(User.current), "User shouldn't be on AD watch list yet" post :new, :project_id => 1, :factor_id => factor.id, @@ -89,14 +64,11 @@ def test_new_ad_factor_discussion assert_not_nil assigns(:factor) add_counts expected_counts, :factors, 1 assert_counts expected_counts - @ad.reload - assert @ad.watchers.collect{|w| w.user}.include?(User.current), "After comment, user should be on watch list" end def test_new_ad_factor_discussion_no_ad expected_counts = get_counts factor = factors(:valid_ad_no_ad) - assert !@ad.watchers.collect{|w| w.user}.include?(User.current), "User shouldn't be on AD watch list yet" post :new, :project_id => 1, :factor_id => factor.id, @@ -106,14 +78,11 @@ def test_new_ad_factor_discussion_no_ad assert_not_nil assigns(:factor) add_counts expected_counts, :factors, 1 assert_counts expected_counts - @ad.reload - assert !@ad.watchers.collect{|w| w.user}.include?(User.current), "After comment, user still shouldn't be on watch list" end def test_new_strategy_discussion expected_counts = get_counts strategy = strategies(:valid) - assert !@ad.watchers.collect{|w| w.user}.include?(User.current), "User shouldn't be on AD watch list yet" post :new, :project_id => 1, :arch_decision_id => @ad.id, @@ -124,8 +93,6 @@ def test_new_strategy_discussion assert_not_nil assigns(:strategy) add_counts expected_counts, :strategies, 1 assert_counts expected_counts - @ad.reload - assert @ad.watchers.collect{|w| w.user}.include?(User.current), "After comment, user should be on watch list" end def test_new_ad_discussion_no_perms