alphanodes / additionals

Redmine plugin for easy customization of settings, text and content display by using personal or role-based dashboards (drag&drop), providing wiki macros and act as library for other plugins.
https://www.redmine.org/plugins/additionals
GNU General Public License v2.0
131 stars 43 forks source link

Changing status from sidebar should fire controller_issues_edit_after_save event #127

Open serpi90 opened 2 years ago

serpi90 commented 2 years ago

Event listeners triggered by controller_issues_edit_after_save are not being fired when status changes from the sidebar.

alexandermeindl commented 2 years ago

Thanks for reporting this. I added support with my last commit.

alexandermeindl commented 2 years ago

I revert the support for controller_issues_edit_after_save hook. The problem with it is, that some plugin developer use a before_action in controller instead of controller_issues_edit_before_save to prepare data for save. I see no possibility which does not break this plugins.

Instead of controller_issues_edit_after_save hook, I introduced for new hooks for issue changes by additionals:

Of course this means, that other plugins have to include this hooks to detect issue changes. But at least now there is a possibility to do it.

serpi90 commented 2 years ago

That's a shame, i can use the new hook, but imho this would require collaboration with the offending plugin to solve the issue to be properly fixed.

Thank you

alexandermeindl commented 2 years ago

One of them is RedmineUp with https://www.redmine.org/plugins/redmine_checklists. It would be great to teach them, but in the past I never succeeded.

serpi90 commented 2 years ago

Oh :hankey: I'm using that plugin. I could try making a patch and uploading it to them as an issue, but: 1. my experience ruby is very limited. 2 I think they won't listen to me.

If i do that and they listen, I'll update this thread.

In the meantime, i'll patch the webhooks plugin to use your hook, which is easier.

alexandermeindl commented 2 years ago

Hi @serpi90,

the patch is trivial, I created it for you. It would be great, if "they" listen to you :smirk:

I cannot add a patch file here, but here is the code:

diff --git a/lib/redmine_checklists/hooks/controller_issues_hook.rb b/lib/redmine_checklists/hooks/controller_issues_hook.rb
index 25a4f15..4f0445b 100644
--- a/lib/redmine_checklists/hooks/controller_issues_hook.rb
+++ b/lib/redmine_checklists/hooks/controller_issues_hook.rb
@@ -20,7 +20,20 @@
 module RedmineChecklists
   module Hooks
     class ControllerIssuesHook < Redmine::Hook::ViewListener
-      def controller_issues_edit_after_save(context = {})
+      def controller_issues_edit_before_save(context = {})
+        issue = context[:issue]
+        params = context[:params]
+        issue.old_checklists = issue.checklists.to_json
+        checklists_params = params[:issue].present? && params[:issue][:checklists_attributes].present? ? params[:issue][:checklists_attributes] : {}
+        issue.removed_checklist_ids =
+          if checklists_params.present?
+            checklists_params = checklists_params.respond_to?(:to_unsafe_hash) ? checklists_params.to_unsafe_hash : checklists_params
+            checklists_params.map { |_k, v| v['id'].to_i if ['1', 'true'].include?(v['_destroy']) }.compact
+          else
+            []
+          end
+        end
+
+        def controller_issues_edit_after_save(context = {})
         old_checklists = context[:issue].old_checklists
         new_checklists = context[:issue].checklists.to_json
         journal = context[:journal]
diff --git a/lib/redmine_checklists/patches/issues_controller_patch.rb b/lib/redmine_checklists/patches/issues_controller_patch.rb
index c48a8a3..a1ee1a2 100644
--- a/lib/redmine_checklists/patches/issues_controller_patch.rb
+++ b/lib/redmine_checklists/patches/issues_controller_patch.rb
@@ -28,7 +28,6 @@ module RedmineChecklists

           alias_method :build_new_issue_from_params_without_checklist, :build_new_issue_from_params
           alias_method :build_new_issue_from_params, :build_new_issue_from_params_with_checklist
-          before_action :save_before_state, :only => [:update]
         end
       end

@@ -50,18 +49,6 @@ module RedmineChecklists
           @issue.checklists_from_params = true
         end

-        def save_before_state
-          @issue.old_checklists = @issue.checklists.to_json
-          checklists_params = params[:issue].present? && params[:issue][:checklists_attributes].present? ? params[:issue][:checklists_attributes] : {}
-          @issue.removed_checklist_ids =
-            if checklists_params.present?
-              checklists_params = checklists_params.respond_to?(:to_unsafe_hash) ? checklists_params.to_unsafe_hash : checklists_params
-              checklists_params.map { |_k, v| v['id'].to_i if ['1', 'true'].include?(v['_destroy']) }.compact
-            else
-              []
-            end
-        end
-
         def fill_checklist_attributes
           return unless params[:issue].blank?