Prevent conflicts with full text-search plugin - Githubissues

akiko-pusu commented 7 years ago

Here is my workaround. Hope this would be any help for you.

diff --git a/init.rb b/init.rb
index 6c28eaf..a5438f5 100644
--- a/init.rb
+++ b/init.rb
@@ -16,3 +16,15 @@ require_dependency 'redmine_xlsx_format_issue_exporter/issues_controller_patch'
 require_dependency 'redmine_xlsx_format_issue_exporter/timelog_controller_patch'

 require_dependency 'redmine_xlsx_format_issue_exporter/view_layouts_base_body_bottom_hook'
+
+Rails.configuration.to_prepare do
+  require_dependency 'issues_controller'
+  require_dependency 'timelog_controller'
+  unless IssuesController.included_modules.include? RedmineXlsxFormatIssueExporter::IssuesControllerPatch
+    IssuesController.send(:prepend, RedmineXlsxFormatIssueExporter::IssuesControllerPatch)
+  end
+
+  unless TimelogController.included_modules.include?(RedmineXlsxFormatIssueExporter::TimelogControllerPatch)
+    TimelogController.send(:prepend, RedmineXlsxFormatIssueExporter::TimelogControllerPatch)
+  end
+end
diff --git a/lib/redmine_xlsx_format_issue_exporter/issues_controller_patch.rb b/lib/redmine_xlsx_format_issue_exporter/issues_controller_patch.rb
index 79bd889..3ad1283 100644
--- a/lib/redmine_xlsx_format_issue_exporter/issues_controller_patch.rb
+++ b/lib/redmine_xlsx_format_issue_exporter/issues_controller_patch.rb
@@ -1,6 +1,3 @@
-require_dependency 'issues_controller'
-require_dependency 'redmine_xlsx_format_issue_exporter/reloader'
-
 module RedmineXlsxFormatIssueExporter
   module IssuesControllerPatch
     include XlsxExportHelper
@@ -20,10 +17,4 @@ module RedmineXlsxFormatIssueExporter
       send_data(query_to_xlsx(@issues, @query, params), :type => 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet;', :filename => 'issues.xlsx')
     end
   end
-
-  Reloader.to_prepare do
-    unless IssuesController.included_modules.include?(RedmineXlsxFormatIssueExporter::IssuesControllerPatch)
-      IssuesController.send(:prepend, RedmineXlsxFormatIssueExporter::IssuesControllerPatch)
-    end
-  end
-end
\ No newline at end of file
+end
diff --git a/lib/redmine_xlsx_format_issue_exporter/reloader.rb b/lib/redmine_xlsx_format_issue_exporter/reloader.rb
deleted file mode 100644
index 94ec38d..0000000
--- a/lib/redmine_xlsx_format_issue_exporter/reloader.rb
+++ /dev/null
@@ -1,25 +0,0 @@
-module RedmineXlsxFormatIssueExporter
-  module Reloader
-    # ActionDispatch::Reloader.to_prepare is deprecated in Rails 5.0 and will be removed from Rails 5.1
-    #
-    # Use ActiveSupport::Reloader if available for Rails 5, fall back to ActionDispatch::Reloader for earlier Rails
-    def self.to_prepare(*args, &block)
-      if defined? ActiveSupport::Reloader
-        ActiveSupport::Reloader.to_prepare(*args, &block)
-      else
-        ActionDispatch::Reloader.to_prepare(*args, &block)
-      end
-    end
-
-    # ActionDispatch::Reloader.to_cleanup is deprecated in Rails 5.0 and will be removed from Rails 5.1
-    #
-    # Use ActiveSupport::Reloader if available for Rails 5, fall back to ActionDispatch::Reloader for earlier Rails
-    def self.to_complete(*args, &block)
-      if defined? ActiveSupport::Reloader
-        ActiveSupport::Reloader.to_complete(*args, &block)
-      else
-        ActionDispatch::Reloader.to_cleanup(*args, &block)
-      end
-    end
-  end
-end
\ No newline at end of file
diff --git a/lib/redmine_xlsx_format_issue_exporter/timelog_controller_patch.rb b/lib/redmine_xlsx_format_issue_exporter/timelog_controller_patch.rb
index 47baadb..8c0b0ed 100644
--- a/lib/redmine_xlsx_format_issue_exporter/timelog_controller_patch.rb
+++ b/lib/redmine_xlsx_format_issue_exporter/timelog_controller_patch.rb
@@ -1,6 +1,3 @@
-require_dependency 'timelog_controller'
-require_dependency 'redmine_xlsx_format_issue_exporter/reloader'
-
 module RedmineXlsxFormatIssueExporter
   module TimelogControllerPatch
     include XlsxExportHelper
@@ -34,10 +31,4 @@ module RedmineXlsxFormatIssueExporter
       send_data(report_to_xlsx(@report), :type => 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet;', :filename => 'timelog.xlsx')
     end
   end
-
-  Reloader.to_prepare do
-    unless TimelogController.included_modules.include?(RedmineXlsxFormatIssueExporter::TimelogControllerPatch)
-      TimelogController.send(:prepend, RedmineXlsxFormatIssueExporter::TimelogControllerPatch)
-    end
-  end
 end

パッチの読み込みを、パッチファイル自身に定義ではなくてinit.rb側に設定しました。 (Wiki Extとかと同様な感じです） xlsx_format_issue_exporter側が逆に正しく動くかはチェックしないといけませんが...

akiko-pusu commented 7 years ago

あー、すみません。これだとダメですね。XLSフォーマットのダウンロードができなくなっちゃう... もう少し確認します。

akiko-pusu commented 7 years ago

最初に添付したパッチ一部なおしました。（最初の投稿のところ利用してみてください） init.rbのところは、両方とも prepend で。

Rails.configuration.to_prepare do
  require_dependency 'issues_controller'
  require_dependency 'timelog_controller'
  unless IssuesController.included_modules.include? RedmineXlsxFormatIssueExporter::IssuesControllerPatch
    IssuesController.send(:prepend, RedmineXlsxFormatIssueExporter::IssuesControllerPatch)
  end

  unless TimelogController.included_modules.include?(RedmineXlsxFormatIssueExporter::TimelogControllerPatch)
    TimelogController.send(:prepend, RedmineXlsxFormatIssueExporter::TimelogControllerPatch)
  end
end

y503unavailable commented 7 years ago

下記チェックしました。動作OKです。

既存チケットの表示・編集新規チケット作成・保存全文検索実行・チケット表示

akiko-pusu commented 7 years ago

Environment:

Base: Redmine3.4 / MySQL5.5 / CentOS7.2
Plugin: redmine_xlsx_format_issue_exporter, full text search

Steps to reproduce:

Install Redmine, and also install above two plugins. (NOTE: Some additional extensions against MySQL are required to use full text search plugin, but this does not have any effect to ruby, Redmine itself. )
Create new project and activate issue tracking.
Open the form to create issue and submit, then, error happens.
- Submit -> POST -> Save are completed successfully.
Issue is created correctly. But error happens when user try to show the issue.

Error message:

At IssuesController#show, this error message was rendered. The method named “display_score?” is belongs to full text search plugin.

Ref: https://github.com/okkez/redmine_full_text_search/blob/master/lib/full_text_search/hooks/settings_helper.rb#L4

It seems some methods included full text search plugin are not recognized in case using both full text search plugin and redmine_xlsx_format_issue_exporter. (Exp. similar_issues are also recognized.)

ActionView::Template::Error (undefined method `display_score?' for #<#<Class:0x007f048ba4b640>:0x007f047acd3ec8>
Did you mean?  display_comments_tree):
    1: <% if display_score? %>
    2:   <%
    3:   duration = nil

Possible cause:

It seems some methods related to the patch of full text search against IssuesController. I guess included modules of full_text_search might be overwrited by redmine_xlsx_format_issue_exporter. (In my experience, if conflict happens, the plugin which is later in alphabetical order is preferred.)

とりあえず再現情報を。（適当な英語で誤字脱字すみません...) それぞれのプラグインでの単体での利用では大丈夫そうです。両方が入った状態で、IssuesController#show のタイミング（たとえば新規作成してreturnで表示）で、Viewhookしているfull_text_searchプラグインが用意しているメソッドが存在しない、とのメッセージでエラーになります。

最初にエラーになるのは、display_score? が呼ばれたタイミングです。試しにこのif文を削除してみて、その後にまた別のfull_text_searchプラグイン用のメソッドが呼ばれるとエラーになりました。結果、IssuesControllerに関わる一連のものが抜けてしまっていると考えました。

full_text_searchプラグイン側もinit.rbでいろいろパッチを適用させているのですが、こちら数が多めで確認が難しかったので、ひとまず redmine_xlsx_format_issue_exporter 側でのworkaroundを報告させていただきました。

akiko-pusu commented 7 years ago

パッチ系は、test / development / production の全てで動くのを確認したほうがいいと思います。（productionでうまくいかないケースがあるので）

two-pack commented 7 years ago

I reproduce this problem and @akiko-pusu 's patch fixes it :) I gonna check any more and merge the patch.

情報ありがとうございました！問題の再現とパッチでの改善を確認できました。再現環境でもう少し確認してからマージしたいと思います。

akiko-pusu commented 7 years ago

ありがとうございます。 redmine_xlsx_format_issue_exporter はtrunk (Rails5.1のRedmine4) のテストも一緒のコミットでカバーされていますか？参考までに...。このやり方がRedmine4で大丈夫かちょっと心配ですので。

two-pack commented 7 years ago

(Rails5.1のRedmine4) のテストも一緒のコミットでカバーされていますか？

はい、カバーしています。単体ではtrunkでもテストは通ってます。

Test of this plugin patched above passed on trunk without full-text-search plugin.

two-pack commented 7 years ago

On reproduced environment, test is FAILED.

two-pack commented 7 years ago

Failed logs are following and more:

  1) Error:
IssuesControllerTest#test_index_xlsx_tw:
ActiveRecord::StatementInvalid: Mysql2::Error: Table 'redmine_test.searcher_records' doesn't exist: SHOW FULL FIELDS FROM `searcher_records`
    plugins/full_text_search/lib/full_text_search/model.rb:21:in `after_save'
    app/models/issue.rb:210:in `create_or_update'
    test/object_helpers.rb:103:in `generate!'
    plugins/redmine_xlsx_format_issue_exporter/test/functional/issues_controller_test.rb:196:in `block in test_index_xlsx_tw'
    test/test_helper.rb:104:in `with_settings'
    plugins/redmine_xlsx_format_issue_exporter/test/functional/issues_controller_test.rb:194:in `test_index_xlsx_tw'

Above is occured with only full_text_search plugin.

two-pack commented 7 years ago

Above error is occured because db:test:prepare remove db data and restore using schema.rb. But schema.rb DO NOT includes db storage engne option from plugin migratation.

I changed redmine.rake to do nothing on db:test:prepare when run plugin test. After that, this plugin test is all passed.

So I accept @akiko-pusu 's patch and add a test to check enable to show issue page. Thanks your working, @akiko-pusu :)

akiko-pusu commented 7 years ago

Good Job!

two-pack / redmine_xlsx_format_issue_exporter

Prevent conflicts with full text-search plugin #50

Environment:

Steps to reproduce:

Error message:

Possible cause: