redmica / redmine_issues_panel

36 stars 14 forks source link

It's too better not to ignore hooks on updating issues #41

Closed salmanmp closed 2 years ago

salmanmp commented 2 years ago

After moving an IssueCard, there is a function move! in IssueCard model that saves changes on issue by itself and ignores hooks like controller_issues_edit_before_save and controller_issues_edit_after_save. Many of other plugins may be developed in way that do changes on issue by defining related hooks.

salmanmp commented 2 years ago

As a suggestion, this is a basic solution:

Index: app/models/issue_card.rb
<+>UTF-8
===================================================================
diff --git a/app/models/issue_card.rb b/app/models/issue_card.rb
--- a/app/models/issue_card.rb  (revision 68:19d31caeaa828f5b063ff060694289f3301cee90)
+++ b/app/models/issue_card.rb  (revision 68+:19d31caeaa82+)
@@ -26,6 +26,11 @@
     end
     if @move_attributes.any? || @custom_field_attributes.any?
       self.safe_attributes = @move_attributes.merge(@custom_field_attributes)
+      Redmine::Hook.call_hook(
+        :controller_issues_edit_before_save,
+        {:params =>{}, :issue => self,
+         :journal => self.current_journal}
+      )
       self.save!
+      Redmine::Hook.call_hook(
+        :controller_issues_edit_after_save,
+        {:params =>{}, :issue => self,
+         :journal => self.current_journal}
+      )     end
   end
takenory commented 2 years ago

@salmanmp Thanks for the feedback. I think the hook you gave us as an example is what the plugin calls to override the update operation on the issue screen (not the move operation on the issue panel).

Many of other plugins may be developed in way that do changes on issue by defining related hooks.

Can you give us an example of what kind of plugin and what kind of functionality it works with?

salmanmp commented 2 years ago

Any plugin that wants to make changes to an issue or do anything else after creating/changing an issue is officially referred to using these hooks. For example, when your plugin wants to create another issue after changing the status of an issue, if it wents to desired status.

I think the hook you gave us as an example is what the plugin calls to override the update operation on the issue screen

There are two type of hooks. These hooks are controller hooks and Redmine call many of theme in some points of code in Controllers.

takenory commented 2 years ago

For example, when your plugin wants to create another issue after changing the status of an issue, if it wents to desired status.

I think it is more appropriate to use AR model callbacks such as after_save, when a plugin overrides data operations that independent from controller.

To show changes behind the user's D&D operations (adding issues, setting assignee, etc.) would require reloading the whole panel, but the current D&D operations are not implemented that way.

It might be possible to check for changes that are not in the D&D (such as the number of issues to be displayed in the panel) before and after the D&D operation, and indicate the need for reloading.

takenory commented 2 years ago

Thanks for the reaction👍 @salmanmp. I'll close the issue.

salmanmp commented 2 years ago

I'm Sorry. I sent reaction :+1: to show that I'm agree; And it seems the latest solution is not perfect but it will work.