From e037b6015eeb24050816a643156230d7bb9f7bac Mon Sep 17 00:00:00 2001 From: Mantas Masalskis Date: Mon, 13 Apr 2020 23:26:09 +0300 Subject: [PATCH] Fixes #2990 - Backward compatibility of deleting own notes --- .../ticket_zoom/article_action/delete.coffee | 17 ++++-- app/policies/ticket/article_policy.rb | 14 ++++- ...00413160113_issue_2990_delete_timeframe.rb | 16 +++++ db/seeds/settings.rb | 13 ++++ spec/requests/ticket/article_spec.rb | 59 +++++++++++++++---- spec/system/ticket/zoom_spec.rb | 53 ++++++++++++++++- 6 files changed, 154 insertions(+), 18 deletions(-) create mode 100644 db/migrate/20200413160113_issue_2990_delete_timeframe.rb diff --git a/app/assets/javascripts/app/controllers/ticket_zoom/article_action/delete.coffee b/app/assets/javascripts/app/controllers/ticket_zoom/article_action/delete.coffee index 5a93af661..bef0485fd 100644 --- a/app/assets/javascripts/app/controllers/ticket_zoom/article_action/delete.coffee +++ b/app/assets/javascripts/app/controllers/ticket_zoom/article_action/delete.coffee @@ -18,9 +18,9 @@ class Delete actions @isDeletable: (actions, ticket, article, ui) -> - return { isDeletable: true } if ui.permissionCheck('admin') - + return { isDeletable: true } if ui.permissionCheck('admin') return { isDeletable: false } if !@deletableForAgent(actions, ticket, article, ui) + return { isDeletable: true } if !@hasDeletableTimeframe() timeout = @deletableTimeout(actions, ticket, article, ui) @@ -28,9 +28,18 @@ class Delete { isDeletable: true, timeout: timeout } + @deletableTimeframeSetting: -> + App.Config.get('ui_ticket_zoom_article_delete_timeframe') + + @hasDeletableTimeframe: -> + @deletableTimeframeSetting() && @deletableTimeframeSetting() > 0 + @deletableTimeout: (actions, ticket, article, ui) -> - created_at = Date.parse(article.created_at) - 600000 - (Date.parse(new Date()) - created_at) + timeframe_miliseconds = @deletableTimeframeSetting() * 1000 + now = Date.parse(new Date()) + created_at = Date.parse(article.created_at) + + timeframe_miliseconds - (now - created_at) @deletableForAgent: (actions, ticket, article, ui) -> return false if !ui.permissionCheck('ticket.agent') diff --git a/app/policies/ticket/article_policy.rb b/app/policies/ticket/article_policy.rb index fa9fa5ff2..a5adc846e 100644 --- a/app/policies/ticket/article_policy.rb +++ b/app/policies/ticket/article_policy.rb @@ -26,13 +26,25 @@ class Ticket::ArticlePolicy < ApplicationPolicy return missing_admin_permission if !user.permissions?('ticket.agent') return missing_admin_permission if record.created_by_id != user.id return missing_admin_permission if record.type.communication? && !record.internal? - return too_old_to_undo if record.created_at <= 10.minutes.ago + return too_old_to_undo if deletable_timeframe? && record.created_at <= deletable_timeframe.ago true end private + def deletable_timeframe_setting + Setting.get('ui_ticket_zoom_article_delete_timeframe') + end + + def deletable_timeframe? + deletable_timeframe_setting&.positive? + end + + def deletable_timeframe + deletable_timeframe_setting.seconds + end + def access?(query) if record.internal == true && user.permissions?('ticket.customer') return false diff --git a/db/migrate/20200413160113_issue_2990_delete_timeframe.rb b/db/migrate/20200413160113_issue_2990_delete_timeframe.rb new file mode 100644 index 000000000..9af8bca68 --- /dev/null +++ b/db/migrate/20200413160113_issue_2990_delete_timeframe.rb @@ -0,0 +1,16 @@ +class Issue2990DeleteTimeframe < ActiveRecord::Migration[5.2] + def change + Setting.create_if_not_exists( + title: 'Define timeframe where a own created note can get deleted.', + name: 'ui_ticket_zoom_article_delete_timeframe', + area: 'UI::TicketZoomArticle', + description: "Set timeframe in seconds. If it's set to 0 you can delete notes without time limits", + options: {}, + state: 600, + preferences: { + permission: ['admin.ui'] + }, + frontend: true + ) + end +end diff --git a/db/seeds/settings.rb b/db/seeds/settings.rb index 8b3639a9b..4273751c4 100644 --- a/db/seeds/settings.rb +++ b/db/seeds/settings.rb @@ -4536,3 +4536,16 @@ Setting.create_if_not_exists( }, frontend: true ) + +Setting.create_if_not_exists( + title: 'Define timeframe where a own created note can get deleted.', + name: 'ui_ticket_zoom_article_delete_timeframe', + area: 'UI::TicketZoomArticle', + description: "Set timeframe in seconds. If it's set to 0 you can delete notes without time limits", + options: {}, + state: 600, + preferences: { + permission: ['admin.ui'] + }, + frontend: true +) diff --git a/spec/requests/ticket/article_spec.rb b/spec/requests/ticket/article_spec.rb index 8e78469ee..d8ff5ecae 100644 --- a/spec/requests/ticket/article_spec.rb +++ b/spec/requests/ticket/article_spec.rb @@ -482,16 +482,7 @@ AAAFCAYAAACNbyblAAAAHElEQVQI12P4//8/w38GIAXDIBKE0DHxgljNBAAO describe 'DELETE /api/v1/ticket_articles/:id', authenticated_as: -> { user } do let(:ticket) do - output = create(:ticket) - - # make group ticket was created in available to current user - role = user.roles.first - map = role.group_ids_access_map - map[output.group.id] = :full - role.group_ids_access_map = map - role.save! - - output + create(:ticket, group: Group.first) end let(:article_communication) do @@ -617,5 +608,53 @@ AAAFCAYAAACNbyblAAAAHElEQVQI12P4//8/w38GIAXDIBKE0DHxgljNBAAO now: false, later: false, much_later: false end + + context 'with custom timeframe' do + before { Setting.set 'ui_ticket_zoom_article_delete_timeframe', 6000 } + + let(:article) { article_note } + + context 'as admin' do + let(:user) { admin_user } + + context 'deleting after timeframe' do + before { article && travel(8000.seconds) } + + include_examples 'succeeds' + end + end + + context 'as agent' do + let(:user) { agent_user } + + context 'deleting before timeframe' do + before { article && travel(5000.seconds) } + + include_examples 'succeeds' + end + + context 'deleting after timeframe' do + before { article && travel(8000.seconds) } + + include_examples 'fails' + end + end + end + + context 'with timeframe as 0' do + before { Setting.set 'ui_ticket_zoom_article_delete_timeframe', 0 } + + let(:article) { article_note } + + context 'as agent' do + let(:user) { agent_user } + + context 'deleting long after' do + before { article && travel(99.days) } + + include_examples 'succeeds' + end + end + end end end diff --git a/spec/system/ticket/zoom_spec.rb b/spec/system/ticket/zoom_spec.rb index 443870166..bf3116fb8 100644 --- a/spec/system/ticket/zoom_spec.rb +++ b/spec/system/ticket/zoom_spec.rb @@ -208,9 +208,9 @@ RSpec.describe 'Ticket zoom', type: :system do end describe 'delete article', authenticated: -> { user } do - let(:admin_user) { User.find_by! email: 'master@example.com' } - let(:agent_user) { create :agent, password: 'test', groups: [Group.first] } - let(:customer_user) { create :customer, password: 'test' } + let(:admin_user) { create :admin, groups: [Group.first] } + let(:agent_user) { create :agent, groups: [Group.first] } + let(:customer_user) { create :customer } let(:ticket) { create :ticket, group: agent_user.groups.first, customer: customer_user } let(:article) { send(item) } @@ -345,6 +345,53 @@ RSpec.describe 'Ticket zoom', type: :system do now: false, later: false, much_later: false end + + context 'with custom offset' do + before { Setting.set 'ui_ticket_zoom_article_delete_timeframe', 6000 } + + context 'as admin' do + let(:user) { admin_user } + + include_examples 'according to permission matrix', item: 'article_note', expects_visible: true, offset: 8000.seconds, description: 'outside of delete timeframe' + end + + context 'as agent' do + let(:user) { agent_user } + + include_examples 'according to permission matrix', item: 'article_note', expects_visible: true, offset: 5000.seconds, description: 'outside of delete timeframe' + include_examples 'according to permission matrix', item: 'article_note', expects_visible: false, offset: 8000.seconds, description: 'outside of delete timeframe' + end + end + + context 'with timeframe as 0' do + before { Setting.set 'ui_ticket_zoom_article_delete_timeframe', 0 } + + context 'as agent' do + let(:user) { agent_user } + + include_examples 'according to permission matrix', item: 'article_note', expects_visible: true, offset: 99.days, description: 'long after' + end + end + end + + context 'button is hidden on the go' do + before { Setting.set 'ui_ticket_zoom_article_delete_timeframe', 5 } + + let(:user) { agent_user } + let(:item) { 'article_note' } + let!(:article) { send(item) } + let(:offset) { 0.seconds } + + it 'successfully' do + refresh # make sure user roles are loaded + + visit "ticket/zoom/#{ticket.id}" + + within :active_ticket_article, article do + find '.js-ArticleAction[data-type=delete]' # make sure delete button did show up + expect(page).to have_no_css('.js-ArticleAction[data-type=delete]', wait: 15) + end + end end end end