Fixes #4544 - Agents see macros although they have no change permissions

app/assets/javascripts/app/controllers/search.coffee

@@ -105,15 +105,16 @@ class App.Search extends App.Controller
       tabs: @tabs
     ))
 
-    @controllerTicketBatch.releaseController() if @controllerTicketBatch
-    @controllerTicketBatch = new App.TicketBatch(
-      el:       elLocal.filter('.js-batch-overlay')
-      parent:   @
-      parentEl: elLocal
-      appEl:    @appEl
-      batchSuccess: =>
-        @search(0, true)
-    )
+    if App.User.current().permission('ticket.agent')
+      @controllerTicketBatch.releaseController() if @controllerTicketBatch
+      @controllerTicketBatch = new App.TicketBatch(
+        el:       elLocal.filter('.js-batch-overlay')
+        parent:   @
+        parentEl: elLocal
+        appEl:    @appEl
+        batchSuccess: =>
+          @search(0, true)
+      )
 
     @html elLocal
     if @query

app/assets/javascripts/app/controllers/ticket_overview.coffee

@@ -33,15 +33,16 @@ class App.TicketOverview extends App.Controller
       view: @view
     )
 
-    @controllerTicketBatch.releaseController() if @controllerTicketBatch
-    @controllerTicketBatch = new App.TicketBatch(
-      el:           elLocal.filter('.js-batch-overlay')
-      parent:       @
-      parentEl:     elLocal
-      appEl:        @appEl
-      batchSuccess: =>
-        @render()
-    )
+    if App.User.current().permission('ticket.agent')
+      @controllerTicketBatch.releaseController() if @controllerTicketBatch
+      @controllerTicketBatch = new App.TicketBatch(
+        el:           elLocal.filter('.js-batch-overlay')
+        parent:       @
+        parentEl:     elLocal
+        appEl:        @appEl
+        batchSuccess: =>
+          @render()
+      )
 
     @contentController.releaseController() if @contentController
     @contentController = new Table(

app/assets/javascripts/app/controllers/widget/ticket_batch.coffee

@@ -43,35 +43,35 @@ class App.TicketBatch extends App.Controller
     ))
 
   renderOptionsMacros: =>
+    ticketsGroupIds = @parentEl
+      .find('[name="bulk"]:checked')
+      .toArray()
+      .map (elem) ->
+        App.Ticket.find(elem.value).group_id
 
-    @possibleMacros = []
-    macros          = App.Macro.getList()
+    userGroupIds = App.User.current().allGroupIds('change').map (elem) -> parseInt(elem)
 
-    items = @parentEl.find('[name="bulk"]:checked')
+    if !_.isEmpty(_.difference(ticketsGroupIds, userGroupIds))
+      @batchMacro.html $(App.view('ticket_overview/batch_overlay_macro')(
+        errorMessage: __('You have no change permission or you are a customer for some of the selected tickets.')
+      ))
 
-    group_ids =[]
-    for item in items
-      ticket = App.Ticket.find($(item).val())
-      group_ids.push ticket.group_id
-
-    group_ids = _.uniq(group_ids)
-
-    for macro in macros
+      return
 
-      # push if no group_ids exists
-      if _.isEmpty(macro.group_ids) && !_.includes(@possibleMacros, macro)
-        @possibleMacros.push macro
+    possibleMacros = App.Macro
+      .getList()
+      .filter (elem) ->
+        _.isEmpty(elem.group_ids) || _.isEmpty(_.difference(ticketsGroupIds, elem.group_ids))
 
-      # push if group_ids are equal
-      if _.isEqual(macro.group_ids, group_ids) && !_.includes(@possibleMacros, macro)
-        @possibleMacros.push macro
+    if _.isEmpty(possibleMacros)
+      @batchMacro.html $(App.view('ticket_overview/batch_overlay_macro')(
+        errorMessage: __('Selected tickets do not match any macros.')
+      ))
 
-      # push if all group_ids of tickets are in macro.group_ids
-      if !_.isEmpty(macro.group_ids) && _.isEmpty(_.difference(group_ids,macro.group_ids)) && !_.includes(@possibleMacros, macro)
-        @possibleMacros.push macro
+      return
 
     @batchMacro.html $(App.view('ticket_overview/batch_overlay_macro')(
-      macros: @possibleMacros
+      macros: possibleMacros
     ))
 
   startDragItem: (event) =>

app/assets/javascripts/app/views/ticket_overview/batch_overlay_macro.jst.eco

@@ -1,8 +1,18 @@
 <div class="batch-overlay-box-inner">
-<% isSmall = @macros.length > 20 %>
-<% for macro in @macros: %>
-  <div class="batch-overlay-macro-entry js-batch-overlay-entry js-batch-hover-target <%- 'small' if isSmall %>" data-action="macro" data-id="<%= macro.id %>">
-    <div class="batch-overlay-macro-entry-name"><%= macro.name %></div>
+<% if _.isEmpty(@macros): %>
+  <div class="batch-overlay-macro-error-message">
+    <%= @T('No macros available.') %>
+    <% if @errorMessage: %>
+      <br>
+      <%= @T(@errorMessage) %>
+    <% end %>
   </div>
+<% else: %>
+  <% isSmall = @macros.length > 20 %>
+  <% for macro in @macros: %>
+    <div class="batch-overlay-macro-entry js-batch-overlay-entry js-batch-hover-target <%- 'small' if isSmall %>" data-action="macro" data-id="<%= macro.id %>">
+      <div class="batch-overlay-macro-entry-name"><%= macro.name %></div>
+    </div>
+  <% end %>
 <% end %>
-</div>
+</div>

app/assets/stylesheets/zammad.scss

@@ -12860,6 +12860,16 @@ output {
         }
       }
     }
+
+    &-error-message {
+      margin: 13px;
+      height: 120px;
+      padding: 13px 13px 10px;
+      display: flex;
+      align-items: center;
+      justify-content: center;
+      font-size: 0.9em;
+    }
   }
 }
 

app/controllers/tickets_mass_controller.rb

@@ -43,7 +43,7 @@ class TicketsMassController < ApplicationController
     @tickets = Ticket.find params[:ticket_ids]
 
     @tickets.each do |elem|
-      authorize!(elem, :follow_up?)
+      authorize!(elem, :agent_update_access?)
     end
   rescue Pundit::NotAuthorizedError => e
     render json: { error: true, ticket_id: e.record.id }, status: :unprocessable_entity

i18n/zammad.pot

@@ -7744,6 +7744,10 @@ msgstr ""
 msgid "No linked issues"
 msgstr ""
 
+#: app/assets/javascripts/app/views/ticket_overview/batch_overlay_macro.jst.eco
+msgid "No macros available."
+msgstr ""
+
 #: app/frontend/apps/mobile/pages/search/views/SearchOverview.vue
 msgid "No previous searches"
 msgstr ""
@@ -9749,6 +9753,10 @@ msgstr ""
 msgid "Selected conditions"
 msgstr ""
 
+#: app/assets/javascripts/app/controllers/widget/ticket_batch.coffee
+msgid "Selected tickets do not match any macros."
+msgstr ""
+
 #: app/assets/javascripts/app/models/trigger.coffee
 msgid "Selective (default)"
 msgstr ""
@@ -13479,6 +13487,10 @@ msgstr ""
 msgid "You have insufficient rights to view this user."
 msgstr ""
 
+#: app/assets/javascripts/app/controllers/widget/ticket_batch.coffee
+msgid "You have no change permission or you are a customer for some of the selected tickets."
+msgstr ""
+
 #: app/assets/javascripts/app/views/telegram/index.jst.eco
 msgid "You have no configured %s right now."
 msgstr ""

spec/system/search_spec.rb

@@ -195,6 +195,93 @@ RSpec.describe 'Search', authenticated: true, searchindex: true, type: :system d
       end
     end
 
+    describe 'when agent cannot change some of the tickets' do
+      def authenticate
+        ticket_1
+        macro_without_group
+        agent
+
+        # add ticket after agent is created to not add default access for agent
+        agent.user_groups.create! group: ticket_3.group, access: 'read'
+
+        agent
+      end
+
+      it 'show macros if agent cannot change selected tickets' do
+        display_macro_batches ticket_1
+
+        within(:active_content) do
+          expect(page).to have_no_text(%r{No macros available}i)
+            .and(have_selector(:macro_batch, macro_without_group.id))
+        end
+      end
+
+      it 'show no macros if agent cannot change selected tickets' do
+        display_macro_batches ticket_3
+
+        within(:active_content) do
+          expect(page).to have_text(%r{No macros available}i)
+            .and(have_text(%r{no change permission}i))
+            .and(have_no_selector(:macro_batch, macro_without_group.id))
+        end
+      end
+    end
+
+    describe 'when user is agent-customer' do
+      let(:agent_customer) { create(:agent_and_customer) }
+
+      def authenticate
+        ticket_1.update!(customer: agent_customer)
+        ticket_2
+
+        macro_without_group && macro_group1 && macro_group2
+
+        agent_customer
+          .tap { |user| user.groups << group_2 }
+      end
+
+      it 'show no macros if the ticket is customer-like' do
+        display_macro_batches ticket_1
+
+        within :active_content do
+          expect(page).to have_text(%r{No macros available}i)
+            .and(have_text(%r{no change permission}i))
+            .and(have_no_selector(:macro_batch, macro_without_group.id))
+            .and(have_no_selector(:macro_batch, macro_group1.id))
+            .and(have_no_selector(:macro_batch, macro_group2.id))
+        end
+      end
+
+      it 'show macros if tickets are only agent-like' do
+        display_macro_batches ticket_2
+
+        within :active_content do
+          expect(page).to have_no_text(%r{No macros available}i)
+            .and(have_selector(:macro_batch, macro_without_group.id))
+            .and(have_no_selector(:macro_batch, macro_group1.id))
+            .and(have_selector(:macro_batch, macro_group2.id))
+        end
+      end
+    end
+
+    describe 'when user is customer' do
+      let(:customer) { create(:customer) }
+
+      def authenticate
+        ticket_1.update!(customer: customer)
+
+        customer
+      end
+
+      it 'shows no overlay' do
+        display_macro_batches ticket_1
+
+        within :active_content do
+          expect(page).to have_no_selector('.batch-overlay-backdrop')
+        end
+      end
+    end
+
     context 'with macro batch overlay' do
       shared_examples "adding 'small' class to macro element" do
         it 'adds a "small" class to the macro element' do

spec/system/ticket/view_spec.rb

@@ -108,6 +108,104 @@ RSpec.describe 'Ticket views', authenticated_as: :authenticate, type: :system do
       end
     end
 
+    describe 'when agent cannot change some of the tickets' do
+      let(:agent) { create(:agent) }
+
+      def authenticate
+        macro_without_group
+
+        agent
+          .tap { |user| user.user_groups.create! group: ticket1.group, access: 'full' }
+          .tap { |user| user.user_groups.create! group: ticket2.group, access: 'overview' }
+      end
+
+      before do
+        visit '#ticket/view/all_open'
+      end
+
+      it 'show macros if agent cannot change selected tickets' do
+        display_macro_batches ticket1
+
+        within(:active_content) do
+          expect(page).to have_no_text(%r{No macros available}i)
+            .and(have_selector(:macro_batch, macro_without_group.id))
+        end
+      end
+
+      it 'show no macros if agent cannot change selected tickets' do
+        display_macro_batches ticket2
+
+        within(:active_content) do
+          expect(page).to have_text(%r{No macros available}i)
+            .and(have_text(%r{no change permission}i))
+            .and(have_no_selector(:macro_batch, macro_without_group.id))
+        end
+      end
+    end
+
+    describe 'when user is agent-customer' do
+      let(:agent_customer) { create(:agent_and_customer) }
+
+      def authenticate
+        ticket1.update!(customer: agent_customer)
+        ticket2
+
+        macro_without_group && macro_group1 && macro_group2
+
+        agent_customer
+          .tap { |user| user.groups << ticket2.group }
+      end
+
+      before do
+        visit '#ticket/view/all_open'
+      end
+
+      it 'show no macros if the ticket is customer-like' do
+        display_macro_batches ticket1
+
+        within :active_content do
+          expect(page).to have_text(%r{No macros available}i)
+            .and(have_text(%r{no change permission}i))
+            .and(have_no_selector(:macro_batch, macro_without_group.id))
+            .and(have_no_selector(:macro_batch, macro_group1.id))
+            .and(have_no_selector(:macro_batch, macro_group2.id))
+        end
+      end
+
+      it 'show macros if tickets are only agent-like' do
+        display_macro_batches ticket2
+
+        within :active_content do
+          expect(page).to have_no_text(%r{No macros available}i)
+            .and(have_selector(:macro_batch, macro_without_group.id))
+            .and(have_no_selector(:macro_batch, macro_group1.id))
+            .and(have_selector(:macro_batch, macro_group2.id))
+        end
+      end
+    end
+
+    describe 'when user is customer' do
+      let(:customer) { create(:customer) }
+
+      def authenticate
+        ticket1.update!(customer: customer)
+
+        customer
+      end
+
+      before do
+        visit '#ticket/view/my_tickets'
+      end
+
+      it 'shows no overlay' do
+        display_macro_batches ticket1
+
+        within :active_content do
+          expect(page).to have_no_selector('.batch-overlay-backdrop')
+        end
+      end
+    end
+
     context 'with macro batch overlay' do
       shared_examples "adding 'small' class to macro element" do
         it 'adds a "small" class to the macro element' do