diff --git a/app/assets/javascripts/app/controllers/customer_ticket_create.coffee b/app/assets/javascripts/app/controllers/customer_ticket_create.coffee index fba6fb8ea..b7be104c8 100644 --- a/app/assets/javascripts/app/controllers/customer_ticket_create.coffee +++ b/app/assets/javascripts/app/controllers/customer_ticket_create.coffee @@ -212,4 +212,15 @@ class Index extends App.ControllerContent @formEnable(@$('.js-submit'), 'button') App.Config.set('customer_ticket_new', Index, 'Routes') -App.Config.set('CustomerTicketNew', { prio: 8003, parent: '#new', name: 'New Ticket', translate: true, target: '#customer_ticket_new', permission: ['ticket.customer'], setting: ['customer_ticket_create'], divider: true }, 'NavBarRight') +App.Config.set('CustomerTicketNew', { + prio: 8003, + parent: '#new', + name: 'New Ticket', + translate: true, + target: '#customer_ticket_new', + permission: (navigation) -> + return false if navigation.permissionCheck('ticket.agent') + return navigation.permissionCheck('ticket.customer') + setting: ['customer_ticket_create'], + divider: true +}, 'NavBarRight') diff --git a/app/assets/javascripts/app/controllers/navigation.coffee b/app/assets/javascripts/app/controllers/navigation.coffee index 8a01ada8e..7626cf799 100644 --- a/app/assets/javascripts/app/controllers/navigation.coffee +++ b/app/assets/javascripts/app/controllers/navigation.coffee @@ -361,6 +361,7 @@ class App.Navigation extends App.ControllerWidgetPermanent filterNavbarPermissionOk: (item) -> return true unless item.permission + return item.permission(@) if typeof item.permission is 'function' return _.any item.permission, (permissionName) => return @permissionCheck(permissionName) diff --git a/app/assets/javascripts/app/controllers/ticket_overview.coffee b/app/assets/javascripts/app/controllers/ticket_overview.coffee index 574ad965a..0fa8e19d6 100644 --- a/app/assets/javascripts/app/controllers/ticket_overview.coffee +++ b/app/assets/javascripts/app/controllers/ticket_overview.coffee @@ -1049,7 +1049,7 @@ class Table extends App.Controller ticketListShow.push App.Ticket.find(ticket.id) # if customer and no ticket exists, show the following message only - if !ticketListShow[0] && @permissionCheck('ticket.customer') + if !ticketListShow[0] && !@permissionCheck('ticket.agent') @html App.view('customer_not_ticket_exists')() return @@ -1057,27 +1057,26 @@ class Table extends App.Controller @overview = App.Overview.find(overview.id) # render init page - checkbox = true + checkbox = false edit = false if @permissionCheck('admin.overview') edit = true - if @permissionCheck('ticket.customer') - checkbox = false - edit = false - view_modes = [ - { - name: 'S' - type: 's' - class: 'active' if @view_mode is 's' - }, - { - name: 'M' - type: 'm' - class: 'active' if @view_mode is 'm' - } - ] - if @permissionCheck('ticket.customer') - view_modes = [] + if @permissionCheck('ticket.agent') + checkbox = true + view_modes = [] + if @permissionCheck('ticket.agent') + view_modes = [ + { + name: 'S' + type: 's' + class: 'active' if @view_mode is 's' + }, + { + name: 'M' + type: 'm' + class: 'active' if @view_mode is 'm' + } + ] html = App.view('agent_ticket_view/content')( overview: @overview view_modes: view_modes diff --git a/app/assets/javascripts/app/controllers/ticket_zoom.coffee b/app/assets/javascripts/app/controllers/ticket_zoom.coffee index a6e685d1d..9ab37aac9 100644 --- a/app/assets/javascripts/app/controllers/ticket_zoom.coffee +++ b/app/assets/javascripts/app/controllers/ticket_zoom.coffee @@ -137,23 +137,20 @@ class App.TicketZoom extends App.Controller ) load: (data, ignoreSame = false, local = false) => - - # check if ticket has changed newTicketRaw = data.assets.Ticket[@ticket_id] - #console.log(newTicketRaw.updated_at) - #console.log(@ticketUpdatedAtLastCall) + loadAssets = true if @ticketUpdatedAtLastCall # ignore if record is already shown if ignoreSame && new Date(newTicketRaw.updated_at).getTime() is new Date(@ticketUpdatedAtLastCall).getTime() #console.log('debug no fetched, current ticket already there or requested') - return + loadAssets = false # do not render if newer ticket is already requested if new Date(newTicketRaw.updated_at).getTime() < new Date(@ticketUpdatedAtLastCall).getTime() #console.log('fetched no fetch, current ticket already newer') - return + loadAssets = false # remember current record if newer as requested record if new Date(newTicketRaw.updated_at).getTime() > new Date(@ticketUpdatedAtLastCall).getTime() @@ -161,35 +158,53 @@ class App.TicketZoom extends App.Controller else @ticketUpdatedAtLastCall = newTicketRaw.updated_at - # notify if ticket changed not by my self - if @initFetched - if newTicketRaw.updated_by_id isnt @Session.get('id') - App.TaskManager.notify(@taskKey) - @initFetched = true - - if !@doNotLog - @doNotLog = 1 - @recentView('Ticket', @ticket_id) - - # remember article ids - @ticket_article_ids = data.ticket_article_ids - - # remember link - @links = data.links - - # remember tags - @tags = data.tags - - # get edit form attributes - @formMeta = data.form_meta - # load assets - App.Collection.loadAssets(data.assets, targetModel: 'Ticket') + if loadAssets - # get data - @ticket = App.Ticket.fullLocal(@ticket_id) + # notify if ticket changed not by my self + if @initFetched + if newTicketRaw.updated_by_id isnt @Session.get('id') + App.TaskManager.notify(@taskKey) + @initFetched = true + + if !@doNotLog + @doNotLog = 1 + @recentView('Ticket', @ticket_id) + + # remember article ids + @ticket_article_ids = data.ticket_article_ids + + # remember link + @links = data.links + + # remember tags + @tags = data.tags + + App.Collection.loadAssets(data.assets, targetModel: 'Ticket') + + # get ticket + @ticket = App.Ticket.fullLocal(@ticket_id) @ticket.article = undefined + view = @ticket.currentView() + readable = @ticket.userGroupAccess('read') + changeable = @ticket.userGroupAccess('change') + fullable = @ticket.userGroupAccess('full') + formMeta = data.form_meta + + # on the following states we want to rerender the ticket: + # - if the object attribute configuration has changed (attribute values, restrictions, filters) + # - if the user view has changed (agent/customer) + # - if the ticket permission has changed (read/write/full) + if @view && ( !_.isEqual(@formMeta, formMeta) || @view isnt view || @readable isnt readable || @changeable isnt changeable || @fullable isnt fullable ) + @renderDone = false + + @view = view + @readable = readable + @changeable = changeable + @fullable = fullable + @formMeta = formMeta + # render page @render(local) @@ -410,7 +425,6 @@ class App.TicketZoom extends App.Controller elLocal = $(App.view('ticket_zoom') ticket: @ticket nav: @nav - isCustomer: @permissionCheck('ticket.customer') scrollbarWidth: App.Utils.getScrollBarWidth() dir: App.i18n.dir() ) @@ -460,6 +474,7 @@ class App.TicketZoom extends App.Controller @highligher = new App.TicketZoomHighlighter( el: elLocal.find('.js-highlighterContainer') + ticket: @ticket ticket_id: @ticket_id ) @@ -611,12 +626,12 @@ class App.TicketZoom extends App.Controller subject: '' type: 'note' body: '' - internal: internal + internal: '' in_reply_to: '' subtype: '' - if @permissionCheck('ticket.customer') - currentStore.article.internal = '' + if @ticket.currentView() is 'agent' + currentStore.article.internal = internal currentStore @@ -637,7 +652,7 @@ class App.TicketZoom extends App.Controller return if modelDiff.ticket.state_id # and we are in the customer interface - return if !@permissionCheck('ticket.customer') + return if @ticket.currentView() isnt 'customer' # and the default is was not set before return if @isDefaultFollowUpStateSet @@ -676,7 +691,7 @@ class App.TicketZoom extends App.Controller delete currentParams.article.form_id - if @permissionCheck('ticket.customer') + if @ticket.currentView() is 'customer' currentParams.article.internal = '' currentParams @@ -802,7 +817,7 @@ class App.TicketZoom extends App.Controller ) # set defaults - if !@permissionCheck('ticket.customer') + if ticket.currentView() is 'agent' if !ticket['owner_id'] ticket['owner_id'] = 1 @@ -875,7 +890,7 @@ class App.TicketZoom extends App.Controller return # time tracking - if @permissionCheck('ticket.customer') + if ticket.currentView() is 'customer' @submitPost(e, ticket, macro) return 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 651a945f2..5097b4d4f 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 @@ -41,7 +41,7 @@ class Delete timeframe_miliseconds - (now - created_at) @deletableForAgent: (actions, ticket, article, ui) -> - return false if !ui.permissionCheck('ticket.agent') + return false if ticket.currentView() is 'customer' return false if article.created_by_id != App.User.current()?.id return false if article.type.communication diff --git a/app/assets/javascripts/app/controllers/ticket_zoom/article_action/email_reply.coffee b/app/assets/javascripts/app/controllers/ticket_zoom/article_action/email_reply.coffee index 584ecfc70..52abad1f7 100644 --- a/app/assets/javascripts/app/controllers/ticket_zoom/article_action/email_reply.coffee +++ b/app/assets/javascripts/app/controllers/ticket_zoom/article_action/email_reply.coffee @@ -1,6 +1,6 @@ class EmailReply extends App.Controller @action: (actions, ticket, article, ui) -> - return actions if !ui.permissionCheck('ticket.agent') + return actions if ticket.currentView() is 'customer' group = ticket.group return actions if !group.email_address_id @@ -241,7 +241,7 @@ class EmailReply extends App.Controller true @articleTypes: (articleTypes, ticket, ui) -> - return articleTypes if !ui.permissionCheck('ticket.agent') + return articleTypes if ticket.currentView() is 'customer' group = ticket.group return articleTypes if !group.email_address_id diff --git a/app/assets/javascripts/app/controllers/ticket_zoom/article_action/facebook_reply.coffee b/app/assets/javascripts/app/controllers/ticket_zoom/article_action/facebook_reply.coffee index e5cfc598d..7399a3966 100644 --- a/app/assets/javascripts/app/controllers/ticket_zoom/article_action/facebook_reply.coffee +++ b/app/assets/javascripts/app/controllers/ticket_zoom/article_action/facebook_reply.coffee @@ -1,6 +1,6 @@ class FacebookReply @action: (actions, ticket, article, ui) -> - return actions if ui.permissionCheck('ticket.customer') + return actions if ticket.currentView() is 'customer' if article.type.name is 'facebook feed post' || article.type.name is 'facebook feed comment' actions.push { @@ -35,7 +35,7 @@ class FacebookReply true @articleTypes: (articleTypes, ticket, ui) -> - return articleTypes if !ui.permissionCheck('ticket.agent') + return articleTypes if ticket.currentView() is 'customer' return articleTypes if !ticket || !ticket.create_article_type_id diff --git a/app/assets/javascripts/app/controllers/ticket_zoom/article_action/internal.coffee b/app/assets/javascripts/app/controllers/ticket_zoom/article_action/internal.coffee index 8fc1e88bd..b748be467 100644 --- a/app/assets/javascripts/app/controllers/ticket_zoom/article_action/internal.coffee +++ b/app/assets/javascripts/app/controllers/ticket_zoom/article_action/internal.coffee @@ -1,6 +1,6 @@ class Internal @action: (actions, ticket, article, ui) -> - return actions if ui.permissionCheck('ticket.customer') + return actions if ticket.currentView() is 'customer' if article.internal is true actions.push { diff --git a/app/assets/javascripts/app/controllers/ticket_zoom/article_action/note.coffee b/app/assets/javascripts/app/controllers/ticket_zoom/article_action/note.coffee index e5b775056..76cafd409 100644 --- a/app/assets/javascripts/app/controllers/ticket_zoom/article_action/note.coffee +++ b/app/assets/javascripts/app/controllers/ticket_zoom/article_action/note.coffee @@ -7,7 +7,7 @@ class Note @articleTypes: (articleTypes, ticket, ui) -> internal = false - if ui.permissionCheck('ticket.agent') + if ticket.currentView() is 'agent' internal = ui.Config.get('ui_ticket_zoom_article_note_new_internal') articleTypes.push { diff --git a/app/assets/javascripts/app/controllers/ticket_zoom/article_action/phone_reply.coffee b/app/assets/javascripts/app/controllers/ticket_zoom/article_action/phone_reply.coffee index b2f35e2fd..6c764de10 100644 --- a/app/assets/javascripts/app/controllers/ticket_zoom/article_action/phone_reply.coffee +++ b/app/assets/javascripts/app/controllers/ticket_zoom/article_action/phone_reply.coffee @@ -6,7 +6,7 @@ class PhoneReply true @articleTypes: (articleTypes, ticket, ui) -> - return articleTypes if !ui.permissionCheck('ticket.agent') + return articleTypes if ticket.currentView() is 'customer' articleTypes.push { name: 'phone' icon: 'phone' diff --git a/app/assets/javascripts/app/controllers/ticket_zoom/article_action/sms_reply.coffee b/app/assets/javascripts/app/controllers/ticket_zoom/article_action/sms_reply.coffee index 016bc8af5..c6f1e6bde 100644 --- a/app/assets/javascripts/app/controllers/ticket_zoom/article_action/sms_reply.coffee +++ b/app/assets/javascripts/app/controllers/ticket_zoom/article_action/sms_reply.coffee @@ -1,6 +1,6 @@ class SmsReply @action: (actions, ticket, article, ui) -> - return actions if ui.permissionCheck('ticket.customer') + return actions if ticket.currentView() is 'customer' if article.sender.name is 'Customer' && article.type.name is 'sms' actions.push { @@ -43,7 +43,7 @@ class SmsReply true @articleTypes: (articleTypes, ticket, ui) -> - return articleTypes if !ui.permissionCheck('ticket.agent') + return articleTypes if ticket.currentView() is 'customer' return articleTypes if !ticket || !ticket.create_article_type_id diff --git a/app/assets/javascripts/app/controllers/ticket_zoom/article_action/split.coffee b/app/assets/javascripts/app/controllers/ticket_zoom/article_action/split.coffee index 716267dd2..8620dc0ad 100644 --- a/app/assets/javascripts/app/controllers/ticket_zoom/article_action/split.coffee +++ b/app/assets/javascripts/app/controllers/ticket_zoom/article_action/split.coffee @@ -1,6 +1,6 @@ class Split @action: (actions, ticket, article, ui) -> - return actions if ui.permissionCheck('ticket.customer') + return actions if ticket.currentView() is 'customer' actions.push { name: 'split' diff --git a/app/assets/javascripts/app/controllers/ticket_zoom/article_action/telegram.coffee b/app/assets/javascripts/app/controllers/ticket_zoom/article_action/telegram.coffee index 90c491ebd..91d781cb7 100644 --- a/app/assets/javascripts/app/controllers/ticket_zoom/article_action/telegram.coffee +++ b/app/assets/javascripts/app/controllers/ticket_zoom/article_action/telegram.coffee @@ -1,6 +1,6 @@ class TelegramReply @action: (actions, ticket, article, ui) -> - return actions if ui.permissionCheck('ticket.customer') + return actions if ticket.currentView() is 'customer' if article.sender.name is 'Customer' && article.type.name is 'telegram personal-message' actions.push { @@ -43,7 +43,7 @@ class TelegramReply true @articleTypes: (articleTypes, ticket, ui) -> - return articleTypes if !ui.permissionCheck('ticket.agent') + return articleTypes if ticket.currentView() is 'customer' return articleTypes if !ticket || !ticket.create_article_type_id diff --git a/app/assets/javascripts/app/controllers/ticket_zoom/article_action/twitter_reply.coffee b/app/assets/javascripts/app/controllers/ticket_zoom/article_action/twitter_reply.coffee index 04ee715ff..e5b027196 100644 --- a/app/assets/javascripts/app/controllers/ticket_zoom/article_action/twitter_reply.coffee +++ b/app/assets/javascripts/app/controllers/ticket_zoom/article_action/twitter_reply.coffee @@ -1,6 +1,6 @@ class TwitterReply @action: (actions, ticket, article, ui) -> - return actions if ui.permissionCheck('ticket.customer') + return actions if ticket.currentView() is 'customer' if article.type.name is 'twitter status' actions.push { @@ -126,7 +126,7 @@ class TwitterReply }) @articleTypes: (articleTypes, ticket, ui) -> - return articleTypes if !ui.permissionCheck('ticket.agent') + return articleTypes if ticket.currentView() is 'customer' return articleTypes if !ticket || !ticket.create_article_type_id diff --git a/app/assets/javascripts/app/controllers/ticket_zoom/article_new.coffee b/app/assets/javascripts/app/controllers/ticket_zoom/article_new.coffee index b40776355..c69a58c27 100644 --- a/app/assets/javascripts/app/controllers/ticket_zoom/article_new.coffee +++ b/app/assets/javascripts/app/controllers/ticket_zoom/article_new.coffee @@ -31,12 +31,12 @@ class App.TicketZoomArticleNew extends App.Controller constructor: -> super - @internalSelector = true + @internalSelector = false @type = @defaults['type'] || 'note' @setPossibleArticleTypes() - if @permissionCheck('ticket.customer') - @internalSelector = false + if @ticket.currentView() is 'agent' + @internalSelector = true @textareaHeight = open: 148 @@ -165,7 +165,7 @@ class App.TicketZoomArticleNew extends App.Controller articleTypes: @articleTypes article: @defaults form_id: @form_id - isCustomer: @permissionCheck('ticket.customer') + isCustomer: ticket.currentView() is 'customer' internalSelector: @internalSelector ) @setArticleTypePre(@type) @@ -246,7 +246,7 @@ class App.TicketZoomArticleNew extends App.Controller @bindAttachmentDelete() # show text module UI - if !@permissionCheck('ticket.customer') + if ticket.currentView() is 'agent' textModule = new App.WidgetTextModule( el: @$('.js-textarea').parent() data: @@ -272,16 +272,18 @@ class App.TicketZoomArticleNew extends App.Controller params.form_id = @form_id params.content_type = 'text/html' - if @permissionCheck('ticket.customer') - sender = App.TicketArticleSender.findByAttribute('name', 'Customer') - type = App.TicketArticleType.findByAttribute('name', 'web') - params.type_id = type.id - params.sender_id = sender.id - else + ticket = App.Ticket.find(@ticket_id) + + if ticket.currentView() is 'agent' sender = App.TicketArticleSender.findByAttribute('name', 'Agent') type = App.TicketArticleType.findByAttribute('name', params['type']) params.sender_id = sender.id params.type_id = type.id + else + sender = App.TicketArticleSender.findByAttribute('name', 'Customer') + type = App.TicketArticleType.findByAttribute('name', 'web') + params.type_id = type.id + params.sender_id = sender.id if params.internal params.internal = true diff --git a/app/assets/javascripts/app/controllers/ticket_zoom/attribute_bar.coffee b/app/assets/javascripts/app/controllers/ticket_zoom/attribute_bar.coffee index b66459686..0e95727f7 100644 --- a/app/assets/javascripts/app/controllers/ticket_zoom/attribute_bar.coffee +++ b/app/assets/javascripts/app/controllers/ticket_zoom/attribute_bar.coffee @@ -46,7 +46,7 @@ class App.TicketZoomAttributeBar extends App.Controller @macroLastUpdated = App.Macro.lastUpdatedAt() @possibleMacros = [] - if _.isEmpty(macros) || !@permissionCheck('ticket.agent') + if _.isEmpty(macros) || @ticket.currentView() is 'customer' macroDisabled = true else for macro in macros @@ -63,7 +63,7 @@ class App.TicketZoomAttributeBar extends App.Controller )) @setSecondaryAction(@secondaryAction, localeEl) - if @permissionCheck('ticket.agent') + if @ticket.currentView() is 'agent' @taskbarWatcher = new App.TaskbarWatcher( taskKey: @taskKey el: localeEl.filter('.js-avatars') diff --git a/app/assets/javascripts/app/controllers/ticket_zoom/highlighter.coffee b/app/assets/javascripts/app/controllers/ticket_zoom/highlighter.coffee index 36352ef05..1ff299a4e 100644 --- a/app/assets/javascripts/app/controllers/ticket_zoom/highlighter.coffee +++ b/app/assets/javascripts/app/controllers/ticket_zoom/highlighter.coffee @@ -36,7 +36,7 @@ class App.TicketZoomHighlighter extends App.Controller constructor: -> super - return if !@permissionCheck('ticket.agent') + return if @ticket.currentView() isnt 'agent' @currentHighlights = {} @@ -93,7 +93,7 @@ class App.TicketZoomHighlighter extends App.Controller # for testing purposes the highlights get stored in article preferences loadHighlights: (ticket_article_id) -> - return if !@permissionCheck('ticket.agent') + return if @ticket.currentView() isnt 'agent' article = App.TicketArticle.find(ticket_article_id) return if !article.preferences return if !article.preferences.highlight diff --git a/app/assets/javascripts/app/controllers/ticket_zoom/meta.coffee b/app/assets/javascripts/app/controllers/ticket_zoom/meta.coffee index ed2aa0228..ed6dfc590 100644 --- a/app/assets/javascripts/app/controllers/ticket_zoom/meta.coffee +++ b/app/assets/javascripts/app/controllers/ticket_zoom/meta.coffee @@ -8,5 +8,5 @@ class App.TicketZoomMeta extends App.ObserverController render: (ticket) => @html App.view('ticket_zoom/meta')( ticket: ticket - isCustomer: @permissionCheck('ticket.customer') + isCustomer: ticket.currentView() is 'customer' ) diff --git a/app/assets/javascripts/app/controllers/ticket_zoom/sidebar_customer.coffee b/app/assets/javascripts/app/controllers/ticket_zoom/sidebar_customer.coffee index 6189ea78a..d217d30ce 100644 --- a/app/assets/javascripts/app/controllers/ticket_zoom/sidebar_customer.coffee +++ b/app/assets/javascripts/app/controllers/ticket_zoom/sidebar_customer.coffee @@ -1,6 +1,6 @@ class SidebarCustomer extends App.Controller sidebarItem: => - return if !@permissionCheck('ticket.agent') + return if @ticket.currentView() isnt 'agent' @item = { name: 'customer' badgeCallback: @badgeRender diff --git a/app/assets/javascripts/app/controllers/ticket_zoom/sidebar_organization.coffee b/app/assets/javascripts/app/controllers/ticket_zoom/sidebar_organization.coffee index 357813ef4..bf1b5c179 100644 --- a/app/assets/javascripts/app/controllers/ticket_zoom/sidebar_organization.coffee +++ b/app/assets/javascripts/app/controllers/ticket_zoom/sidebar_organization.coffee @@ -1,6 +1,6 @@ class SidebarOrganization extends App.Controller sidebarItem: => - return if !@permissionCheck('ticket.agent') + return if @ticket.currentView() isnt 'agent' return if !@ticket.organization_id @item = { name: 'organization' diff --git a/app/assets/javascripts/app/controllers/ticket_zoom/sidebar_ticket.coffee b/app/assets/javascripts/app/controllers/ticket_zoom/sidebar_ticket.coffee index fd345caa8..fd64b3ed7 100644 --- a/app/assets/javascripts/app/controllers/ticket_zoom/sidebar_ticket.coffee +++ b/app/assets/javascripts/app/controllers/ticket_zoom/sidebar_ticket.coffee @@ -19,10 +19,10 @@ class Edit extends App.ObserverController if followUpPossible == 'new_ticket' && ticketState != 'closed' || followUpPossible != 'new_ticket' || - @permissionCheck('admin') || @permissionCheck('ticket.agent') + @permissionCheck('admin') || ticket.currentView() is 'agent' new App.ControllerForm( elReplace: @el - model: App.Ticket + model: { configure_attributes: @formMeta.configure_attributes } screen: 'edit' handlersConfig: handlers filter: @formMeta.filter @@ -35,7 +35,7 @@ class Edit extends App.ObserverController else new App.ControllerForm( elReplace: @el - model: App.Ticket + model: { configure_attributes: @formMeta.configure_attributes } screen: 'edit' handlersConfig: handlers filter: @formMeta.filter @@ -76,7 +76,7 @@ class SidebarTicket extends App.Controller sidebarHead: 'Ticket' sidebarCallback: @editTicket } - if @permissionCheck('ticket.agent') + if @ticket.currentView() is 'agent' @item.sidebarActions = [ { title: 'History' @@ -127,7 +127,7 @@ class SidebarTicket extends App.Controller taskKey: @taskKey ) - if @permissionCheck('ticket.agent') + if @ticket.currentView() is 'agent' @tagWidget = new App.WidgetTag( el: localEl.filter('.tags') object_type: 'Ticket' diff --git a/app/assets/javascripts/app/controllers/ticket_zoom/time_unit.coffee b/app/assets/javascripts/app/controllers/ticket_zoom/time_unit.coffee index b5ecd023c..d89ebe6b3 100644 --- a/app/assets/javascripts/app/controllers/ticket_zoom/time_unit.coffee +++ b/app/assets/javascripts/app/controllers/ticket_zoom/time_unit.coffee @@ -4,7 +4,7 @@ class App.TicketZoomTimeUnit extends App.ObserverController time_unit: true render: (ticket) => - return if !@permissionCheck('ticket.agent') + return if ticket.currentView() isnt 'agent' return if !ticket.time_unit @html App.view('ticket_zoom/time_unit')( ticket: ticket diff --git a/app/assets/javascripts/app/models/ticket.coffee b/app/assets/javascripts/app/models/ticket.coffee index 1564f9cd6..a74967cf1 100644 --- a/app/assets/javascripts/app/models/ticket.coffee +++ b/app/assets/javascripts/app/models/ticket.coffee @@ -305,12 +305,32 @@ class App.Ticket extends App.Model editable: (permission = 'change') -> user = App.User.current() return false if !user? - return true if user.id is @customer_id - return true if user.organization_id && @organization_id && user.organization_id is @organization_id - return false if !@group_id + return true if @currentView() is 'customer' && @userIsCustomer() + return true if @currentView() is 'customer' && user.organization_id && @organization_id && user.organization_id is @organization_id + return @userGroupAccess(permission) + + userGroupAccess: (permission) -> + user = App.User.current() group_ids = user.allGroupIds(permission) + return false if !@group_id + for local_group_id in group_ids if local_group_id.toString() is @group_id.toString() return true + + return false + + userIsCustomer: -> + user = App.User.current() + return true if user.id is @customer_id false + userIsOwner: -> + user = App.User.current() + return true if user.id is @owner_id + false + + currentView: -> + return 'agent' if App.User.current()?.permission('ticket.agent') && @userGroupAccess('read') + return 'customer' if App.User.current()?.permission('ticket.customer') + return diff --git a/app/models/channel/filter/sender_is_system_address.rb b/app/models/channel/filter/sender_is_system_address.rb index 44324abd3..495727c36 100644 --- a/app/models/channel/filter/sender_is_system_address.rb +++ b/app/models/channel/filter/sender_is_system_address.rb @@ -38,7 +38,19 @@ module Channel::Filter::SenderIsSystemAddress return if !user.permissions?('ticket.agent') mail['x-zammad-ticket-create-article-sender'.to_sym] = 'Agent' - mail['x-zammad-article-sender'.to_sym] = 'Agent' + mail['x-zammad-article-sender'.to_sym] = 'Agent' + + # if the agent is also customer of the ticket then + # we need to set the sender as customer. + ticket_id = mail['x-zammad-ticket-id'.to_sym] + if ticket_id.present? + ticket = Ticket.lookup(id: ticket_id) + + if ticket.present? && ticket.customer_id == user.id + mail['x-zammad-ticket-create-article-sender'.to_sym] = 'Customer' + mail['x-zammad-article-sender'.to_sym] = 'Customer' + end + end return true rescue => e Rails.logger.error 'SenderIsSystemAddress: ' + e.inspect diff --git a/app/models/object_manager/attribute.rb b/app/models/object_manager/attribute.rb index d24447235..315af7aa4 100644 --- a/app/models/object_manager/attribute.rb +++ b/app/models/object_manager/attribute.rb @@ -455,116 +455,6 @@ get the attribute model based on object and name =begin -get user based list of used object attributes - - attribute_list = ObjectManager::Attribute.by_object('Ticket', user) - -returns: - - [ - { name: 'api_key', display: 'API KEY', tag: 'input', null: true, edit: true, maxlength: 32 }, - { name: 'api_ip_regexp', display: 'API IP RegExp', tag: 'input', null: true, edit: true }, - { name: 'api_ip_max', display: 'API IP Max', tag: 'input', null: true, edit: true }, - ] - -=end - - def self.by_object(object, user) - - # lookups - if object - object_lookup_id = ObjectLookup.by_name(object) - end - - # get attributes in right order - result = ObjectManager::Attribute.where( - object_lookup_id: object_lookup_id, - active: true, - to_create: false, - to_delete: false, - ).order('position ASC, name ASC') - attributes = [] - result.each do |item| - data = { - name: item.name, - display: item.display, - tag: item.data_type, - #:null => item.null, - } - if item.data_option[:permission]&.any? - next if !user - - hint = false - item.data_option[:permission].each do |permission| - next if !user.permissions?(permission) - - hint = true - break - end - next if !hint - end - - if item.screens - data[:screen] = {} - item.screens.each do |screen, permission_options| - data[:screen][screen] = {} - - if permission_options['-all-'] - data[:screen][screen] = permission_options['-all-'] - next - end - - permission_options.each do |permission, options| - next if !user&.permissions?(permission) - - options.each do |key, value| - if [true, false].include?(data[:screen][screen][key]) - data[:screen][screen][key] = data[:screen][screen][key].nil? ? false : data[:screen][screen][key] - if options[key] - data[:screen][screen][key] = true - end - else - data[:screen][screen][key] = value - end - end - end - end - end - if item.data_option - data = data.merge(item.data_option.symbolize_keys) - end - attributes.push data - end - attributes - end - -=begin - -get user based list of object attributes as hash - - attribute_list = ObjectManager::Attribute.by_object_as_hash('Ticket', user) - -returns: - - { - 'api_key' => { name: 'api_key', display: 'API KEY', tag: 'input', null: true, edit: true, maxlength: 32 }, - 'api_ip_regexp' => { name: 'api_ip_regexp', display: 'API IP RegExp', tag: 'input', null: true, edit: true }, - 'api_ip_max' => { name: 'api_ip_max', display: 'API IP Max', tag: 'input', null: true, edit: true }, - } - -=end - - def self.by_object_as_hash(object, user) - list = by_object(object, user) - hash = {} - list.each do |item| - hash[ item[:name] ] = item - end - hash - end - -=begin - discard migration changes ObjectManager::Attribute.discard_changes diff --git a/app/models/object_manager/element.rb b/app/models/object_manager/element.rb new file mode 100644 index 000000000..fa8db246f --- /dev/null +++ b/app/models/object_manager/element.rb @@ -0,0 +1,7 @@ +class ObjectManager::Element + include ::Mixin::HasBackends + + def self.for_object(object) + "#{name}::#{object}".safe_constantize || ObjectManager::Element::Backend + end +end diff --git a/app/models/object_manager/element/backend.rb b/app/models/object_manager/element/backend.rb new file mode 100644 index 000000000..d84cbbdf1 --- /dev/null +++ b/app/models/object_manager/element/backend.rb @@ -0,0 +1,68 @@ +class ObjectManager::Element::Backend + + attr_reader :user, :attribute, :record + + def initialize(user:, attribute:, record:) + @user = user + @attribute = attribute + @record = record + end + + def visible? + return true if attribute.data_option[:permission].blank? + return false if user.blank? + + attribute.data_option[:permission].any? do |permission| + authorized?(permission) + end + end + + def authorized?(permission) + user.permissions?(permission) + end + + def data + data = default_data + + data[:screen] = screens if attribute.screens.present? + + return data if attribute.data_option.blank? + + data.merge(attribute.data_option.symbolize_keys) + end + + def default_data + { + name: attribute.name, + display: attribute.display, + tag: attribute.data_type, + #:null => attribute.null, + } + end + + def screens + attribute.screens.transform_values do |permission_options| + screen_value(permission_options) + end + end + + def screen_value(permission_options) + return permission_options['-all-'] if permission_options['-all-'] + return {} if user.blank? + + screen_permission_options(permission_options) + end + + def screen_permission_options(permission_options) + permission_options.each_with_object({}) do |(permission, options), result| + + next if !authorized?(permission) + + options.each do |key, value| + next if [true, false].include?(result[key]) && !value + + result[key] = value + end + end + end +end diff --git a/app/models/object_manager/element/ticket.rb b/app/models/object_manager/element/ticket.rb new file mode 100644 index 000000000..08bd2dbf0 --- /dev/null +++ b/app/models/object_manager/element/ticket.rb @@ -0,0 +1,37 @@ +class ObjectManager::Element::Ticket < ObjectManager::Element::Backend + + private + + def authorized?(permission) + return false if skip?(permission) + + super + end + + def skip?(permission) + return true if agent_in_general_view?(permission) + return true if agent_access_missing?(permission) + + authorized_customer_and_agent?(permission) + end + + def agent_in_general_view?(permission) + record.blank? && permission == 'ticket.customer' && agent? + end + + def agent_access_missing?(permission) + record.present? && permission == 'ticket.agent' && agent? && !read_access? + end + + def authorized_customer_and_agent?(permission) + record.present? && permission == 'ticket.customer' && agent? && read_access? + end + + def agent? + user.permissions?('ticket.agent') + end + + def read_access? + user.group_access?(record.group_id, 'read') + end +end diff --git a/app/models/object_manager/object.rb b/app/models/object_manager/object.rb new file mode 100644 index 000000000..e51299b08 --- /dev/null +++ b/app/models/object_manager/object.rb @@ -0,0 +1,62 @@ +class ObjectManager::Object + attr_reader :object_name + + def initialize(object_name) + @object_name = object_name + end + +=begin + +get user based list of used object attributes + + object = ObjectManager::Object.new('Ticket') + attribute_list = object.attributes(user) + +returns: + + [ + { name: 'api_key', display: 'API KEY', tag: 'input', null: true, edit: true, maxlength: 32 }, + { name: 'api_ip_regexp', display: 'API IP RegExp', tag: 'input', null: true, edit: true }, + { name: 'api_ip_max', display: 'API IP Max', tag: 'input', null: true, edit: true }, + ] + +=end + + def attributes(user, record = nil) + @attributes ||= begin + attribute_records.each_with_object([]) do |attribute_record, result| + + element = element_class.new( + user: user, + attribute: attribute_record, + record: record, + ) + + next if !element.visible? + + result.push element.data + end + end + end + + private + + def attribute_records + @attribute_records ||= begin + ObjectManager::Attribute.where( + object_lookup_id: object, + active: true, + to_create: false, + to_delete: false, + ).order('position ASC, name ASC') + end + end + + def object + @object ||= ObjectLookup.by_name(object_name) + end + + def element_class + @element_class ||= ObjectManager::Element.for_object(object_name) + end +end diff --git a/app/models/ticket.rb b/app/models/ticket.rb index 5f5ab0605..3a16f9f19 100644 --- a/app/models/ticket.rb +++ b/app/models/ticket.rb @@ -90,13 +90,28 @@ returns =end def self.access_condition(user, access) + sql = [] + bind = [] + if user.permissions?('ticket.agent') - ['group_id IN (?)', user.group_ids_access(access)] - elsif !user.organization || ( !user.organization.shared || user.organization.shared == false ) - ['tickets.customer_id = ?', user.id] - else - ['(tickets.customer_id = ? OR tickets.organization_id = ?)', user.id, user.organization.id] + sql.push('group_id IN (?)') + bind.push(user.group_ids_access(access)) end + + if user.permissions?('ticket.customer') + if !user.organization || ( !user.organization.shared || user.organization.shared == false ) + sql.push('tickets.customer_id = ?') + bind.push(user.id) + else + sql.push('(tickets.customer_id = ? OR tickets.organization_id = ?)') + bind.push(user.id) + bind.push(user.organization.id) + end + end + + return if sql.blank? + + [ sql.join(' OR ') ].concat(bind) end =begin diff --git a/app/models/ticket/overviews.rb b/app/models/ticket/overviews.rb index 7ebc64666..5e87aa1a3 100644 --- a/app/models/ticket/overviews.rb +++ b/app/models/ticket/overviews.rb @@ -18,34 +18,26 @@ returns =end def self.all(data) - current_user = data[:current_user] - links = data[:links] + current_user = data[:current_user] + overview_filter = {} + overview_filter_not = { out_of_office: true } + + return [] if !current_user.permissions?('ticket.customer') && !current_user.permissions?('ticket.agent') - # get customer overviews role_ids = User.joins(:roles).where(users: { id: current_user.id, active: true }, roles: { active: true }).pluck('roles.id') - if current_user.permissions?('ticket.customer') - overview_filter = { active: true, organization_shared: false } - if current_user.organization_id && current_user.organization.shared - overview_filter.delete(:organization_shared) - end - if links.present? - overview_filter[:link] = links - end - overviews = Overview.joins(:roles).left_joins(:users).where(overviews_roles: { role_id: role_ids }, overviews_users: { user_id: nil }, overviews: overview_filter).or(Overview.joins(:roles).left_joins(:users).where(overviews_roles: { role_id: role_ids }, overviews_users: { user_id: current_user.id }, overviews: overview_filter)).distinct('overview.id').order(:prio, :name) - return overviews + + if data[:links].present? + overview_filter[:link] = data[:links] end - # get agent overviews - return [] if !current_user.permissions?('ticket.agent') - - overview_filter = { active: true } - overview_filter_not = { out_of_office: true } + overview_filter[:active] = true if User.where('out_of_office = ? AND out_of_office_start_at <= ? AND out_of_office_end_at >= ? AND out_of_office_replacement_id = ? AND active = ?', true, Time.zone.today, Time.zone.today, current_user.id, true).count.positive? overview_filter_not = {} end - if links.present? - overview_filter[:link] = links + if !current_user.organization_id || !current_user.organization.shared + overview_filter[:organization_shared] = false end + Overview.joins(:roles).left_joins(:users).where(overviews_roles: { role_id: role_ids }, overviews_users: { user_id: nil }, overviews: overview_filter).or(Overview.joins(:roles).left_joins(:users).where(overviews_roles: { role_id: role_ids }, overviews_users: { user_id: current_user.id }, overviews: overview_filter)).where.not(overview_filter_not).distinct('overview.id').order(:prio, :name) end diff --git a/app/models/ticket/screen_options.rb b/app/models/ticket/screen_options.rb index 5d901811a..df2c0d07e 100644 --- a/app/models/ticket/screen_options.rb +++ b/app/models/ticket/screen_options.rb @@ -131,26 +131,18 @@ returns end end -=begin - # for performance reasons we moved from api calls to optimized sql queries - groups.each do |group| - filter[:group_id].push group.id - assets = group.assets(assets) - dependencies[:group_id][group.id] = { owner_id: [] } - User.group_access(group.id, 'full').each do |user| - dependencies[:group_id][ group.id ][:owner_id].push user.id - next if agents[user.id] - agents[user.id] = true - assets = user.assets(assets) - end + configure_attributes = nil + if params[:ticket].present? + configure_attributes = ObjectManager::Object.new('Ticket').attributes(params[:current_user], params[:ticket]) end -=end + { assets: assets, form_meta: { - filter: filter, - dependencies: dependencies, + filter: filter, + dependencies: dependencies, + configure_attributes: configure_attributes, } } end diff --git a/app/models/ticket/search.rb b/app/models/ticket/search.rb index e472f7734..8ab811ff2 100644 --- a/app/models/ticket/search.rb +++ b/app/models/ticket/search.rb @@ -127,16 +127,18 @@ returns # try search index backend if condition.blank? && SearchIndexBackend.enabled? - query_extension = {} - query_extension['bool'] = {} - query_extension['bool']['must'] = [] + query_or = [] if current_user.permissions?('ticket.agent') group_ids = current_user.group_ids_access('read') - access_condition = { - 'query_string' => { 'default_field' => 'group_id', 'query' => "\"#{group_ids.join('" OR "')}\"" } - } - else + if group_ids.present? + access_condition = { + 'query_string' => { 'default_field' => 'group_id', 'query' => "\"#{group_ids.join('" OR "')}\"" } + } + query_or.push(access_condition) + end + end + if current_user.permissions?('ticket.customer') access_condition = if !current_user.organization || ( !current_user.organization.shared || current_user.organization.shared == false ) { 'query_string' => { 'default_field' => 'customer_id', 'query' => current_user.id } @@ -150,9 +152,22 @@ returns # customer_id: XXX OR organization_id: XXX # conditions = [ '( customer_id = ? OR organization_id = ? )', current_user.id, current_user.organization.id ] end + query_or.push(access_condition) end - query_extension['bool']['must'].push access_condition + return [] if query_or.blank? + + query_extension = { + 'bool': { + 'must': [ + { + 'bool': { + 'should': query_or, + }, + }, + ], + } + } items = SearchIndexBackend.search(query, 'Ticket', limit: limit, query_extension: query_extension, diff --git a/app/models/transaction/notification.rb b/app/models/transaction/notification.rb index daaaf00e4..022abab92 100644 --- a/app/models/transaction/notification.rb +++ b/app/models/transaction/notification.rb @@ -239,7 +239,7 @@ class Transaction::Notification locale = user.locale # only show allowed attributes - attribute_list = ObjectManager::Attribute.by_object_as_hash('Ticket', user) + attribute_list = ObjectManager::Object.new('Ticket').attributes(user).index_by { |item| item[:name] } user_related_changes = {} @item[:changes].each do |key, value| diff --git a/app/models/transaction/slack.rb b/app/models/transaction/slack.rb index f50f8b972..7ede4fc48 100644 --- a/app/models/transaction/slack.rb +++ b/app/models/transaction/slack.rb @@ -208,7 +208,7 @@ class Transaction::Slack locale = user.preferences[:locale] || Setting.get('locale_default') || 'en-us' # only show allowed attributes - attribute_list = ObjectManager::Attribute.by_object_as_hash('Ticket', user) + attribute_list = ObjectManager::Object.new('Ticket').attributes(user).index_by { |item| item[:name] } #puts "AL #{attribute_list.inspect}" user_related_changes = {} @item[:changes].each do |key, value| diff --git a/app/policies/organization_policy.rb b/app/policies/organization_policy.rb index 8b52fa13a..48d0c9b78 100644 --- a/app/policies/organization_policy.rb +++ b/app/policies/organization_policy.rb @@ -2,15 +2,15 @@ class OrganizationPolicy < ApplicationPolicy def show? return true if user.permissions?(['admin', 'ticket.agent']) - return false if !user.permissions?('ticket.customer') + return true if record.id == user.organization_id - record.id == user.organization_id + false end def update? - return false if user.permissions?('ticket.customer') + return true if user.permissions?(['admin', 'ticket.agent']) - user.permissions?(['admin', 'ticket.agent']) + false end class Scope < ApplicationPolicy::Scope diff --git a/app/policies/ticket/article_policy.rb b/app/policies/ticket/article_policy.rb index 69edf00c9..79bb19314 100644 --- a/app/policies/ticket/article_policy.rb +++ b/app/policies/ticket/article_policy.rb @@ -55,9 +55,7 @@ class Ticket::ArticlePolicy < ApplicationPolicy end def access?(query) - if record.internal == true && user.permissions?('ticket.customer') - return false - end + return false if record.internal == true && !user.permissions?('ticket.agent') ticket = Ticket.lookup(id: record.ticket_id) Pundit.authorize(user, ticket, query) diff --git a/app/policies/ticket_policy.rb b/app/policies/ticket_policy.rb index ab78f8718..74e0c363f 100644 --- a/app/policies/ticket_policy.rb +++ b/app/policies/ticket_policy.rb @@ -38,26 +38,26 @@ class TicketPolicy < ApplicationPolicy def access?(access) - # check customer - if user.permissions?('ticket.customer') - - # access ok if its own ticket - return true if record.customer_id == user.id - - # check organization ticket access - return false if record.organization_id.blank? - return false if user.organization_id.blank? - return false if record.organization_id != user.organization_id - - return record.organization.shared? - end - - # check agent - - # access if requester is owner + # agent - access if requester is owner return true if record.owner_id == user.id - # access if requester is in group - user.group_access?(record.group.id, access) + # agent - access if requester is in group + return true if user.group_access?(record.group.id, access) + + # check customer + return false if !user.permissions?('ticket.customer') + + # access ok if its own ticket + return true if record.customer_id == user.id + + organization_access? + end + + def organization_access? + return false if record.organization_id.blank? + return false if user.organization_id.blank? + return false if record.organization_id != user.organization_id + + record.organization.shared? end end diff --git a/db/migrate/20200527000000_agent_customer.rb b/db/migrate/20200527000000_agent_customer.rb new file mode 100644 index 000000000..eeb9cf430 --- /dev/null +++ b/db/migrate/20200527000000_agent_customer.rb @@ -0,0 +1,20 @@ +class AgentCustomer < ActiveRecord::Migration[5.2] + def change + # return if it's a new setup + return if !Setting.exists?(name: 'system_init_done') + + Role.where(name: %w[Admin Agent Customer]).each do |role| + role.preferences.delete(:not) + role.save! + end + + move_filter + end + + def move_filter + Setting.find_by(name: '0010_postmaster_filter_trusted').update(name: '0005_postmaster_filter_trusted') + Setting.find_by(name: '0020_postmaster_filter_auto_response_check').update(name: '0006_postmaster_filter_auto_response_check') + Setting.find_by(name: '0100_postmaster_filter_follow_up_check').update(name: '0007_postmaster_filter_follow_up_check') + Setting.find_by(name: '0110_postmaster_filter_follow_up_merged').update(name: '0008_postmaster_filter_follow_up_merged') + end +end diff --git a/db/seeds/roles.rb b/db/seeds/roles.rb index 0f6a9d60c..d25f76c5e 100644 --- a/db/seeds/roles.rb +++ b/db/seeds/roles.rb @@ -2,9 +2,7 @@ Role.create_if_not_exists( id: 1, name: 'Admin', note: 'To configure your system.', - preferences: { - not: ['Customer'], - }, + preferences: {}, default_at_signup: false, updated_by_id: 1, created_by_id: 1 @@ -14,9 +12,7 @@ Role.create_if_not_exists( name: 'Agent', note: 'To work on Tickets.', default_at_signup: false, - preferences: { - not: ['Customer'], - }, + preferences: {}, updated_by_id: 1, created_by_id: 1 ) @@ -24,9 +20,7 @@ Role.create_if_not_exists( id: 3, name: 'Customer', note: 'People who create Tickets ask for help.', - preferences: { - not: %w[Agent Admin], - }, + preferences: {}, default_at_signup: true, updated_by_id: 1, created_by_id: 1 diff --git a/db/seeds/settings.rb b/db/seeds/settings.rb index fd74743ae..9e8418c6d 100644 --- a/db/seeds/settings.rb +++ b/db/seeds/settings.rb @@ -3306,13 +3306,40 @@ Setting.create_if_not_exists( Setting.create_if_not_exists( title: 'Defines postmaster filter.', - name: '0010_postmaster_filter_trusted', + name: '0005_postmaster_filter_trusted', area: 'Postmaster::PreFilter', description: 'Defines postmaster filter to remove X-Zammad headers from not trusted sources.', options: {}, state: 'Channel::Filter::Trusted', frontend: false ) +Setting.create_if_not_exists( + title: 'Defines postmaster filter.', + name: '0006_postmaster_filter_auto_response_check', + area: 'Postmaster::PreFilter', + description: 'Defines postmaster filter to identify auto responses to prevent auto replies from Zammad.', + options: {}, + state: 'Channel::Filter::AutoResponseCheck', + frontend: false +) +Setting.create_if_not_exists( + title: 'Defines postmaster filter.', + name: '0007_postmaster_filter_follow_up_check', + area: 'Postmaster::PreFilter', + description: 'Defines postmaster filter to identify follow-ups (based on admin settings).', + options: {}, + state: 'Channel::Filter::FollowUpCheck', + frontend: false +) +Setting.create_if_not_exists( + title: 'Defines postmaster filter.', + name: '0008_postmaster_filter_follow_up_merged', + area: 'Postmaster::PreFilter', + description: 'Defines postmaster filter to identify follow-up ticket for merged tickets.', + options: {}, + state: 'Channel::Filter::FollowUpMerged', + frontend: false +) Setting.create_if_not_exists( title: 'Defines postmaster filter.', name: '0011_postmaster_sender_based_on_reply_to', @@ -3358,15 +3385,6 @@ Setting.create_if_not_exists( state: 'Channel::Filter::SecureMailing', frontend: false ) -Setting.create_if_not_exists( - title: 'Defines postmaster filter.', - name: '0020_postmaster_filter_auto_response_check', - area: 'Postmaster::PreFilter', - description: 'Defines postmaster filter to identify auto responses to prevent auto replies from Zammad.', - options: {}, - state: 'Channel::Filter::AutoResponseCheck', - frontend: false -) Setting.create_if_not_exists( title: 'Defines postmaster filter.', name: '0030_postmaster_filter_out_of_office_check', @@ -3376,24 +3394,6 @@ Setting.create_if_not_exists( state: 'Channel::Filter::OutOfOfficeCheck', frontend: false ) -Setting.create_if_not_exists( - title: 'Defines postmaster filter.', - name: '0100_postmaster_filter_follow_up_check', - area: 'Postmaster::PreFilter', - description: 'Defines postmaster filter to identify follow-ups (based on admin settings).', - options: {}, - state: 'Channel::Filter::FollowUpCheck', - frontend: false -) -Setting.create_if_not_exists( - title: 'Defines postmaster filter.', - name: '0110_postmaster_filter_follow_up_merged', - area: 'Postmaster::PreFilter', - description: 'Defines postmaster filter to identify follow-up ticket for merged tickets.', - options: {}, - state: 'Channel::Filter::FollowUpMerged', - frontend: false -) Setting.create_if_not_exists( title: 'Defines postmaster filter.', name: '0200_postmaster_filter_follow_up_possible_check', diff --git a/lib/session_helper.rb b/lib/session_helper.rb index 0bfea263d..b83284d38 100644 --- a/lib/session_helper.rb +++ b/lib/session_helper.rb @@ -31,7 +31,7 @@ module SessionHelper models = {} objects = ObjectManager.list_objects objects.each do |object| - attributes = ObjectManager::Attribute.by_object(object, user) + attributes = ObjectManager::Object.new(object).attributes(user) models[object] = attributes end models diff --git a/public/assets/tests/model_ticket.js b/public/assets/tests/model_ticket.js index e6fbb5714..91b9101ab 100644 --- a/public/assets/tests/model_ticket.js +++ b/public/assets/tests/model_ticket.js @@ -65,6 +65,48 @@ window.onload = function() { active: true, }]) + App.Role.refresh([ + { + "name":"Agent", + "permission_ids":[ + 48, + ], + "group_ids":{}, + "default_at_signup":false, + "note":"To work on Tickets.", + "active":true, + "updated_at":"2020-07-29T14:57:27.304Z", + "id":2 + }, + { + "name":"Customer", + "permission_ids":[ + 49 + ], + "group_ids":{}, + "default_at_signup":true, + "note":"People who create Tickets ask for help.", + "active":true, + "updated_at":"2020-07-29T14:57:27.314Z", + "id":3 + } + ]) + + App.Permission.refresh([ + { + "name":"ticket.agent", + "note":"Access to Agent Tickets based on Group Access", + "active":true, + "id":48 + }, + { + "name":"ticket.customer", + "note":"Access to Customer Tickets based on current_user and organization", + "active":true, + "id":49 + }, + ]) + test('ticket.editabe customer user #1', function() { App.Session.set(33) ticket1 = App.Ticket.find(1); diff --git a/spec/factories/overview.rb b/spec/factories/overview.rb index 23a4567e0..9d421391c 100644 --- a/spec/factories/overview.rb +++ b/spec/factories/overview.rb @@ -3,7 +3,7 @@ FactoryBot.define do sequence(:name) { |n| "Test Overview #{n}" } prio { 1 } role_ids { Role.where(name: %w[Customer Agent Admin]).pluck(:id) } - out_of_office { true } + out_of_office { false } updated_by_id { 1 } created_by_id { 1 } diff --git a/spec/factories/user.rb b/spec/factories/user.rb index d63b33937..61af1d31d 100644 --- a/spec/factories/user.rb +++ b/spec/factories/user.rb @@ -29,6 +29,14 @@ FactoryBot.define do end end + factory :agent_and_customer do + role_ids { Role.signup_role_ids.push( Role.find_by(name: 'Agent').id ).sort } + + trait :with_org do + organization + end + end + factory :agent do roles { Role.where(name: 'Agent') } end diff --git a/spec/models/channel/email_parser_spec.rb b/spec/models/channel/email_parser_spec.rb index 57da44d47..c2744b6fa 100644 --- a/spec/models/channel/email_parser_spec.rb +++ b/spec/models/channel/email_parser_spec.rb @@ -140,6 +140,24 @@ RSpec.describe Channel::EmailParser, type: :model do end end + context 'when from address matches an existing agent customer' do + let!(:agent_customer) { create(:agent_and_customer, email: 'foo@bar.com') } + let!(:ticket) { create(:ticket, customer: agent_customer) } + let!(:raw_email) { <<~RAW.chomp } + From: foo@bar.com + To: myzammad@example.com + Subject: [#{Setting.get('ticket_hook') + Setting.get('ticket_hook_divider') + ticket.number}] test + + Lorem ipsum dolor + RAW + + it 'sets article.sender to "Customer"' do + described_class.new.process({}, raw_email) + + expect(Ticket::Article.last.sender.name).to eq('Customer') + end + end + context 'when from address matches an existing customer' do let!(:customer) { create(:customer, email: 'foo@bar.com') } diff --git a/spec/models/object_manager/attribute_spec.rb b/spec/models/object_manager/attribute_spec.rb index edbdec6f4..a5ce46fdc 100644 --- a/spec/models/object_manager/attribute_spec.rb +++ b/spec/models/object_manager/attribute_spec.rb @@ -2,17 +2,6 @@ require 'rails_helper' RSpec.describe ObjectManager::Attribute, type: :model do - let(:user_attribute_permissions) do - create(:user, roles: [role_attribute_permissions]) - end - - let(:role_attribute_permissions) do - create(:role).tap do |role| - role.permission_grant('admin.organization') - role.permission_grant('ticket.agent') - end - end - describe 'callbacks' do context 'for setting default values on local data options' do let(:subject) { described_class.new } @@ -118,36 +107,4 @@ RSpec.describe ObjectManager::Attribute, type: :model do end.not_to raise_error end end - - describe 'attribute permissions', db_strategy: :reset do - it 'merges attribute permissions' do - create(:object_manager_attribute_text, screens: { create: { 'admin.organization': { shown: true }, 'ticket.agent': { shown: false } } }, name: 'test_permissions') - - migration = described_class.migration_execute - expect(migration).to be true - - attribute = described_class.by_object('Ticket', user_attribute_permissions).detect { |attr| attr[:name] == 'test_permissions' } - expect(attribute[:screen]['create']['shown']).to be true - end - - it 'overwrites permissions if all get set' do - create(:object_manager_attribute_text, screens: { create: { '-all-': { shown: true }, 'admin.organization': { shown: false }, 'ticket.agent': { shown: false } } }, name: 'test_permissions_all') - - migration = described_class.migration_execute - expect(migration).to be true - - attribute = described_class.by_object('Ticket', user_attribute_permissions).detect { |attr| attr[:name] == 'test_permissions_all' } - expect(attribute[:screen]['create']['shown']).to be true - end - - it 'is able to handle other values than true or false' do - create(:object_manager_attribute_text, screens: { create: { '-all-': { shown: true, item_class: 'column' }, 'admin.organization': { shown: false }, 'ticket.agent': { shown: false } } }, name: 'test_permissions_item') - - migration = described_class.migration_execute - expect(migration).to be true - - attribute = described_class.by_object('Ticket', user_attribute_permissions).detect { |attr| attr[:name] == 'test_permissions_item' } - expect(attribute[:screen]['create']['item_class']).to eq('column') - end - end end diff --git a/spec/models/object_manager/object_spec.rb b/spec/models/object_manager/object_spec.rb new file mode 100644 index 000000000..2946d084b --- /dev/null +++ b/spec/models/object_manager/object_spec.rb @@ -0,0 +1,110 @@ +require 'rails_helper' + +RSpec.describe ObjectManager::Object do + + describe 'attribute permissions', db_strategy: :reset do + + let(:user) do + create(:user, roles: [role_attribute_permissions]) + end + let(:attribute) { described_class.new('Ticket').attributes(user).detect { |attribute| attribute[:name] == attribute_name } } + + let(:role_attribute_permissions) do + create(:role).tap do |role| + role.permission_grant('admin.organization') + role.permission_grant('ticket.agent') + end + end + + let(:attribute_name) { 'example_attribute' } + + before do + create(:object_manager_attribute_text, name: attribute_name, screens: screens) + ObjectManager::Attribute.migration_execute + end + + context 'when true and false values for show exist' do + let(:screens) do + { + create: { + 'admin.organization': { + shown: true + }, + 'ticket.agent': { + shown: false + } + } + } + end + + it 'uses true' do + expect(attribute[:screen]['create']['shown']).to be true + end + end + + context 'when -all- is present' do + let(:screens) do + { + create: { + '-all-': { + shown: true + }, + 'admin.organization': { + shown: false + }, + 'ticket.agent': { + shown: false + } + } + } + end + + it 'takes its values into account' do + expect(attribute[:screen]['create']['shown']).to be true + end + end + + context 'when non boolean values are present' do + let(:screens) do + { + create: { + '-all-': { + shown: true, + item_class: 'column' + }, + 'admin.organization': { + shown: false + }, + 'ticket.agent': { + shown: false + } + } + } + end + + it 'takes these values into account' do + expect(attribute[:screen]['create']['item_class']).to eq('column') + end + end + + context 'when agent is also customer' do + let(:user) { create(:agent_and_customer) } + let(:screens) do + { + create: { + 'ticket.customer': { + filter: [2, 4] + }, + 'ticket.agent': { + filter: [3, 5] + } + } + } + end + + it 'prefers agent over customer permissions' do + expect(attribute[:screen]['create']['filter']).to eq([3, 5]) + end + end + end +end diff --git a/spec/models/ticket/overviews_spec.rb b/spec/models/ticket/overviews_spec.rb index a401aac00..d3bb73612 100644 --- a/spec/models/ticket/overviews_spec.rb +++ b/spec/models/ticket/overviews_spec.rb @@ -2,7 +2,48 @@ require 'rails_helper' RSpec.describe Ticket::Overviews do - describe '#index' do + describe '.all' do + + let(:views) { described_class.all(current_user: current_user).map(&:name) } + + shared_examples 'containing' do |overview| + it "returns #{overview}" do + expect(views).to include(overview) + end + end + + shared_examples 'not containing' do |overview| + it "doesn't return #{overview}" do + expect(views).not_to include(overview) + end + end + + context 'when Agent' do + let(:current_user) { create(:agent) } + + it_behaves_like 'containing', 'Open' + it_behaves_like 'not containing', 'My Tickets' + it_behaves_like 'not containing', 'My Organization Tickets' + end + + context 'when Agent is also Customer' do + let(:current_user) { create(:agent_and_customer, :with_org) } + + it_behaves_like 'containing', 'Open' + it_behaves_like 'containing', 'My Tickets' + it_behaves_like 'containing', 'My Organization Tickets' + end + + context 'when Customer' do + let(:current_user) { create(:customer, :with_org) } + + it_behaves_like 'not containing', 'Open' + it_behaves_like 'containing', 'My Tickets' + it_behaves_like 'containing', 'My Organization Tickets' + end + end + + describe '.index' do # https://github.com/zammad/zammad/issues/1769 it 'does not return multiple results for a single ticket' do diff --git a/spec/models/ticket_spec.rb b/spec/models/ticket_spec.rb index d300f65f3..6dfc5c0da 100644 --- a/spec/models/ticket_spec.rb +++ b/spec/models/ticket_spec.rb @@ -861,6 +861,123 @@ RSpec.describe Ticket, type: :model do end end + describe '.search' do + + shared_examples 'search permissions' do + let(:group) { create(:group) } + + before do + ticket + end + + shared_examples 'permitted' do + it 'finds Ticket' do + expect( described_class.search(query: ticket.number, current_user: current_user).count ).to eq(1) + end + end + + shared_examples 'no permission' do + it "doesn't find Ticket" do + expect( described_class.search(query: ticket.number, current_user: current_user) ).to be_blank + end + end + + context 'Agent with Group access' do + + let(:ticket) do + ticket = create(:ticket, group: group) + create(:ticket_article, ticket: ticket) + ticket + end + + let(:current_user) { create(:agent, groups: [group]) } + + it_behaves_like 'permitted' + end + + context 'when Agent is Customer of Ticket' do + + let(:ticket) do + ticket = create(:ticket, customer: current_user) + create(:ticket_article, ticket: ticket) + ticket + end + + let(:current_user) { create(:agent_and_customer) } + + it_behaves_like 'permitted' + end + + context 'for Organization access' do + + let(:ticket) do + ticket = create(:ticket, customer: customer) + create(:ticket_article, ticket: ticket) + ticket + end + + let(:customer) { create(:customer, organization: organization) } + + context 'when Organization is shared' do + let(:organization) { create(:organization, shared: true) } + + context 'for unrelated Agent' do + let(:current_user) { create(:agent) } + + it_behaves_like 'no permission' + end + + context 'for Agent in same Organization' do + let(:current_user) { create(:agent_and_customer, organization: organization) } + + it_behaves_like 'permitted' + end + + context 'for Customer of Ticket' do + let(:current_user) { customer } + + it_behaves_like 'permitted' + end + end + + context 'when Organization is not shared' do + let(:organization) { create(:organization, shared: false) } + + context 'for unrelated Agent' do + let(:current_user) { create(:agent) } + + it_behaves_like 'no permission' + end + + context 'for Agent in same Organization' do + let(:current_user) { create(:agent_and_customer, organization: organization) } + + it_behaves_like 'no permission' + end + + context 'for Customer of Ticket' do + let(:current_user) { customer } + + it_behaves_like 'permitted' + end + end + end + end + + context 'with searchindex', searchindex: true do + + include_examples 'search permissions' do + before do + configure_elasticsearch(required: true, rebuild: true) + end + end + end + + context 'without searchindex' do + include_examples 'search permissions' + end + end + describe 'Callbacks & Observers -' do describe 'NULL byte handling (via ChecksAttributeValuesAndLength concern):' do it 'removes them from title on creation, if necessary (postgres doesn’t like them)' do diff --git a/spec/policies/organization_policy_spec.rb b/spec/policies/organization_policy_spec.rb new file mode 100644 index 000000000..c1cddfee7 --- /dev/null +++ b/spec/policies/organization_policy_spec.rb @@ -0,0 +1,38 @@ +require 'rails_helper' + +describe OrganizationPolicy do + subject { described_class.new(user, record) } + + let(:record) { create(:organization) } + + context 'when customer' do + let(:user) { create(:customer, organization: record) } + + it { is_expected.to permit_actions(%i[show]) } + it { is_expected.not_to permit_actions(%i[update]) } + end + + context 'when customer without organization' do + let(:user) { create(:customer) } + + it { is_expected.not_to permit_actions(%i[show update]) } + end + + context 'when agent and customer' do + let(:user) { create(:agent_and_customer, organization: record) } + + it { is_expected.to permit_actions(%i[show update]) } + end + + context 'when agent' do + let(:user) { create(:agent) } + + it { is_expected.to permit_actions(%i[show update]) } + end + + context 'when admin' do + let(:user) { create(:admin) } + + it { is_expected.to permit_actions(%i[show update]) } + end +end diff --git a/spec/policies/ticket/article_policy_spec.rb b/spec/policies/ticket/article_policy_spec.rb new file mode 100644 index 000000000..e469dec36 --- /dev/null +++ b/spec/policies/ticket/article_policy_spec.rb @@ -0,0 +1,57 @@ +require 'rails_helper' + +describe Ticket::ArticlePolicy do + subject { described_class.new(user, record) } + + let!(:group) { create(:group) } + let!(:ticket_customer) { create(:customer) } + + let(:record) do + ticket = create(:ticket, group: group, customer: ticket_customer) + create(:ticket_article, ticket: ticket) + end + + context 'when article internal' do + let(:record) do + ticket = create(:ticket, group: group, customer: ticket_customer) + create(:ticket_article, ticket: ticket, internal: true) + end + + context 'when agent' do + let(:user) { create(:agent, groups: [group]) } + + it { is_expected.to permit_actions(%i[show]) } + end + + context 'when agent and customer' do + let(:user) { create(:agent_and_customer, groups: [group]) } + + it { is_expected.to permit_actions(%i[show]) } + end + + context 'when customer' do + let(:user) { ticket_customer } + + it { is_expected.not_to permit_actions(%i[show]) } + end + end + + context 'when agent' do + let(:user) { create(:agent, groups: [group]) } + + it { is_expected.to permit_actions(%i[show]) } + end + + context 'when agent and customer' do + let(:user) { create(:agent_and_customer, groups: [group]) } + + it { is_expected.to permit_actions(%i[show]) } + end + + context 'when customer' do + let(:user) { ticket_customer } + + it { is_expected.to permit_actions(%i[show]) } + end + +end diff --git a/spec/policies/ticket_policy_spec.rb b/spec/policies/ticket_policy_spec.rb index c96d0e0c6..d4e518279 100644 --- a/spec/policies/ticket_policy_spec.rb +++ b/spec/policies/ticket_policy_spec.rb @@ -11,6 +11,12 @@ describe TicketPolicy do it { is_expected.to permit_actions(%i[show full]) } end + context 'when given user that is agent and customer' do + let(:user) { create(:agent_and_customer, groups: [record.group]) } + + it { is_expected.to permit_actions(%i[show full]) } + end + context 'when given a user that is neither owner nor customer' do let(:user) { create(:agent) } diff --git a/spec/system/ticket/zoom_spec.rb b/spec/system/ticket/zoom_spec.rb index 81861b735..7a0ecd884 100644 --- a/spec/system/ticket/zoom_spec.rb +++ b/spec/system/ticket/zoom_spec.rb @@ -877,4 +877,151 @@ RSpec.describe 'Ticket zoom', type: :system do expect(images_identical?(images.first, images.second)).to be(true) end end + + context 'object manager attribute permission view' do + let!(:group_users) { Group.find_by(name: 'Users') } + + shared_examples 'shows attributes and values for agent view and editable' do + it 'shows attributes and values for agent view and editable', authenticated_as: :current_user do + visit "ticket/zoom/#{ticket.id}" + refresh # refresh to have assets generated for ticket + + expect(page).to have_select('state_id', options: ['new', 'open', 'pending reminder', 'pending close', 'closed']) + expect(page).to have_select('priority_id') + expect(page).to have_select('owner_id') + expect(page).to have_css('div.tabsSidebar-tab[data-tab=customer]') + end + end + + shared_examples 'shows attributes and values for agent view but disabled' do + it 'shows attributes and values for agent view but disabled', authenticated_as: :current_user do + visit "ticket/zoom/#{ticket.id}" + refresh # refresh to have assets generated for ticket + + expect(page).to have_css('select[name=state_id][disabled]') + expect(page).to have_css('select[name=priority_id][disabled]') + expect(page).to have_css('select[name=owner_id][disabled]') + expect(page).to have_css('div.tabsSidebar-tab[data-tab=customer]') + end + end + + shared_examples 'shows attributes and values for customer view' do + it 'shows attributes and values for customer view', authenticated_as: :current_user do + visit "ticket/zoom/#{ticket.id}" + refresh # refresh to have assets generated for ticket + + expect(page).to have_select('state_id', options: %w[new open closed]) + expect(page).not_to have_select('priority_id') + expect(page).not_to have_select('owner_id') + expect(page).not_to have_css('div.tabsSidebar-tab[data-tab=customer]') + end + end + + context 'as customer' do + let!(:current_user) { create(:customer) } + let(:ticket) { create(:ticket, customer: current_user) } + + include_examples 'shows attributes and values for customer view' + end + + context 'as agent with full permissions' do + let(:current_user) { create(:agent, groups: [ group_users ] ) } + let(:ticket) { create(:ticket, group: group_users ) } + + include_examples 'shows attributes and values for agent view and editable' + end + + context 'as agent with change permissions' do + let!(:current_user) { create(:agent) } + let(:ticket) { create(:ticket, group: group_users) } + + before do + current_user.group_names_access_map = { + group_users.name => %w[read change], + } + end + + include_examples 'shows attributes and values for agent view and editable' + end + + context 'as agent with read permissions' do + let!(:current_user) { create(:agent) } + let(:ticket) { create(:ticket, group: group_users) } + + before do + current_user.group_names_access_map = { + group_users.name => 'read', + } + end + + include_examples 'shows attributes and values for agent view but disabled' + end + + context 'as agent+customer with full permissions' do + let!(:current_user) { create(:agent_and_customer, groups: [ group_users ] ) } + + context 'normal ticket' do + let(:ticket) { create(:ticket, group: group_users ) } + + include_examples 'shows attributes and values for agent view and editable' + end + + context 'ticket where current_user is also customer' do + let(:ticket) { create(:ticket, customer: current_user, group: group_users ) } + + include_examples 'shows attributes and values for agent view and editable' + end + end + + context 'as agent+customer with change permissions' do + let!(:current_user) { create(:agent_and_customer) } + + before do + current_user.group_names_access_map = { + group_users.name => %w[read change], + } + end + + context 'normal ticket' do + let(:ticket) { create(:ticket, group: group_users) } + + include_examples 'shows attributes and values for agent view and editable' + end + + context 'ticket where current_user is also customer' do + let(:ticket) { create(:ticket, customer: current_user, group: group_users) } + + include_examples 'shows attributes and values for agent view and editable' + end + end + + context 'as agent+customer with read permissions' do + let!(:current_user) { create(:agent_and_customer) } + + before do + current_user.group_names_access_map = { + group_users.name => 'read', + } + end + + context 'normal ticket' do + let(:ticket) { create(:ticket, group: group_users) } + + include_examples 'shows attributes and values for agent view but disabled' + end + + context 'ticket where current_user is also customer' do + let(:ticket) { create(:ticket, customer: current_user, group: group_users) } + + include_examples 'shows attributes and values for agent view but disabled' + end + end + + context 'as agent+customer but only customer for the ticket (no agent access)' do + let!(:current_user) { create(:agent_and_customer) } + let(:ticket) { create(:ticket, customer: current_user) } + + include_examples 'shows attributes and values for customer view' + end + end end diff --git a/test/unit/ticket_overview_out_of_office_test.rb b/test/unit/ticket_overview_out_of_office_test.rb index 70feb74c2..79c8143af 100644 --- a/test/unit/ticket_overview_out_of_office_test.rb +++ b/test/unit/ticket_overview_out_of_office_test.rb @@ -123,7 +123,7 @@ class TicketOverviewOutOfOfficeTest < ActiveSupport::TestCase link: 'my_tickets', prio: 1100, role_ids: [overview_role.id], - out_of_office: true, + out_of_office: false, condition: { 'ticket.state_id' => { operator: 'is', diff --git a/test/unit/user_test.rb b/test/unit/user_test.rb index de5dcf0bd..d96c84ad5 100644 --- a/test/unit/user_test.rb +++ b/test/unit/user_test.rb @@ -651,12 +651,6 @@ class UserTest < ActiveSupport::TestCase created_by_id: 1, ) - assert_raises(RuntimeError) do - customer3.roles = Role.where(name: %w[Customer Admin]) - end - assert_raises(RuntimeError) do - customer3.roles = Role.where(name: %w[Customer Agent]) - end customer3.roles = Role.where(name: %w[Admin Agent]) customer3.roles.each do |role| assert_not_equal(role.name, 'Customer')