From b7143c5feb665cb4fdbaa9e2538805fc2e12a737 Mon Sep 17 00:00:00 2001 From: Akiko Takano Date: Thu, 6 Aug 2020 00:33:12 +0900 Subject: [PATCH] Remove Checklist integration feature. --- README.md | 2 +- .../concerns/issue_templates_common.rb | 14 +---- .../global_issue_templates_controller.rb | 4 +- app/controllers/issue_templates_controller.rb | 5 +- app/models/concerns/issue_template/common.rb | 2 + app/models/global_issue_template.rb | 1 - app/models/issue_template.rb | 1 - .../global_issue_templates/_form.html.erb | 23 -------- app/views/global_issue_templates/new.html.erb | 3 +- .../global_issue_templates/show.html.erb | 2 +- app/views/issue_templates/_form.html.erb | 23 -------- app/views/issue_templates/new.html.erb | 3 +- app/views/issue_templates/show.html.erb | 18 +----- assets/javascripts/issue_templates.js | 22 -------- assets/javascripts/template_checklists.js | 54 ------------------ .../concerns/issue_templates_common_spec.rb | 17 ------ .../global_issue_templates_controller_test.rb | 23 -------- .../issue_templates_controller_test.rb | 55 +------------------ 18 files changed, 13 insertions(+), 259 deletions(-) delete mode 100644 assets/javascripts/template_checklists.js diff --git a/README.md b/README.md index 04ed5827..421eb2ce 100644 --- a/README.md +++ b/README.md @@ -114,7 +114,7 @@ If you have any requests, bug reports, please use GitHub issues. * Bugfix: template_type is not defined error (GitHub: #364 / Thanks for reporting issue, @toku463ne) diff --git a/app/controllers/concerns/issue_templates_common.rb b/app/controllers/concerns/issue_templates_common.rb index 1e9d7f65..4dbb18dc 100644 --- a/app/controllers/concerns/issue_templates_common.rb +++ b/app/controllers/concerns/issue_templates_common.rb @@ -48,10 +48,6 @@ def apply_all_projects? plugin_setting['apply_global_template_to_all_projects'].to_s == 'true' end - def checklists - template_params[:checklists].presence || [] - end - def builtin_fields_json value = template_params[:builtin_fields].blank? ? {} : JSON.parse(template_params[:builtin_fields]) return value if value.is_a?(Hash) @@ -59,17 +55,9 @@ def builtin_fields_json raise InvalidTemplateFormatError end - def checklist_enabled? - Redmine::Plugin.registered_plugins.key? :redmine_checklists - rescue StandardError - false - end - def valid_params - # convert attribute name and data for checklist plugin supporting - attributes = template_params.except(:checklists, :builtin_fields) + attributes = template_params.except(:builtin_fields) attributes[:builtin_fields_json] = builtin_fields_json if builtin_fields_enabled? - attributes[:checklist_json] = checklists.to_json if checklist_enabled? attributes end diff --git a/app/controllers/global_issue_templates_controller.rb b/app/controllers/global_issue_templates_controller.rb index 68c39c35..a6360efa 100644 --- a/app/controllers/global_issue_templates_controller.rb +++ b/app/controllers/global_issue_templates_controller.rb @@ -131,7 +131,7 @@ def template_params params.require(:global_issue_template) .permit(:title, :tracker_id, :issue_title, :description, :note, :is_default, :enabled, :author_id, :position, :related_link, :link_title, :builtin_fields, - project_ids: [], checklists: []) + project_ids: []) end def render_form_params @@ -142,7 +142,7 @@ def render_form_params .merge(custom_fields_map_by_tracker_id(tracker_id)).to_json { layout: !request.xhr?, - locals: { checklist_enabled: checklist_enabled?, trackers: trackers, apply_all_projects: apply_all_projects?, + locals: { trackers: trackers, apply_all_projects: apply_all_projects?, issue_template: @global_issue_template, projects: projects, custom_fields: custom_fields.to_s, builtin_fields_enable: builtin_fields_enabled? } } end diff --git a/app/controllers/issue_templates_controller.rb b/app/controllers/issue_templates_controller.rb index 8684ed39..40d96ac6 100644 --- a/app/controllers/issue_templates_controller.rb +++ b/app/controllers/issue_templates_controller.rb @@ -216,8 +216,7 @@ def inherit_templates def template_params params.require(:issue_template).permit(:tracker_id, :title, :note, :issue_title, :description, :is_default, :enabled, :author_id, :position, :enabled_sharing, - :related_link, :link_title, :builtin_fields, - checklists: []) + :related_link, :link_title, :builtin_fields) end def templates_exist? @@ -231,7 +230,7 @@ def render_form_params { layout: !request.xhr?, locals: { issue_template: template, project: @project, child_project_used_count: child_project_used_count, - checklist_enabled: checklist_enabled?, custom_fields: custom_fields.to_s, builtin_fields_enable: builtin_fields_enabled? } } + custom_fields: custom_fields.to_s, builtin_fields_enable: builtin_fields_enabled? } } end def loadable_trigger? diff --git a/app/models/concerns/issue_template/common.rb b/app/models/concerns/issue_template/common.rb index efee22f3..eb3c6ff0 100644 --- a/app/models/concerns/issue_template/common.rb +++ b/app/models/concerns/issue_template/common.rb @@ -58,6 +58,8 @@ def <=>(other) position <=> other.position end + # Keep this method for a while, but this will be deprecated. + # Please see: https://github.com/akiko-pusu/redmine_issue_templates/issues/363 def checklist return [] if checklist_json.blank? diff --git a/app/models/global_issue_template.rb b/app/models/global_issue_template.rb index b32357f8..062b088a 100644 --- a/app/models/global_issue_template.rb +++ b/app/models/global_issue_template.rb @@ -18,7 +18,6 @@ class GlobalIssueTemplate < ActiveRecord::Base 'project_ids', 'position', 'author_id', - 'checklist_json', 'related_link', 'link_title', 'builtin_fields_json' diff --git a/app/models/issue_template.rb b/app/models/issue_template.rb index a88be5b1..9645732d 100644 --- a/app/models/issue_template.rb +++ b/app/models/issue_template.rb @@ -19,7 +19,6 @@ class IssueTemplate < ActiveRecord::Base 'enabled_sharing', 'visible_children', 'position', - 'checklist_json', 'related_link', 'link_title', 'builtin_fields_json' diff --git a/app/views/global_issue_templates/_form.html.erb b/app/views/global_issue_templates/_form.html.erb index a3d9e4ad..0d44d0ab 100644 --- a/app/views/global_issue_templates/_form.html.erb +++ b/app/views/global_issue_templates/_form.html.erb @@ -30,26 +30,6 @@ required: true, label: l(:issue_description), class: 'wiki-edit', style: 'overflow: auto;' %>

- - <% if checklist_enabled %> -

- - - -

-

- <% end %> @@ -258,6 +238,3 @@ <%= javascript_include_tag('template_fields', plugin: 'redmine_issue_templates') %> <% end %> -<% if checklist_enabled %> -<%= javascript_include_tag('template_checklists', plugin: 'redmine_issue_templates') %> -<% end %> diff --git a/app/views/global_issue_templates/new.html.erb b/app/views/global_issue_templates/new.html.erb index abd6c96f..5588eb53 100644 --- a/app/views/global_issue_templates/new.html.erb +++ b/app/views/global_issue_templates/new.html.erb @@ -9,8 +9,7 @@ html: { id: 'global_issue_template-form', class: nil, multipart: false } do |f| %> - <%= render 'form', { f: f, checklist_enabled: checklist_enabled, - trackers: trackers, projects: projects, + <%= render 'form', { f: f, trackers: trackers, projects: projects, issue_template: issue_template, apply_all_projects: apply_all_projects, custom_fields: custom_fields, builtin_fields_enable: builtin_fields_enable } %> <% end %> diff --git a/app/views/global_issue_templates/show.html.erb b/app/views/global_issue_templates/show.html.erb index 4c96c54b..6014a946 100644 --- a/app/views/global_issue_templates/show.html.erb +++ b/app/views/global_issue_templates/show.html.erb @@ -18,7 +18,7 @@ url: { controller: 'global_issue_templates', action: 'update', id: issue_template }, html: { id: 'global_issue_template-form', class: nil, multipart: false } do |f| %> - <%= render 'form', { f: f, checklist_enabled: checklist_enabled, trackers: trackers, + <%= render 'form', { f: f, trackers: trackers, issue_template: issue_template, projects: projects, apply_all_projects: apply_all_projects, custom_fields: custom_fields, builtin_fields_enable: builtin_fields_enable } %> <% end %> diff --git a/app/views/issue_templates/_form.html.erb b/app/views/issue_templates/_form.html.erb index 443e2fc4..3478feb5 100644 --- a/app/views/issue_templates/_form.html.erb +++ b/app/views/issue_templates/_form.html.erb @@ -33,26 +33,6 @@ required: true, label: l(:issue_description), class: 'wiki-edit', style: 'overflow: auto;' %>

- - <% if checklist_enabled %> -

- - - -

-

- <% end %> @@ -233,6 +213,3 @@ <%= javascript_include_tag('template_fields', plugin: 'redmine_issue_templates') %> <% end %> -<% if checklist_enabled %> -<%= javascript_include_tag('template_checklists', plugin: 'redmine_issue_templates') %> -<% end %> diff --git a/app/views/issue_templates/new.html.erb b/app/views/issue_templates/new.html.erb index 45f862ea..660ac784 100644 --- a/app/views/issue_templates/new.html.erb +++ b/app/views/issue_templates/new.html.erb @@ -10,8 +10,7 @@ url: { controller: 'issue_templates', action: 'create', project_id: project }, html: { id: 'issue_template-form', class: nil, multipart: false } do |f| %> - <%= render 'form', { f: f, checklist_enabled: checklist_enabled, - issue_template: issue_template, project: project, custom_fields: custom_fields, + <%= render 'form', { f: f, issue_template: issue_template, project: project, custom_fields: custom_fields, builtin_fields_enable: builtin_fields_enable } %>
<%= submit_tag l(:button_create) %> diff --git a/app/views/issue_templates/show.html.erb b/app/views/issue_templates/show.html.erb index 1df405c8..4e43c487 100644 --- a/app/views/issue_templates/show.html.erb +++ b/app/views/issue_templates/show.html.erb @@ -37,8 +37,7 @@ project_id: project, id: issue_template }, html: { id: 'issue_template-form', class: nil, multipart: false } do |f| %> - <%= render 'form', { f: f, checklist_enabled: checklist_enabled, - issue_template: issue_template, project: project, + <%= render 'form', { f: f, issue_template: issue_template, project: project, custom_fields: custom_fields, builtin_fields_enable: builtin_fields_enable } %>
<%= submit_tag l(:button_save) %> @@ -74,21 +73,6 @@

<%= textilizable(issue_template.description) %>
- - <% if checklist_enabled %> -

- -

-

- <% end %> diff --git a/assets/javascripts/issue_templates.js b/assets/javascripts/issue_templates.js index bf2e0ba5..983cce9d 100644 --- a/assets/javascripts/issue_templates.js +++ b/assets/javascripts/issue_templates.js @@ -193,7 +193,6 @@ ISSUE_TEMPLATE.prototype = { } ns.setRelatedLink(obj) - ns.addCheckList(obj) ns.builtinFields(obj) ns.confirmToReplace = true }, @@ -294,27 +293,6 @@ ISSUE_TEMPLATE.prototype = { document.getElementById('issue_template').dispatchEvent(changeEvent) }) }, - addCheckList: function (obj) { - let list = obj.checklist - if (list == null) return false - let checklistForm = document.getElementById('checklist_form') - if (!checklistForm) return - - // NOTE: If Checklist does not work fine, please confirm its version and the DOM element of - // checklist input field exists. - // If some difference, please report the issue or feedback to IssueTemplate's repository. - try { - for (let i = 0; i < list.length; i++) { - let node = document.querySelector('span.checklist-item.new > span.checklist-edit-box > input.edit-box') - if (node) { - node.value = list[i] - document.querySelector('span.checklist-item.new > span.icon.icon-add.checklist-new-only.save-new-by-button').click() - } - } - } catch (e) { - console.log(`NOTE: Checklist could not be applied due to this error. ${e.message} : ${e.message}`) - } - }, setRelatedLink: function (obj) { let relatedLink = document.getElementById('issue_template_related_link') if (obj.related_link != null && obj.related_link !== '') { diff --git a/assets/javascripts/template_checklists.js b/assets/javascripts/template_checklists.js deleted file mode 100644 index 13672175..00000000 --- a/assets/javascripts/template_checklists.js +++ /dev/null @@ -1,54 +0,0 @@ -'use strict' -const removeCheckList = (obj) => { - let target = obj.closest('li') - target.remove() -} - -const addCheckList = () => { - let checkListInputText = document.getElementById('checklist_text') - let text = checkListInputText.value - if (text === '') return false - - let checkListItems = document.getElementsByClassName('checklist-item') - // NOTE: some, find, every and forEach method scan all the element and not exit this function via return. - for (let i = 0; i < checkListItems.length; i++) { - let elem = checkListItems[i] - if (text === elem.value) { - return - } - } - - addCheckListItem(text) - checkListInputText.value = '' -} - -const addCheckListItem = (value) => { - let li = document.createElement('li') - let span = document.createElement('span') - span.classList.add('text') - - let checkListText = document.createTextNode(value) - span.appendChild(checkListText) - - let hidden = document.createElement('input') - hidden.classList.add('checklist-item') - hidden.value = value - hidden.setAttribute('type', 'hidden') - hidden.setAttribute('id', templateType + '_checklist') - hidden.setAttribute('name', templateType + '[checklists][]') - - li.appendChild(hidden) - - let removeLink = document.createElement('i') - removeLink.classList.add('icon', 'icon-del') - span.appendChild(removeLink) - - li.appendChild(span) - - let checklist = document.querySelector('ul.checklist') - checklist.appendChild(li) - - removeLink.addEventListener('click', (event) => { - removeCheckList(event.target) - }, false) -} diff --git a/spec/controllers/concerns/issue_templates_common_spec.rb b/spec/controllers/concerns/issue_templates_common_spec.rb index e5656a0b..bb0e4e80 100644 --- a/spec/controllers/concerns/issue_templates_common_spec.rb +++ b/spec/controllers/concerns/issue_templates_common_spec.rb @@ -12,23 +12,6 @@ class FakesController < ApplicationController end let(:mock_controller) { FakesController.new } - describe '#checklist_enabled?' do - subject { mock_controller.checklist_enabled? } - context 'checklist plugin not registered' do - before do - allow(Redmine::Plugin).to receive(:registered_plugins).and_return({}) - end - it { is_expected.to be_falsey } - end - - context 'checklist plugin registered' do - before do - allow(Redmine::Plugin).to receive(:registered_plugins).and_return(redmine_checklists: 'mock data') - end - it { is_expected.to be_truthy } - end - end - describe '#log_action' do subject { mock_controller.log_action } diff --git a/test/functional/global_issue_templates_controller_test.rb b/test/functional/global_issue_templates_controller_test.rb index 46315f7c..d65f0b0f 100644 --- a/test/functional/global_issue_templates_controller_test.rb +++ b/test/functional/global_issue_templates_controller_test.rb @@ -32,29 +32,6 @@ def test_update_template assert_equal 'Update Test Global template2', global_issue_template.description end - def test_update_template_when_checklist_enable - # Use stub to test with other plugin - mock = MiniTest::Mock.new - mock.expect(:call, true, [:redmine_checklists]) - Redmine::Plugin.registered_plugins.stub(:key?, mock) do - checklists_param = %w[check1 check2] - - num = GlobalIssueTemplate.count - # when title blank, validation bloks to save. - put :update, params: { id: 2, global_issue_template: - { description: 'Update Test Global template2 with checklist param', checklists: checklists_param } } - - assert_response :redirect # show - assert_equal(num, GlobalIssueTemplate.count) - - global_issue_template = GlobalIssueTemplate.find(2) - - assert_equal(checklists_param.to_json, global_issue_template.checklist_json) - assert_equal(checklists_param, global_issue_template.checklist) - end - mock.verify - end - def test_update_template_with_empty_title put :update, params: { id: 2, global_issue_template: { title: '' } } diff --git a/test/functional/issue_templates_controller_test.rb b/test/functional/issue_templates_controller_test.rb index 6911ea8c..9bbf4416 100644 --- a/test/functional/issue_templates_controller_test.rb +++ b/test/functional/issue_templates_controller_test.rb @@ -73,7 +73,7 @@ def test_create_template num = IssueTemplate.count post :create, params: { - issue_template: { title: 'newtitle', note: 'note', checklists: %w[check1 check2], + issue_template: { title: 'newtitle', note: 'note', description: 'description', tracker_id: 1, enabled: 1, author_id: 3 }, project_id: 1 } @@ -95,34 +95,6 @@ def test_create_template assert_equal([], template.checklist) end - def test_create_template_when_checklist_enable - edit_permission - - # Use stub to test with other plugin - mock = MiniTest::Mock.new - mock.expect(:call, true, [:redmine_checklists]) - Redmine::Plugin.registered_plugins.stub(:key?, mock) do - checklists_param = %w[check1 check2] - - num = IssueTemplate.count - post :create, params: { - issue_template: { title: 'newtitle', note: 'note', checklists: checklists_param, - description: 'description', tracker_id: 1, enabled: 1, author_id: 3 }, - project_id: 1 - } - - template = IssueTemplate.last - assert_response :redirect - - assert_equal(num + 1, IssueTemplate.count) - - assert_not_nil template - assert_equal(checklists_param.to_json, template.checklist_json) - assert_equal(checklists_param, template.checklist) - end - mock.verify - end - def test_create_template_with_empty_title edit_permission @@ -181,31 +153,6 @@ def test_update_template_with_empty_title assert_select 'div#errorExplanation', { count: 1, text: /Title cannot be blank/ }, @response.body.to_s end - def test_update_template_when_checklist_enable - edit_permission - - # Use stub to test with other plugin - mock = MiniTest::Mock.new - mock.expect(:call, true, [:redmine_checklists]) - Redmine::Plugin.registered_plugins.stub(:key?, mock) do - checklists_param = %w[check1 check2] - - num = IssueTemplate.count - # when title blank, validation bloks to save. - put :update, params: { id: 2, - issue_template: { title: 'update with checklist param', checklists: checklists_param }, project_id: 1 } - - assert_response :redirect # show - assert_equal(num, IssueTemplate.count) - - issue_template = IssueTemplate.find(2) - - assert_equal(checklists_param.to_json, issue_template.checklist_json) - assert_equal(checklists_param, issue_template.checklist) - end - mock.verify - end - def test_delete_template_fail_if_enabled edit_permission