Fixes #2990 - Backward compatibility of deleting own notes

app/assets/javascripts/app/controllers/ticket_zoom/article_action/delete.coffee

@@ -19,8 +19,8 @@ class Delete
 
   @isDeletable: (actions, ticket, article, ui) ->
     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) ->
+    timeframe_miliseconds = @deletableTimeframeSetting() * 1000
+    now                   = Date.parse(new Date())
     created_at            = Date.parse(article.created_at)
-    600000 - (Date.parse(new Date()) - created_at)
+
+    timeframe_miliseconds - (now - created_at)
 
   @deletableForAgent: (actions, ticket, article, ui) ->
     return false if !ui.permissionCheck('ticket.agent')

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

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

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
+)

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)
+      create(:ticket, group: Group.first)
-
-      # 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
     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

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(:admin_user)    { create :admin, groups: [Group.first] }
-    let(:agent_user)    { create :agent, password: 'test', groups: [Group.first] }
+    let(:agent_user)    { create :agent, groups: [Group.first] }
-    let(:customer_user) { create :customer, password: 'test' }
+    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