From 2914eef3759a37a6a3cfcf7dab70a6c64fd3d71c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karel=20Pi=C4=8Dman?= Date: Tue, 23 May 2023 13:57:40 +0200 Subject: [PATCH] Approval workflow actions --- app/controllers/dmsf_controller.rb | 4 +- app/controllers/dmsf_files_controller.rb | 8 ++-- app/controllers/dmsf_workflows_controller.rb | 22 +++++---- app/helpers/dmsf_upload_helper.rb | 13 ++--- app/helpers/dmsf_workflows_helper.rb | 47 ++++++++++--------- app/models/dmsf_workflow.rb | 31 ++++++------ assets/stylesheets/redmine_dmsf.css | 5 +- .../dmsf_workflow_controller_test.rb | 24 ++++------ 8 files changed, 76 insertions(+), 78 deletions(-) diff --git a/app/controllers/dmsf_controller.rb b/app/controllers/dmsf_controller.rb index cb5d3e73..a246f99e 100644 --- a/app/controllers/dmsf_controller.rb +++ b/app/controllers/dmsf_controller.rb @@ -634,8 +634,8 @@ class DmsfController < ApplicationController if Setting.plugin_redmine_dmsf['dmsf_display_notified_recipients'] && recipients.any? max_receivers = Setting.plugin_redmine_dmsf['dmsf_max_notification_receivers_info'].to_i to = recipients.collect { |user, _| user.name }.first(max_receivers).join(', ') - to << recipients.count > max_receivers ? ',...' : '.' - flash[:warning] = l(:warning_email_notifications, to: to) + to << (recipients.count > max_receivers ? ',...' : '.') + flash[:warning] = l(:warning_email_notifications, to: to) if to.present? end rescue StandardError => e Rails.logger.error { "Could not send email notifications: #{e.message}" } diff --git a/app/controllers/dmsf_files_controller.rb b/app/controllers/dmsf_files_controller.rb index 50060afd..8589b842 100644 --- a/app/controllers/dmsf_files_controller.rb +++ b/app/controllers/dmsf_files_controller.rb @@ -182,7 +182,7 @@ class DmsfFilesController < ApplicationController if Setting.plugin_redmine_dmsf['dmsf_display_notified_recipients'] && recipients.any? max_notifications = Setting.plugin_redmine_dmsf['dmsf_max_notification_receivers_info'].to_i to = recipients.collect { |user, _| user.name }.first(max_notifications).join(', ') - to << recipients.count > max_notifications ? ',...' : '.' + to << (recipients.count > max_notifications ? ',...' : '.') end rescue StandardError => e Rails.logger.error "Could not send email notifications: #{e.message}" @@ -198,7 +198,7 @@ class DmsfFilesController < ApplicationController respond_to do |format| format.html do flash[:error] = l(:error_file_is_locked) if @file.locked_for_user? - flash[:warning] = l(:warning_email_notifications, to: to) if to + flash[:warning] = l(:warning_email_notifications, to: to) if to.present? flash[:error] = @file.errors.full_messages.to_sentence if @file.errors.any? flash[:error] = revision.errors.full_messages.to_sentence if revision.errors.any? flash[:notice] = (flash[:notice].nil? ? '' : flash[:notice]) + l(:notice_file_revision_created) if ok @@ -223,8 +223,8 @@ class DmsfFilesController < ApplicationController if Setting.plugin_redmine_dmsf['dmsf_display_notified_recipients'] && recipients.any? max_notification = Setting.plugin_redmine_dmsf['dmsf_max_notification_receivers_info'].to_i to = recipients.collect { |user, _| user.name }.first(max_notification).join(', ') - to << recipients.count > max_notification ? ',...' : '.' - flash[:warning] = l(:warning_email_notifications, to: to) + to << (recipients.count > max_notification ? ',...' : '.') + flash[:warning] = l(:warning_email_notifications, to: to) if to.present? end rescue StandardError => e Rails.logger.error "Could not send email notifications: #{e.message}" diff --git a/app/controllers/dmsf_workflows_controller.rb b/app/controllers/dmsf_workflows_controller.rb index 2121e2a5..bb6b776a 100644 --- a/app/controllers/dmsf_workflows_controller.rb +++ b/app/controllers/dmsf_workflows_controller.rb @@ -52,12 +52,16 @@ class DmsfWorkflowsController < ApplicationController @path = dmsf_workflows_path end - def action; end + def action + # Noting to do + end - def show; end + def show + # Noting to do + end def new_action - if params[:commit] != l(:button_submit) && !request.post? + unless params[:commit] == l(:button_submit) && request.post? redirect_back_or_default dmsf_folder_path(id: @project, folder_id: @folder) return end @@ -97,8 +101,8 @@ class DmsfWorkflowsController < ApplicationController if Setting.plugin_redmine_dmsf['dmsf_display_notified_recipients'] && recipients.present? max_notifications = Setting.plugin_redmine_dmsf['dmsf_max_notification_receivers_info'].to_i to = recipients.collect(&:name).first(max_notifications).join(', ') - to << recipients.count > max_notifications ? ',...' : '.' - flash[:warning] = l(:warning_email_notifications, to: to) + to << (recipients.count > max_notifications ? ',...' : '.') + flash[:warning] = l(:warning_email_notifications, to: to) if to.present? end end elsif Setting.notified_events.include?('dmsf_workflow_plural') # Just rejected @@ -118,8 +122,8 @@ class DmsfWorkflowsController < ApplicationController if Setting.plugin_redmine_dmsf['dmsf_display_notified_recipients'] && recipients.present? max_notifications = Setting.plugin_redmine_dmsf['dmsf_max_notification_receivers_info'].to_i to = recipients.collect(&:name).first(max_notifications).join(', ') - to << recipients.count > max_notifications ? ',...' : '.' - flash[:warning] = l(:warning_email_notifications, to: to) + to << (recipients.count > max_notifications ? ',...' : '.') + flash[:warning] = l(:warning_email_notifications, to: to) if to.present? end end elsif action.action == DmsfWorkflowStepAction::ACTION_DELEGATE @@ -185,8 +189,8 @@ class DmsfWorkflowsController < ApplicationController unless recipients.empty? max_notifications = Setting.plugin_redmine_dmsf['dmsf_max_notification_receivers_info'].to_i to = recipients.collect(&:name).first(max_notifications).join(', ') - to << recipients.count > max_notifications ? ',...' : '.' - flash[:warning] = l(:warning_email_notifications, to: to) + to << (recipients.count > max_notifications ? ',...' : '.') + flash[:warning] = l(:warning_email_notifications, to: to) if to.present? end end end diff --git a/app/helpers/dmsf_upload_helper.rb b/app/helpers/dmsf_upload_helper.rb index 22c3f78a..117a7b55 100644 --- a/app/helpers/dmsf_upload_helper.rb +++ b/app/helpers/dmsf_upload_helper.rb @@ -139,15 +139,10 @@ module DmsfUploadHelper begin recipients = DmsfMailer.deliver_files_updated(project, files) if Setting.plugin_redmine_dmsf['dmsf_display_notified_recipients'] && recipients.any? - to = recipients.collect { |user, _| user.name }.first( - Setting.plugin_redmine_dmsf['dmsf_max_notification_receivers_info'].to_i - ).join(', ') - to << if recipients.count > Setting.plugin_redmine_dmsf['dmsf_max_notification_receivers_info'].to_i - ',...' - else - '.' - end - controller.flash[:warning] = l(:warning_email_notifications, to: to) if controller + max_recipients = Setting.plugin_redmine_dmsf['dmsf_max_notification_receivers_info'].to_i + to = recipients.collect { |user, _| user.name }.first(max_recipients).join(', ') + to << (recipients.count > max_recipients ? ',...' : '.') + controller.flash[:warning] = l(:warning_email_notifications, to: to) if controller && to.present? end rescue StandardError => e Rails.logger.error { "Could not send email notifications: #{e.message}" } diff --git a/app/helpers/dmsf_workflows_helper.rb b/app/helpers/dmsf_workflows_helper.rb index e6796053..a8dd0da2 100644 --- a/app/helpers/dmsf_workflows_helper.rb +++ b/app/helpers/dmsf_workflows_helper.rb @@ -27,26 +27,29 @@ module DmsfWorkflowsHelper principal_pages = Redmine::Pagination::Paginator.new principal_count, 100, params['page'] principals = scope.offset(principal_pages.offset).limit(principal_pages.per_page).to_a # Delegation - s = if dmsf_workflow_step_assignment_id - content_tag( - 'div', - content_tag('div', principals_radio_button_tags('step_action', principals), id: 'users_for_delegate'), - class: 'objects-selection' - ) - # New step - else - content_tag( - 'div', - content_tag('div', principals_check_box_tags('user_ids[]', principals), id: 'users'), - class: 'objects-selection' - ) - end + s = [] + s << if dmsf_workflow_step_assignment_id + content_tag( + 'div', + content_tag('div', principals_radio_button_tags('step_action', principals), + id: 'dmsf_users_for_delegate'), + class: 'objects-selection' + ) + # New step + else + content_tag( + 'div', + content_tag('div', principals_check_box_tags('user_ids[]', principals), id: 'users'), + class: 'objects-selection' + ) + end links = pagination_links_full(principal_pages, principal_count, per_page_links: false) do |text, parameters, _| link_to text, autocomplete_for_user_dmsf_workflow_path(workflow, parameters.merge(q: params[:q], format: 'js')), remote: true end - s + content_tag('span', links, class: 'pagination') + s << content_tag('span', links, class: 'pagination') + safe_join s end def dmsf_workflow_steps_options_for_select(steps) @@ -91,14 +94,14 @@ module DmsfWorkflowsHelper end def principals_radio_button_tags(name, principals) - s = +'' + s = [] principals.each do |principal| - s << content_tag( - 'label', - "#{radio_button_tag(name, principal.id * 10, false, onclick: 'noteMandatory(true);', id: nil)} #{h(principal)}" - ) - s << "\n" + n = principal.id * 10 + id = "principal_#{n}" + s << radio_button_tag(name, n, false, onclick: 'noteMandatory(true);', id: id) + s << content_tag(:label, h(principal), for: id) + s << tag.br end - s + safe_join s end end diff --git a/app/models/dmsf_workflow.rb b/app/models/dmsf_workflow.rb index 230252e7..a089d28a 100644 --- a/app/models/dmsf_workflow.rb +++ b/app/models/dmsf_workflow.rb @@ -57,17 +57,18 @@ class DmsfWorkflow < ApplicationRecord def self.workflow_info(workflow, workflow_id, revision_id) text = '' names = '' - dmsf_workflow = DmsfWorkflow.find_by(id: workflow_id) - if dmsf_workflow - assignments = dmsf_workflow.next_assignments(revision_id) - if assignments.any? - user_ids = assignments.map(&:user_id) - users = User.where(id: user_ids).all - names = users.map(&:name).join(',') - workflow_step_id = assignments.first[:dmsf_workflow_step_id] - if workflow_step_id - step = DmsfWorkflowStep.find_by(id: workflow_step_id) - text = step.name if step&.name.present? + if workflow.to_i == STATE_WAITING_FOR_APPROVAL + dmsf_workflow = DmsfWorkflow.find_by(id: workflow_id) + if dmsf_workflow + assignments = dmsf_workflow.next_assignments(revision_id) + if assignments.any? + user_ids = assignments.map(&:user_id) + names = User.where(id: user_ids).all.map(&:name).join(',') + workflow_step_id = assignments.first[:dmsf_workflow_step_id] + if workflow_step_id + step = DmsfWorkflowStep.find_by(id: workflow_step_id) + text = step.name if step&.name.present? + end end end end @@ -250,9 +251,9 @@ class DmsfWorkflow < ApplicationRecord ) return unless Setting.plugin_redmine_dmsf['dmsf_display_notified_recipients'] && controller && recipients.present? - to = recipients.collect(&:name) - .first(Setting.plugin_redmine_dmsf['dmsf_max_notification_receivers_info'].to_i).join(', ') - to << recipients.count > Setting.plugin_redmine_dmsf['dmsf_max_notification_receivers_info'].to_i ? ',...' : '.' - controller.flash[:warning] = l(:warning_email_notifications, to: to) + max_recipients = Setting.plugin_redmine_dmsf['dmsf_max_notification_receivers_info'].to_i + to = recipients.collect(&:name).first(max_recipients).join(', ') + to << (recipients.count > max_recipients.to_i ? ',...' : '.') + controller.flash[:warning] = l(:warning_email_notifications, to: to) if controller && to.present? end end diff --git a/assets/stylesheets/redmine_dmsf.css b/assets/stylesheets/redmine_dmsf.css index 0cca98f6..2d94a08f 100644 --- a/assets/stylesheets/redmine_dmsf.css +++ b/assets/stylesheets/redmine_dmsf.css @@ -56,11 +56,12 @@ /* Approval workflow */ #dmsf_users_for_delegate { height: 200px; - overflow:auto; + overflow: auto; + column-width: auto } #dmsf_users_for_delegate label { - display: block; + display: unset; } .dmsf-workflows.locked a { diff --git a/test/functional/dmsf_workflow_controller_test.rb b/test/functional/dmsf_workflow_controller_test.rb index 7bab24a0..af65b942 100644 --- a/test/functional/dmsf_workflow_controller_test.rb +++ b/test/functional/dmsf_workflow_controller_test.rb @@ -173,7 +173,7 @@ class DmsfWorkflowsControllerTest < RedmineDmsf::Test::TestCase end assert_response :success ws = DmsfWorkflowStep.order(id: :desc).first - assert_equal @wf1.id, ws.dmsf_workflow_id + assert_equal @wf1.id, ws&.dmsf_workflow_id assert_equal 1, ws.step assert_equal '1st step', ws.name assert_equal @someone.id, ws.user_id @@ -187,7 +187,7 @@ class DmsfWorkflowsControllerTest < RedmineDmsf::Test::TestCase end assert_response :redirect ws = DmsfWorkflowStep.where(dmsf_workflow_id: @wf1.id).order(id: :asc).first - assert_equal 1, ws.step + assert_equal 1, ws&.step end def test_reorder_steps_to_lower @@ -253,7 +253,7 @@ class DmsfWorkflowsControllerTest < RedmineDmsf::Test::TestCase def test_action_approve post( :new_action, params: { - commit: l(:but19ton_submit), + commit: l(:button_submit), id: @wf1.id, dmsf_workflow_step_assignment_id: @wfsa2.id, dmsf_file_revision_id: @revision1.id, @@ -263,10 +263,8 @@ class DmsfWorkflowsControllerTest < RedmineDmsf::Test::TestCase } ) assert_redirected_to dmsf_folder_path(id: @project1) - assert DmsfWorkflowStepAction.where( - dmsf_workflow_step_assignment_id: @wfsa2.id, - action: DmsfWorkflowStepAction::ACTION_APPROVE - ).first + assert DmsfWorkflowStepAction.exists?(dmsf_workflow_step_assignment_id: @wfsa2.id, + action: DmsfWorkflowStepAction::ACTION_APPROVE) end def test_action_reject @@ -281,10 +279,8 @@ class DmsfWorkflowsControllerTest < RedmineDmsf::Test::TestCase } ) assert_response :redirect - assert DmsfWorkflowStepAction.where( - dmsf_workflow_step_assignment_id: @wfsa2.id, - action: DmsfWorkflowStepAction::ACTION_REJECT - ).first + assert DmsfWorkflowStepAction.exists?(dmsf_workflow_step_assignment_id: @wfsa2.id, + action: DmsfWorkflowStepAction::ACTION_REJECT) end def test_action @@ -314,10 +310,8 @@ class DmsfWorkflowsControllerTest < RedmineDmsf::Test::TestCase } ) assert_redirected_to dmsf_folder_path(id: @project1) - assert DmsfWorkflowStepAction.where( - dmsf_workflow_step_assignment_id: @wfsa2.id, - action: DmsfWorkflowStepAction::ACTION_DELEGATE - ).first + assert DmsfWorkflowStepAction.exists?(dmsf_workflow_step_assignment_id: @wfsa2.id, + action: DmsfWorkflowStepAction::ACTION_DELEGATE) @wfsa2.reload assert_equal @wfsa2.user_id, @admin.id end