Updated CHANGES.

(cherry picked from commit 428c68d06a)

.gitlab-ci.yml

@@ -300,7 +300,7 @@ test:integration:otrs_6_mysql:
     - mysql
   script:
     - export RAILS_ENV=test
-    - export IMPORT_OTRS_ENDPOINT="http://vz1185.test.znuny.com/otrs/public.pl?Action=ZammadMigrator"
+    - export IMPORT_OTRS_ENDPOINT="https://vz1185.test.znuny.com/otrs/public.pl?Action=ZammadMigrator"
     - rake db:create
     - rake db:migrate
     - ruby -I test/ test/integration/otrs_import_test.rb
@@ -313,7 +313,7 @@ test:integration:otrs_6_postgresql:
     - postgresql
   script:
     - export RAILS_ENV=test
-    - export IMPORT_OTRS_ENDPOINT="http://vz1185.test.znuny.com/otrs/public.pl?Action=ZammadMigrator"
+    - export IMPORT_OTRS_ENDPOINT="https://vz1185.test.znuny.com/otrs/public.pl?Action=ZammadMigrator"
     - rake db:create
     - rake db:migrate
     - ruby -I test/ test/integration/otrs_import_test.rb
@@ -1053,3 +1053,18 @@ test:browser:integration:zendesk_chrome:
     - script/build/test_startup.sh $RAILS_ENV $BROWSER_PORT $WS_PORT 1
     - ruby -I test/ test/integration/zendesk_import_browser_test.rb || script/build/test_shutdown.sh $RAILS_ENV $BROWSER_PORT $WS_PORT 1 1
     - script/build/test_shutdown.sh $RAILS_ENV $BROWSER_PORT $WS_PORT 0 1
+
+test:browser:integration:idoit_chrome:
+  stage: browser-integration
+  dependencies:
+    - browser:build
+  tags:
+    - browser
+  script:
+    - export BROWSER=chrome
+    - export BROWSER_URL=http://$IP:$BROWSER_PORT
+    - RAILS_ENV=test rake db:create
+    - cp contrib/auto_wizard_test.json auto_wizard.json
+    - script/build/test_startup.sh $RAILS_ENV $BROWSER_PORT $WS_PORT 1
+    - ruby -I test/ test/integration/idoit_browser_test.rb || script/build/test_shutdown.sh $RAILS_ENV $BROWSER_PORT $WS_PORT 1 1
+    - script/build/test_shutdown.sh $RAILS_ENV $BROWSER_PORT $WS_PORT 0 1

CHANGELOG.md

@@ -1,16 +1,45 @@
 # Change Log
 
-## [2.5.0](https://github.com/zammad/zammad/tree/2.5.0) (2018-xx-xx)
-[Full Changelog](https://github.com/zammad/zammad/compare/2.4.0...2.5.0)
+## [2.5.1](https://github.com/zammad/zammad/tree/2.5.1) (2018-08-08)
+[Full Changelog](https://github.com/zammad/zammad/compare/2.5.0...2.5.1)
 
 **Implemented enhancements:**
-
-
-
+- Add delete existing records to CSV import feature. [\#2064](https://github.com/zammad/zammad/issues/2064) [[enhancement](https://github.com/zammad/zammad/labels/enhancement)]
 
 **Fixed bugs:**
-
-
-
+- Do not allow to create pre existing attributes like updated\_at via object manager [\#2172](https://github.com/zammad/zammad/issues/2172) [[bug](https://github.com/zammad/zammad/labels/bug)]
+- Overview not showing unassigned tickets "if not defined" [\#2171](https://github.com/zammad/zammad/issues/2171) [[bug](https://github.com/zammad/zammad/labels/bug)]
+- i-doit: Setting object before filling in the ticket information removed the i-doit objects [\#2165](https://github.com/zammad/zammad/issues/2165) [[bug](https://github.com/zammad/zammad/labels/bug)]
+- i-doit: Double click needed to select object after searching \(w/o category\) [\#2164](https://github.com/zammad/zammad/issues/2164) [[bug](https://github.com/zammad/zammad/labels/bug)]
+- 30 concurrent agents + each of them 30 overviews - script/scheduler.rb takes 100% CPU time - background jobs cannot be processed [\#2108](https://github.com/zammad/zammad/issues/2108) [[bug](https://github.com/zammad/zammad/labels/bug)]
+- Visible organizsation content \(organization profile\) not updated [\#1616](https://github.com/zammad/zammad/issues/1616) [[bug](https://github.com/zammad/zammad/labels/bug)]
+- RegexpError: failed to allocate memory for large attachments [\#2141](https://github.com/zammad/zammad/issues/2141) [[bug](https://github.com/zammad/zammad/labels/bug)]
+- Unable to link objects at ticket creation [\#2132](https://github.com/zammad/zammad/issues/2132) [[bug](https://github.com/zammad/zammad/labels/bug)]
+- Ticket "update" cancels active attachment upload [\#2117](https://github.com/zammad/zammad/issues/2117) [[bug](https://github.com/zammad/zammad/labels/bug)]
+- Deleting all tickets throws errors for old IDs [\#2066](https://github.com/zammad/zammad/issues/2066) [[bug](https://github.com/zammad/zammad/labels/bug)]
+- LDAP sync: Updating association of deactivated instance fails [\#2110](https://github.com/zammad/zammad/issues/2110) [[bug](https://github.com/zammad/zammad/labels/bug)]
+- User history shows LDAP deactivation was done by a regular agent [\#2111](https://github.com/zammad/zammad/issues/2111) [[bug](https://github.com/zammad/zammad/labels/bug)]
+- strip out auto generated links by the browser [\#2019](https://github.com/zammad/zammad/issues/2019) [[bug](https://github.com/zammad/zammad/labels/bug)]
+- Route for /auth/failure is missing to show login failure messages form oauth provider \(if request was technical ok - only login was not possible by oauth provider\) [\#2128](https://github.com/zammad/zammad/issues/2128) [[bug](https://github.com/zammad/zammad/labels/bug)]
+- Auto responder is not set from correct group [\#2125](https://github.com/zammad/zammad/issues/2125) [[bug](https://github.com/zammad/zammad/labels/bug)]
+- Shown ticket changed unexpected when viewing an attached image [\#2104](https://github.com/zammad/zammad/issues/2104) [[bug](https://github.com/zammad/zammad/labels/bug)]
+- Update button unusable after modal dialog is gone [\#2105](https://github.com/zammad/zammad/issues/2105) [[bug](https://github.com/zammad/zammad/labels/bug)]
+- Viewpoint::EWS::EwsFolderNotFound during Exchange connection [\#1802](https://github.com/zammad/zammad/issues/1802) [[UX/UI](https://github.com/zammad/zammad/labels/UX/UI)] [[bug](https://github.com/zammad/zammad/labels/bug)]
+- A ticket created by customer via web with state new is set to open on second update by customer [\#2118](https://github.com/zammad/zammad/issues/2118) [[bug](https://github.com/zammad/zammad/labels/bug)]
+- autoreply email filter [\#1316](https://github.com/zammad/zammad/issues/1316) [[bug](https://github.com/zammad/zammad/labels/bug)] [[trigger](https://github.com/zammad/zammad/labels/trigger)]
+- No mime\_type on nil message \(no message body\) / email will not be processed [\#2087](https://github.com/zammad/zammad/issues/2087) [[bug](https://github.com/zammad/zammad/labels/bug)]
+- Report profiles: "before\(relative\)" and friends broken. [\#2098](https://github.com/zammad/zammad/issues/2098) [[bug](https://github.com/zammad/zammad/labels/bug)] [[reporting](https://github.com/zammad/zammad/labels/reporting)]
+- Changing data type of new object attributes will lead to errors [\#2099](https://github.com/zammad/zammad/issues/2099) [[bug](https://github.com/zammad/zammad/labels/bug)]
+- Autocomplete hangs on dot in the new user form [\#2058](https://github.com/zammad/zammad/issues/2058) [[bug](https://github.com/zammad/zammad/labels/bug)]
+- Unable to create an overview with a new ticket file which is starting with number like "1\_test" [\#2090](https://github.com/zammad/zammad/issues/2090) [[bug](https://github.com/zammad/zammad/labels/bug)]
+- Inline images are lost on forward or quoted reply [\#2101](https://github.com/zammad/zammad/issues/2101) [[bug](https://github.com/zammad/zammad/labels/bug)]
+- Filename of images \(c&p\) downloaded via lightbox is invalid [\#1246](https://github.com/zammad/zammad/issues/1246) [[bug](https://github.com/zammad/zammad/labels/bug)]
+- Special characters get HTML encoded when displayed in overviews was\(Umlauts in overview not working\) [\#2046](https://github.com/zammad/zammad/issues/2046) [[bug](https://github.com/zammad/zammad/labels/bug)]
+- Scheduler will stop after removing own ticket attribute but overview with condition which is including attribute git dropped [\#2016](https://github.com/zammad/zammad/issues/2016) [[bug](https://github.com/zammad/zammad/labels/bug)]
+- Unable to create an overview with a new ticket file which is starting with number like "1\_test" [\#2090](https://github.com/zammad/zammad/issues/2090) [[bug](https://github.com/zammad/zammad/labels/bug)]
+- escalation\_at are not updated correctly [\#2082](https://github.com/zammad/zammad/issues/2082) [[bug](https://github.com/zammad/zammad/labels/bug)]
+- Inserting space character in translations \(in Overviews menu item and User menu\) [\#1531](https://github.com/zammad/zammad/issues/1531) [[bug](https://github.com/zammad/zammad/labels/bug)]
+- Cannot close tickets when you reply to an E-Mail at the same time [\#2069](https://github.com/zammad/zammad/issues/2069) [[bug](https://github.com/zammad/zammad/labels/bug)] [[trigger](https://github.com/zammad/zammad/labels/trigger)]
+- Error after apt upgrade on ubuntu 16.04 [\#2054](https://github.com/zammad/zammad/issues/2054) [[bug](https://github.com/zammad/zammad/labels/bug)]
 
 \* *This Change Log was automatically generated by [github_changelog_generator](https://github.com/skywinder/Github-Changelog-Generator)*

Gemfile.lock

@@ -105,7 +105,7 @@ GEM
       thor
     crack (0.4.3)
       safe_yaml (~> 1.0.0)
-    crass (1.0.3)
+    crass (1.0.4)
     daemons (1.2.5)
     dalli (2.7.6)
     delayed_job (4.1.3)
@@ -118,7 +118,7 @@ GEM
     docile (1.1.5)
     domain_name (0.5.20180417)
       unf (>= 0.0.5, < 1.0.0)
-    doorkeeper (4.2.6)
+    doorkeeper (4.4.0)
       railties (>= 4.2)
     eco (1.0.0)
       coffee-script
@@ -238,7 +238,7 @@ GEM
     net-ldap (0.16.1)
     netrc (0.11.0)
     nio4r (2.3.0)
-    nokogiri (1.8.2)
+    nokogiri (1.8.4)
       mini_portile2 (~> 2.3.0)
     nori (2.6.0)
     notiffany (0.1.1)
@@ -306,7 +306,7 @@ GEM
       slop (~> 3.0)
     public_suffix (3.0.1)
     puma (3.11.0)
-    rack (2.0.4)
+    rack (2.0.5)
     rack-livereload (0.3.16)
       rack
     rack-test (1.0.0)
@@ -405,7 +405,7 @@ GEM
       simplecov (>= 0.4.1)
     slack-notifier (2.3.1)
     slop (3.6.0)
-    sprockets (3.7.1)
+    sprockets (3.7.2)
       concurrent-ruby (~> 1.0)
       rack (> 1, < 3)
     sprockets-rails (3.2.1)

app/assets/javascripts/app/controllers/_application_controller.coffee

@@ -663,6 +663,7 @@ class App.ControllerModal extends App.Controller
   large: false
   small: false
   head: '?'
+  autoFocusOnFirstInput: true
   container: null
   buttonClass: 'btn--success'
   centerButtons: []
@@ -812,7 +813,8 @@ class App.ControllerModal extends App.Controller
     @onShown(e)
 
   onShown: (e) =>
-    @$('input:not([disabled]):not([type="hidden"]):not(".btn"), textarea').first().focus()
+    if @autoFocusOnFirstInput
+      @$('input:not([disabled]):not([type="hidden"]):not(".btn"), textarea').first().focus()
     @initalFormParams = @formParams()
 
   localOnClose: (e) =>

app/assets/javascripts/app/controllers/_application_controller_generic.coffee

@@ -1285,6 +1285,7 @@ class App.Import extends App.ControllerModal
   buttonClose: true
   buttonCancel: true
   buttonSubmit: 'Import'
+  autoFocusOnFirstInput: false
   head: 'Import'
   large: true
   templateDirectory: 'generic/object_import'
@@ -1296,6 +1297,7 @@ class App.Import extends App.ControllerModal
     content = $(App.view("#{@templateDirectory}/index")(
       head: 'Import'
       import_example_url: "#{@baseUrl}/import_example"
+      deleteOption: @deleteOption
     ))
 
     # check if data is processing...
@@ -1342,6 +1344,7 @@ class App.ImportTryResult extends App.ControllerModal
   buttonClose: true
   buttonCancel: true
   buttonSubmit: 'Yes, start real import.'
+  autoFocusOnFirstInput: false
   head: 'Import'
   large: true
   templateDirectory: 'generic/object_import/'
@@ -1386,6 +1389,7 @@ class App.ImportResult extends App.ControllerModal
   buttonClose: true
   buttonCancel: true
   buttonSubmit: 'Close'
+  autoFocusOnFirstInput: false
   head: 'Import'
   large: true
   templateDirectory: 'generic/object_import/'

app/assets/javascripts/app/controllers/_channel/email.coffee

@@ -60,68 +60,23 @@ class App.ChannelEmailFilter extends App.Controller
 
   new: (e) =>
     e.preventDefault()
-    new App.ChannelEmailFilterEdit(
+    new App.ControllerGenericNew(
+      pageData:
+        object: 'Postmaster Filter'
+      genericObject: 'PostmasterFilter'
       container: @el.closest('.content')
+      callback: @load
     )
-
+    
   edit: (id, e) =>
     e.preventDefault()
-    new App.ChannelEmailFilterEdit(
-      object:    App.PostmasterFilter.find(id)
+    new App.ControllerGenericEdit(
+      id: id,
+      pageData:
+        object: 'Postmaster Filter'
+      genericObject: 'PostmasterFilter'
       container: @el.closest('.content')
-    )
-
-class App.ChannelEmailFilterEdit extends App.ControllerModal
-  buttonClose: true
-  buttonCancel: true
-  buttonSubmit: true
-  head: 'Postmaster Filter'
-
-  content: =>
-    if @object
-      @form = new App.ControllerForm(
-        model:     App.PostmasterFilter,
-        params:    @object,
-        autofocus: true,
-      )
-    else
-      @form = new App.ControllerForm(
-        model:     App.PostmasterFilter,
-        autofocus: true,
-      )
-
-    @form.form
-
-  onSubmit: (e) =>
-    e.preventDefault()
-
-    # get params
-    params = @formParam(e.target)
-    params['channel'] = 'email'
-
-    object = @object || new App.PostmasterFilter
-    object.load(params)
-
-    # validate form
-    errors = @form.validate(params)
-
-    # show errors in form
-    if errors
-      @log 'error', errors
-      @formValidate(form: e.target, errors: errors)
-      return false
-
-    # disable form
-    @formDisable(e)
-
-    # save object
-    object.save(
-      done: =>
-        @close()
-      fail: (settings, details) =>
-        @log 'errors', details
-        @formEnable(e)
-        @form.showAlert(details.error_human || details.error || 'Unable to create object!')
+      callback: @load
     )
 
 class App.ChannelEmailSignature extends App.Controller

app/assets/javascripts/app/controllers/_ui_element/object_manager_attribute.coffee

@@ -28,6 +28,12 @@ class App.UiElement.object_manager_attribute extends App.UiElement.ApplicationUi
       boolean: 'Boolean'
       integer: 'Integer'
 
+    # if attribute already exists, do not allow to change it anymore
+    if params.data_type
+      for key, value of options
+        if key isnt params.data_type
+          delete options[key]
+
     configureAttributes = [
       { name: attribute.name, display: '', tag: 'select', null: false, options: options, translate: true, default: 'input', disabled: attribute.disabled },
     ]

app/assets/javascripts/app/controllers/_ui_element/richtext.coffee

@@ -73,12 +73,19 @@ class App.UiElement.richtext
               @attachmentPlaceholder.addClass('hide')
               @attachmentUpload.removeClass('hide')
               @cancelContainer.removeClass('hide')
+              # Disable the create ticket button during uploading
+              $('.main .newTicket .page-content .js-submit')
+                .text(App.i18n.translateInline('Uploading'))
+                .addClass('is-disabled')
               App.Log.debug 'UiElement.richtext', 'upload start'
 
             onAborted: =>
               @attachmentPlaceholder.removeClass('hide')
               @attachmentUpload.addClass('hide')
               item.find('input').val('')
+              $('.main .newTicket .page-content .js-submit')
+                .text(App.i18n.translateInline('Create'))
+                .removeClass('is-disabled')
 
             # Called after received response from the server
             onCompleted: (response) =>
@@ -93,6 +100,9 @@ class App.UiElement.richtext
 
               renderFile(response.data)
               item.find('input').val('')
+              $('.main .newTicket .page-content .js-submit')
+                .text(App.i18n.translateInline('Create'))
+                .removeClass('is-disabled')
 
               App.Log.debug 'UiElement.richtext', 'upload complete', response.data
 

app/assets/javascripts/app/controllers/agent_ticket_create.coffee

@@ -503,6 +503,10 @@ class App.TicketCreate extends App.Controller
           if !confirm(App.i18n.translateContent('You use %s in text but no attachment is attached. Do you want to continue?', matchingWord))
             return
 
+    # add sidebar params
+    if @sidebarWidget && @sidebarWidget.postParams
+      @sidebarWidget.postParams(ticket: ticket)
+
     # disable form
     @formDisable(e)
     ui = @

app/assets/javascripts/app/controllers/agent_ticket_create/sidebar.coffee

@@ -13,6 +13,11 @@ class App.TicketCreateSidebar extends App.Controller
       if backend && backend.commit
         backend.commit(args)
 
+  postParams: (args) =>
+    for key, backend of @sidebarBackends
+      if backend && backend.postParams
+        backend.postParams(args)
+
   render: (params) =>
     if params
       @params = params

app/assets/javascripts/app/controllers/idoit_object_selector.coffee

@@ -22,11 +22,11 @@ class App.IdoitObjectSelector extends App.ControllerModal
 
         @contentInline.find('.js-typeSelect').html(@renderTypeSelector(result))
 
-        @contentInline.filter('.js-search').on('change', 'select, input', (e) =>
+        @contentInline.on('change', 'input.js-shadow', (e) =>
           params = @formParam(e.target)
           @search(params)
         )
-        @contentInline.filter('.js-search').on('keyup', 'input', (e) =>
+        @contentInline.on('keyup', 'input.js-searchField', (e) =>
           params = @formParam(e.target)
           @search(params)
         )

app/assets/javascripts/app/controllers/object_manager.coffee

@@ -143,12 +143,17 @@ class Items extends App.ControllerSubContent
     e.preventDefault()
     id   = $(e.target).closest('tr').data('id')
     item = App.ObjectManagerAttribute.find(id)
+    ui = @
     @ajax(
       id:    "object_manager_attributes/#{id}"
       type:  'DELETE'
       url:   "#{@apiPath}/object_manager_attributes/#{id}"
       success: (data) =>
         @render()
+      error: (jqXHR, textStatus, errorThrown) ->
+        ui.log 'errors'
+        # this code is unreachable so alert will do fine
+        alert(jqXHR.responseJSON.error)
     )
 
   discard: (e) ->

app/assets/javascripts/app/controllers/organization_profile.coffee

@@ -195,6 +195,7 @@ class Member extends App.ObserverController
     lastname: true
     login: true
     email: true
+    active: true
   globalRerender: false
 
   render: (user) =>

app/assets/javascripts/app/controllers/text_module.coffee

@@ -12,6 +12,7 @@ class Index extends App.ControllerSubContent
         new App.Import(
           baseUrl: '/api/v1/text_modules'
           container: @el.closest('.content')
+          deleteOption: true
         )
       pageData:
         home: 'text_modules'

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

@@ -34,6 +34,11 @@ class App.TicketOverview extends App.Controller
     @bind 'ui:rerender', =>
       @renderBatchOverlay()
 
+    load = (data) =>
+      App.Collection.loadAssets(data.assets)
+      @formMeta = data.form_meta
+    @bindId = App.TicketCreateCollection.bind(load)
+
   startDragItem: (event) =>
     return if !@batchSupport
     @grabbedItem = $(event.currentTarget)
@@ -413,17 +418,10 @@ class App.TicketOverview extends App.Controller
 
     groupId = @hoveredBatchEntry.attr('data-id')
     group = App.Group.find(groupId)
-    users = []
-
-    for user_id in _.uniq(group.user_ids)
-      if App.User.exists(user_id)
-        user = App.User.find(user_id)
-        if user.active is true
-          users.push user
 
     @batchAssignGroupName.text group.displayName()
     @batchAssignGroupInner.html $(App.view('ticket_overview/batch_overlay_user_group')(
-      users: users
+      users: @usersInGroups([groupId])
       groups: []
       groupId: groupId
     ))
@@ -542,6 +540,19 @@ class App.TicketOverview extends App.Controller
       .velocity({ opacity: [0.5, 1] }, { duration: 120 })
       .velocity({ opacity: [1, 0.5] }, { duration: 60, delay: 40 })
 
+  usersInGroups: (group_ids) ->
+    ids_by_group = _.chain(@formMeta?.dependencies?.group_id)
+      .pick(group_ids)
+      .values()
+      .map( (e) -> e.owner_id)
+      .value()
+
+    # Underscore's intersection doesn't work when chained
+    ids_in_all_groups = _.intersection(ids_by_group...)
+
+    users = App.User.findAll(ids_in_all_groups)
+    _.sortBy(users, (user) -> user.firstname)
+
   render: ->
     elLocal = $(App.view('ticket_overview/index')())
 
@@ -589,47 +600,42 @@ class App.TicketOverview extends App.Controller
     @refreshElements()
 
   renderOptions: =>
-    macros = App.Macro.findAllByAttribute('active', true)
-    groups = App.Group.findAllByAttribute('active', true)
-    users = []
+    @renderOptionsGroups()
+    @renderOptionsMacros()
+
+  renderOptionsGroups: =>
     items = @el.find('[name="bulk"]:checked')
 
-    # find all possible owners for selected tickets
-    possibleUsers = {}
-    possibleUserGroups = {}
-    for item in items
-      #console.log "selected items with id ", $(item).val()
-      ticket = App.Ticket.find($(item).val())
-      if !possibleUserGroups[ticket.group_id.toString()]
-        group = App.Group.find(ticket.group_id)
-        for user_id in group.user_ids
-          if !possibleUserGroups[ticket.group_id.toString()]
-            possibleUsers[user_id.toString()] = true
-          else
-            hit = false
-            for user_id, exists of possibleUsers
-              if possibleUsers[user_id.toString()]
-                hit = true
-            if !hit
-              delete possibleUsers[user_id.toString()]
-        possibleUserGroups[ticket.group_id.toString()] = true
-    for user_id, _exists of possibleUsers
-      if App.User.exists(user_id)
-        user = App.User.find(user_id)
-        if user.active is true
-          users.push user
+    # we want to display all users for which we can assign the tickets directly
+    # for this we need to get the groups of all selected tickets
+    # after we got those we need to check which users are available in all groups
+    # users that are not in all groups can't get the tickets assigned
+    ticket_ids       = _.map(items, (el) -> $(el).val() )
+    ticket_group_ids = _.map(App.Ticket.findAll(ticket_ids), (ticket) -> ticket.group_id)
+    users            = @usersInGroups(ticket_group_ids)
+
+    # get the list of possible groups for the current user
+    # from the TicketCreateCollection
+    # (filled for e.g. the TicketCreation or TicketZoom assignment)
+    # and order them by name
+    group_ids     = _.keys(@formMeta?.dependencies?.group_id)
+    groups        = App.Group.findAll(group_ids)
+    groups_sorted = _.sortBy(groups, (group) -> group.name)
+
+    # get the number of visible users per group
+    # from the TicketCreateCollection
+    # (filled for e.g. the TicketCreation or TicketZoom assignment)
     for group in groups
-      valid_user_ids = []
-      for user_id in _.uniq(group.user_ids)
-        if App.User.exists(user_id)
-          if App.User.find(user_id).active is true
-            valid_user_ids.push user_id
-      group.valid_user_ids = valid_user_ids
+      group.valid_users_count = @formMeta?.dependencies?.group_id?[group.id]?.owner_id.length || 0
 
     @batchAssignInner.html $(App.view('ticket_overview/batch_overlay_user_group')(
       users: users
-      groups: groups
+      groups: groups_sorted
     ))
+
+  renderOptionsMacros: =>
+    macros = App.Macro.search(filter: { active: true }, sortBy:'name', order:'DESC')
+
     @batchMacro.html $(App.view('ticket_overview/batch_overlay_macro')(
       macros: macros
     ))
@@ -695,9 +701,10 @@ class App.TicketOverview extends App.Controller
   changed: ->
     false
 
-  release: ->
+  release: =>
     @keyboardOff()
     super
+    App.TicketCreateCollection.unbindById(@bindId)
 
   keyboardOn: =>
     $(window).off 'keydown.overview_navigation'

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

@@ -291,6 +291,7 @@ class App.TicketZoom extends App.Controller
 
   hide: =>
     @activeState = false
+    $('body > .modal').modal('hide')
     @positionPageHeaderStop()
     @autosaveStop()
     @shortcutNavigationstop()
@@ -633,6 +634,11 @@ class App.TicketZoom extends App.Controller
     # and the default is was not set before
     return if @isDefaultFollowUpStateSet
 
+    # and only if ticket is not in "new" state
+    if @ticket && @ticket.state_id
+      state = App.TicketState.findByAttribute('id', @ticket.state_id)
+      return if state && state.default_create is true
+
     # prevent multiple changes for the default follow up state
     @isDefaultFollowUpStateSet = true
 
@@ -816,6 +822,10 @@ class App.TicketZoom extends App.Controller
 
       ticket.article = article
 
+    # add sidebar params
+    if @sidebarWidget && @sidebarWidget.postParams
+      @sidebarWidget.postParams(ticket: ticket)
+
     if !ticket.article
       @submitPost(e, ticket)
       return

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

@@ -131,6 +131,7 @@ class EmailReply extends App.Controller
     selected = App.ClipBoard.getSelected('html')
     if selected
       selected = App.Utils.htmlCleanup(selected).html()
+      selected = App.Utils.htmlImage2DataUrl(selected)
     if !selected
       selected = App.ClipBoard.getSelected('text')
       if selected
@@ -173,6 +174,8 @@ class EmailReply extends App.Controller
     body = ''
     if article.content_type.match('html')
       body = App.Utils.textCleanup(article.body)
+      body = App.Utils.htmlImage2DataUrl(article.body)
+
     if article.content_type.match('plain')
       body = App.Utils.textCleanup(article.body)
       body = App.Utils.text2html(body)

app/assets/javascripts/app/controllers/ticket_zoom/article_new.coffee

@@ -184,11 +184,18 @@ class App.TicketZoomArticleNew extends App.Controller
             @attachmentPlaceholder.addClass('hide')
             @attachmentUpload.removeClass('hide')
             @cancelContainer.removeClass('hide')
+            # Disable the update ticket button during uploading
+            $('.active .attributeBar .js-submit')
+              .text(App.i18n.translateInline('Uploading'))
+              .addClass('is-disabled')
 
           onAborted: =>
             @attachmentPlaceholder.removeClass('hide')
             @attachmentUpload.addClass('hide')
             @$('.article-attachment input').val('')
+            $('.active .attributeBar .js-submit')
+              .text(App.i18n.translateInline('Update'))
+              .removeClass('is-disabled')
 
           # Called after received response from the server
           onCompleted: (response) =>
@@ -205,6 +212,9 @@ class App.TicketZoomArticleNew extends App.Controller
 
             @renderAttachment(response.data)
             @$('.article-attachment input').val('')
+            $('.active .attributeBar .js-submit')
+              .text(App.i18n.translateInline('Update'))
+              .removeClass('is-disabled')
 
           # Called during upload progress, first parameter
           # is decimal value from 0 to 100.

app/assets/javascripts/app/controllers/ticket_zoom/sidebar.coffee

@@ -14,6 +14,11 @@ class App.TicketZoomSidebar extends App.ObserverController
       if backend && backend.commit
         backend.commit(args)
 
+  postParams: (args) =>
+    for key, backend of @sidebarBackends
+      if backend && backend.postParams
+        backend.postParams(args)
+
   render: (ticket) =>
     @sidebarBackends ||= {}
     @sidebarItems = []

app/assets/javascripts/app/controllers/ticket_zoom/sidebar_idoit.coffee

@@ -92,6 +92,9 @@ class SidebarIdoit extends App.Controller
   showError: (message) =>
     @html App.i18n.translateInline(message)
 
+  reload: =>
+    @showObjectsContent()
+
   delete: (objectId) =>
     localObjects = []
     for localObjectId in @objectIds
@@ -102,13 +105,14 @@ class SidebarIdoit extends App.Controller
       @updateTicket(@ticket.id, @objectIds)
     @showObjectsContent()
 
-  commit: (args) =>
-    return if @ticket && @ticket.id
+  postParams: (args) =>
+    return if !args.ticket
+    return if args.ticket.created_at
     return if !@objectIds
     return if _.isEmpty(@objectIds)
-    return if !args
-    return if !args.ticket_id
-    @updateTicket(args.ticket_id, @objectIds)
+    args.ticket.preferences ||= {}
+    args.ticket.preferences.idoit ||= {}
+    args.ticket.preferences.idoit.object_ids = @objectIds
 
   updateTicket: (ticket_id, objectIds, callback) =>
     App.Ajax.request(

app/assets/javascripts/app/controllers/ticket_zoom/time_accounting.coffee

@@ -29,6 +29,10 @@ class App.TicketZoomTimeAccounting extends App.ControllerModal
     return if !@cancelCallback
     @cancelCallback()
 
+  onClose: ->
+    return if !@cancelCallback
+    @cancelCallback()
+
   onSubmit: =>
     @$('[name="time_unit"]').removeClass('has-error')
     params = @formParams()

app/assets/javascripts/app/lib/app_post/i18n.coffee

@@ -207,7 +207,7 @@ class _i18nSingleton extends Spine.Module
     return string if !string
 
     if App.Config.get('translation_inline')
-      return '<span class="translation" onclick="arguments[0].stopPropagation(); return false" contenteditable="true" title="' + App.Utils.htmlEscape(string) + '">' + App.Utils.htmlEscape(@translate(string)) + '</span>'
+      return '<span class="translation" onkeydown="arguments[0].stopPropagation(); return true" onclick="arguments[0].stopPropagation(); return false" contenteditable="true" title="' + App.Utils.htmlEscape(string) + '">' + App.Utils.htmlEscape(@translate(string)) + '</span>'
 
     translated = @translate(string, args, true, true)
 

app/assets/javascripts/app/lib/app_post/utils.coffee

@@ -262,6 +262,10 @@ class App.Utils
     # remove word markup
     @_removeWordMarkup(html)
 
+    # strip out browser-inserted (broken) link
+    # (see https://github.com/zammad/zammad/issues/2019)
+    @_stripDoubleDomainAnchors(html)
+
     # remove tags, keep content
     html.find('font, small, time, form, label').replaceWith( ->
       $(@).contents()
@@ -395,6 +399,15 @@ class App.Utils
       return window.word_filter(html)
     html
 
+  @_stripDoubleDomainAnchors: (html) ->
+    html.find('a').each( ->
+      origHref  = $(@).attr('href')
+      return if !origHref?
+
+      fixedHref = origHref.replace(/^https?:\/\/.*(?=(https?|#{config.http_type}):\/\/)/, '')
+      if origHref != fixedHref then $(@).attr('href', fixedHref)
+    )
+
   # signatureNeeded = App.Utils.signatureCheck(message, signature)
   @signatureCheck: (message, signature) ->
     messageText   = $('<div>' + message + '</div>').text().trim()
@@ -1080,3 +1093,23 @@ class App.Utils
       )
     App.Delay.set(a, 500, undefined, 'tags')
 
+  @htmlImage2DataUrl: (html) ->
+    return html if !html
+    return html if !html.match(/<img/i)
+    html = @_checkTypeOf("<div>#{html}</div>")
+
+    html.find('img').each( (index) ->
+      src = $(@).attr('src')
+      if !src.match(/^data:/i)
+        base64 = App.Utils._htmlImage2DataUrl(@)
+        $(@).attr('src', base64)
+    )
+    html.get(0).innerHTML
+
+  @_htmlImage2DataUrl: (img) ->
+    canvas = document.createElement('canvas')
+    canvas.width = img.width
+    canvas.height = img.height
+    ctx = canvas.getContext('2d')
+    ctx.drawImage(img, 0, 0)
+    canvas.toDataURL('image/png')

app/assets/javascripts/app/models/postmaster_filter.coffee

@@ -21,3 +21,6 @@ class App.PostmasterFilter extends App.Model
     'name',
   ]
   @configure_clone = true
+
+  @on 'create', (newRecord) ->
+    newRecord.channel = 'email'

app/assets/javascripts/app/models/ticket.coffee

@@ -1,5 +1,5 @@
 class App.Ticket extends App.Model
-  @configure 'Ticket', 'number', 'title', 'group_id', 'owner_id', 'customer_id', 'state_id', 'priority_id', 'article', 'tags', 'links', 'updated_at'
+  @configure 'Ticket', 'number', 'title', 'group_id', 'owner_id', 'customer_id', 'state_id', 'priority_id', 'article', 'tags', 'links', 'updated_at', 'preferences'
   @extend Spine.Model.Ajax
   @url: @apiPath + '/tickets'
   @configure_attributes = [

app/assets/javascripts/app/views/generic/object_import/index.jst.eco

@@ -14,10 +14,10 @@
 <h2><input checked="checked" disabled="disabled" type="checkbox" name="update" value="true"> <%- @T('Update existing records') %></h2>
 <%- @T('Update existing records with the attributes specified in the import data.') %>
 
-<!--
-<h2><input checked="" type="checkbox" name="delete" value="true"> <%- @T('Delete records') %></h2>
-<%- @T('Delete all existigs records first.') %>
--->
+<% if @deleteOption is true: %>
+<h2><input type="checkbox" name="delete" value="true"> <%- @T('Delete records') %></h2>
+<%- @T('Delete all existing records first.') %>
+<% end %>
 
 <h2><%- @T('Select CSV file') %></h2>
 <input name="file" type="file">

app/assets/javascripts/app/views/generic/table_row_group_by.jst.eco

@@ -1,3 +1,3 @@
 <tr>
-  <td colspan="<%= @columnsLength %>"><b><%= @groupByName %><% if @groupByCount:%> (<%= @groupByCount %>)<% end %></b></td>
+  <td colspan="<%= @columnsLength %>"><b><%- @groupByName %><% if @groupByCount:%> (<%= @groupByCount %>)<% end %></b></td>
 </tr>

app/assets/javascripts/app/views/integration/idoit_object_selector.jst.eco

@@ -1,5 +1,5 @@
 <form class="js-search flex horizontal">
   <div class="controls controls--select js-typeSelect"></div>
-  <input type="text" name="title" value="" autocomplete="off" placeholder="<%- @Ti('Search') %>" class="form-control"/>
+  <input type="text" name="title" value="" autocomplete="off" placeholder="<%- @Ti('Search') %>" class="form-control js-searchField"/>
 </form>
 <form class="js-result"></form>

app/assets/javascripts/app/views/object_manager/index.jst.eco

@@ -71,8 +71,12 @@
             <%- @T('will be deleted') %>
           <% else if item.to_migrate is true || item.to_config is true: %>
             <%- @T('has changed') %>
-          <% else if item.editable isnt false: %>
-            <a href="#" class="js-delete" title="<%- @Ti('Delete') %>"><%- @Icon('trash') %></a>
+          <% else if item.editable: %>
+            <% if item.deletable: %>
+              <a href="#" class="js-delete" title="<%- @Ti('Delete') %>"><%- @Icon('trash') %></a>
+            <% else: %>
+              <span class="is-disabled" title="<%= item.not_deletable_reason %>"><%- @Icon('trash') %></span>
+            <% end %>
           <% end %>
         </td>
       </tr>

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

@@ -8,6 +8,6 @@
 <div class="batch-overlay-assign-entry js-batch-overlay-entry" data-action="group_assign" data-id="<%= group.id %>">
 	<div class="js-batch-hover-target"><%- group.avatar(80) %></div>
   <div class="batch-overlay-assign-entry-name"><%- group.displayName() %></div>
-  <div class="batch-overlay-assign-entry-detail"><%- @T('%s people', group.valid_user_ids.length) %></div>
+  <div class="batch-overlay-assign-entry-detail"><%- @T('%s people', group.valid_users_count) %></div>
 </div>
 <% end %>

app/assets/stylesheets/zammad.scss

@@ -9610,3 +9610,8 @@ body.fit {
 .flex-spacer {
   flex: 1;
 }
+
+span.is-disabled {
+  cursor: not-allowed;
+  opacity: 0.5;
+}

app/controllers/application_controller/handles_errors.rb

@@ -45,7 +45,9 @@ module ApplicationController::HandlesErrors
     respond_to do |format|
       format.json { render json: humanize_error(e.message), status: status }
       format.any do
+        errors = humanize_error(e.message)
         @exception = e
+        @message = errors[:error_human] || errors[:error] || param[:message]
         @traceback = !Rails.env.production?
         file = File.open(Rails.root.join('public', "#{status_code}.html"), 'r')
         render inline: file.read, status: status

app/controllers/concerns/creates_ticket_articles.rb

@@ -89,14 +89,17 @@ module CreatesTicketArticles
           preferences[store_key] = attachment[key]
         end
 
-        if !attachment[:data].match?(%r{^([A-Za-z0-9+/]{4})*([A-Za-z0-9+/]{4}|[A-Za-z0-9+/]{3}=|[A-Za-z0-9+/]{2}==)$})
+        begin
+          base64_data = attachment[:data].gsub(/[\r\n]/, '')
+          attachment_data = Base64.strict_decode64(base64_data)
+        rescue ArgumentError => e
           raise Exceptions::UnprocessableEntity, "Invalid base64 for attachment with index '#{index}'"
         end
 
         Store.add(
           object: 'Ticket::Article',
           o_id: article.id,
-          data: Base64.decode64(attachment[:data]),
+          data: attachment_data,
           filename: attachment[:filename],
           preferences: preferences,
         )

app/controllers/integration/exchange_controller.rb

@@ -16,18 +16,21 @@ class Integration::ExchangeController < ApplicationController
         client.http.ssl_config.verify_mode = OpenSSL::SSL::VERIFY_NONE
       end
 
-      {
-        endpoint: client.try(:autodiscover).try(:ews_url),
-      }
+      begin
+        { endpoint: client.autodiscover&.ews_url }
+      rescue Errno::EADDRNOTAVAIL
+        {}
+      end
     end
   end
 
   def folders
     answer_with do
       Sequencer.process('Import::Exchange::AvailableFolders',
-                        parameters: {
-                          ews_config: ews_config
-                        })
+                        parameters: { ews_config: ews_config })
+               .tap do |res|
+                 raise 'No folders found for given user credentials.' if res[:folders].blank?
+               end
     end
   end
 
@@ -35,14 +38,12 @@ class Integration::ExchangeController < ApplicationController
     answer_with do
       raise 'Please select at least one folder.' if params[:folders].blank?
 
-      examples = Sequencer.process('Import::Exchange::AttributesExamples',
-                                   parameters: {
-                                     ews_folder_ids: params[:folders],
-                                     ews_config:     ews_config
-                                   })
-      examples.tap do |result|
-        raise 'No entries found in selected folder(s).' if result[:attributes].blank?
-      end
+      Sequencer.process('Import::Exchange::AttributesExamples',
+                        parameters: { ews_folder_ids: params[:folders],
+                                      ews_config:     ews_config })
+               .tap do |res|
+                 raise 'No entries found in selected folder(s).' if res[:attributes].blank?
+               end
     end
   end
 

app/controllers/integration/idoit_controller.rb

@@ -38,10 +38,12 @@ class Integration::IdoitController < ApplicationController
   def update
     params[:object_ids] ||= []
     ticket = Ticket.find(params[:ticket_id])
-    access!(ticket, 'read')
-    ticket.preferences[:idoit] ||= {}
-    ticket.preferences[:idoit][:object_ids] = Array(params[:object_ids]).uniq
-    ticket.save!
+    ticket.with_lock do
+      access!(ticket, 'read')
+      ticket.preferences[:idoit] ||= {}
+      ticket.preferences[:idoit][:object_ids] = Array(params[:object_ids]).uniq
+      ticket.save!
+    end
 
     render json: {
       result: 'ok',

app/controllers/object_manager_attributes_controller.rb

@@ -77,6 +77,9 @@ class ObjectManagerAttributesController < ApplicationController
       name: object_manager_attribute.name,
     )
     model_destroy_render_item
+  rescue => e
+    logger.error e
+    raise Exceptions::UnprocessableEntity, e
   end
 
   # POST /object_manager_attributes_discard_changes

app/controllers/organizations_controller.rb

@@ -354,6 +354,7 @@ curl http://localhost/api/v1/organization/{id} -v -u #{login}:#{password} -H "Co
         col_sep: params[:col_sep] || ',',
       },
       try: params[:try],
+      delete: params[:delete],
     )
     render json: result, status: :ok
   end

app/controllers/sessions_controller.rb

@@ -2,7 +2,7 @@
 
 class SessionsController < ApplicationController
   prepend_before_action :authentication_check, only: %i[switch_to_user list delete]
-  skip_before_action :verify_csrf_token, only: %i[create show destroy create_omniauth create_sso]
+  skip_before_action :verify_csrf_token, only: %i[create show destroy create_omniauth failure_omniauth create_sso]
 
   # "Create" a login, aka "log the user in"
   def create
@@ -164,6 +164,10 @@ class SessionsController < ApplicationController
     redirect_to '/'
   end
 
+  def failure_omniauth
+    raise Exceptions::UnprocessableEntity, "Message from #{params[:strategy]}: #{params[:message]}"
+  end
+
   def create_sso
 
     # in case, remove switched_from_user_id

app/controllers/text_modules_controller.rb

@@ -193,6 +193,7 @@ curl http://localhost/api/v1/text_modules.json -v -u #{login}:#{password} -H "Co
         col_sep: params[:col_sep] || ',',
       },
       try: params[:try],
+      delete: params[:delete],
     )
     render json: result, status: :ok
   end

app/controllers/ticket_articles_controller.rb

@@ -112,6 +112,9 @@ class TicketArticlesController < ApplicationController
     clean_params = Ticket::Article.association_name_to_id_convert(params)
     clean_params = Ticket::Article.param_cleanup(clean_params, true)
 
+    # only apply preferences changes (keep not updated keys/values)
+    clean_params = article.param_preferences_merge(clean_params)
+
     article.update!(clean_params)
 
     if response_expand?

app/controllers/tickets_controller.rb

@@ -222,6 +222,9 @@ class TicketsController < ApplicationController
     clean_params = Ticket.association_name_to_id_convert(params)
     clean_params = Ticket.param_cleanup(clean_params, true)
 
+    # only apply preferences changes (keep not updated keys/values)
+    clean_params = ticket.param_preferences_merge(clean_params)
+
     # overwrite params
     if !current_user.permissions?('ticket.agent')
       %i[owner owner_id customer customer_id organization organization_id preferences].each do |key|

app/controllers/users_controller.rb

@@ -1107,6 +1107,7 @@ curl http://localhost/api/v1/users/avatar -v -u #{login}:#{password} -H "Content
         col_sep: params[:col_sep] || ',',
       },
       try: params[:try],
+      delete: params[:delete],
     )
     render json: result, status: :ok
   end

app/models/application_model/can_cleanup_param.rb

@@ -33,30 +33,30 @@ returns
 
       data = {}
       params.each do |key, value|
-        data[key.to_sym] = value
+        data[key.to_s] = value
       end
 
       # ignore id for new objects
       if new_object && params[:id]
-        data.delete(:id)
+        data.delete('id')
       end
 
       # only use object attributes
-      clean_params = {}
+      clean_params = ActiveSupport::HashWithIndifferentAccess.new
       new.attributes.each_key do |attribute|
-        next if !data.key?(attribute.to_sym)
+        next if !data.key?(attribute)
 
         # check reference records, referenced by _id attributes
         reflect_on_all_associations.map do |assoc|
           class_name = assoc.options[:class_name]
           next if !class_name
-          name = "#{assoc.name}_id".to_sym
+          name = "#{assoc.name}_id"
           next if !data.key?(name)
           next if data[name].blank?
           next if assoc.klass.lookup(id: data[name])
           raise ArgumentError, "Invalid value for param '#{name}': #{data[name].inspect}"
         end
-        clean_params[attribute.to_sym] = data[attribute.to_sym]
+        clean_params[attribute] = data[attribute]
       end
 
       # we do want to set this via database
@@ -89,5 +89,23 @@ returns
       end
       data
     end
+
+  end
+
+=begin
+
+merge preferences param
+
+  record = Model.find(123)
+
+  new_preferences = record.param_preferences_merge(param_preferences)
+
+=end
+
+  def param_preferences_merge(new_params)
+    return new_params if new_params.blank?
+    return new_params if preferences.blank?
+    new_params[:preferences] = preferences.merge(new_params[:preferences] || {})
+    new_params
   end
 end

app/models/channel/email_parser.rb

@@ -248,7 +248,7 @@ class Channel::EmailParser
       data[:body] = mail.body.decoded
       data[:body] = Encode.conv(mail.charset, data[:body])
 
-      if !data[:body].force_encoding('UTF-8').valid_encoding?
+      if data[:body].present? && !data[:body].force_encoding('UTF-8').valid_encoding?
         data[:body] = data[:body].encode('utf-8', 'binary', invalid: :replace, undef: :replace, replace: '?')
       end
       data[:content_type] = 'text/plain'
@@ -276,9 +276,11 @@ class Channel::EmailParser
     end
 
     # strip not wanted chars
-    data[:body].gsub!(/\n\r/, "\n")
-    data[:body].gsub!(/\r\n/, "\n")
-    data[:body].tr!("\r", "\n")
+    if data[:body].present?
+      data[:body].gsub!(/\n\r/, "\n")
+      data[:body].gsub!(/\r\n/, "\n")
+      data[:body].tr!("\r", "\n")
+    end
 
     # get mail date
     begin

app/models/concerns/can_csv_import.rb

@@ -16,6 +16,7 @@ module CanCsvImport
       col_sep: ',',
     },
     try: true,
+    delete: false,
   )
 
   result = Model.csv_import(
@@ -24,6 +25,7 @@ module CanCsvImport
       col_sep: ',',
     },
     try: true,
+    delete: false,
   )
 
   result = TextModule.csv_import(
@@ -32,6 +34,7 @@ module CanCsvImport
       col_sep: ',',
     },
     try: false,
+    delete: false,
   )
 
 returns
@@ -45,7 +48,25 @@ returns
 =end
 
     def csv_import(data)
+      try = true
+      if data[:try] != 'true' && data[:try] != true
+        try = false
+      end
+      delete = false
+      if data[:delete] == true || data[:delete] == 'true'
+        delete = true
+      end
+
       errors = []
+      if delete == true && @csv_delete_possible != true
+        errors.push "Delete is not possible for #{new.class}."
+        result = {
+          errors: errors,
+          try: try,
+          result: 'failed',
+        }
+        return result
+      end
 
       if data[:file].present?
         raise Exceptions::UnprocessableEntity, "No such file '#{data[:file]}'" if !File.exist?(data[:file])
@@ -60,7 +81,7 @@ returns
         errors.push "Unable to parse empty file/string for #{new.class}."
         result = {
           errors: errors,
-          try: data[:try],
+          try: try,
           result: 'failed',
         }
         return result
@@ -72,7 +93,7 @@ returns
         errors.push "Unable to parse file/string without header for #{new.class}."
         result = {
           errors: errors,
-          try: data[:try],
+          try: try,
           result: 'failed',
         }
         return result
@@ -89,7 +110,7 @@ returns
         errors.push "No records found in file/string for #{new.class}."
         result = {
           errors: errors,
-          try: data[:try],
+          try: try,
           result: 'failed',
         }
         return result
@@ -127,13 +148,22 @@ returns
         payload.push attributes
       end
 
-      # create or update records
-      csv_object_ids_ignored = @csv_object_ids_ignored || []
-      records = []
       stats = {
         created: 0,
         updated: 0,
       }
+
+      # delete
+      if delete == true
+        stats[:deleted] = self.count
+        if try == false
+          destroy_all
+        end
+      end
+
+      # create or update records
+      csv_object_ids_ignored = @csv_object_ids_ignored || []
+      records = []
       line_count = 0
       payload.each do |attributes|
         line_count += 1
@@ -166,7 +196,7 @@ returns
         # create object
         Transaction.execute(disable_notification: true, reset_user_id: true) do
           UserInfo.current_user_id = clean_params[:updated_by_id] || clean_params[:created_by_id]
-          if !record
+          if !record || delete == true
             stats[:created] += 1
             begin
               csv_verify_attributes(clean_params)
@@ -177,7 +207,7 @@ returns
                 clean_params[:updated_by_id] = 1
               end
               record = new(clean_params)
-              next if data[:try] == 'true' || data[:try] == true
+              next if try == true
               record.associations_from_param(attributes)
               record.save!
             rescue => e
@@ -186,7 +216,7 @@ returns
             end
           else
             stats[:updated] += 1
-            next if data[:try] == 'true' || data[:try] == true
+            next if try == true
             begin
               csv_verify_attributes(clean_params)
               clean_params = param_cleanup(clean_params)
@@ -218,7 +248,7 @@ returns
         stats: stats,
         records: records,
         errors: errors,
-        try: data[:try],
+        try: try,
         result: result,
       }
 
@@ -371,5 +401,20 @@ end
       @csv_attributes_ignored = attributes
     end
 
+=begin
+
+serve methode to define if delete option is possible or not
+
+class Model < ApplicationModel
+  include CanCsvImport
+  csv_delete_possible true
+
+end
+
+=end
+
+    def csv_delete_possible(value)
+      @csv_delete_possible = value
+    end
   end
 end

app/models/concerns/checks_latest_change_observed.rb

@@ -3,19 +3,11 @@ module ChecksLatestChangeObserved
   extend ActiveSupport::Concern
 
   included do
-    after_create  :latest_change_set_from_observer
-    after_update  :latest_change_set_from_observer
-    after_touch   :latest_change_set_from_observer
-    after_destroy :latest_change_set_from_observer_destroy
+    after_commit :latest_change_set_from_observer
   end
 
   def latest_change_set_from_observer
-    self.class.latest_change_set(updated_at)
-    true
-  end
-
-  def latest_change_set_from_observer_destroy
-    self.class.latest_change_set(nil)
-    true
+    latest_change = destroyed? ? nil : updated_at
+    self.class.latest_change_set(latest_change)
   end
 end

app/models/concerns/has_group_relation_definition.rb

@@ -0,0 +1,64 @@
+# Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/
+module HasGroupRelationDefinition
+  extend ActiveSupport::Concern
+
+  included do
+
+    self.table_name   = "groups_#{group_relation_model_identifier}s"
+    self.primary_keys = ref_key, :group_id, :access
+
+    belongs_to group_relation_model_identifier
+    belongs_to :group
+
+    validates :access, presence: true
+    validate :validate_access
+
+    after_save :touch_related
+    after_destroy :touch_related
+  end
+
+  private
+
+  def group_relation_instance
+    @group_relation_instance ||= send(group_relation_model_identifier)
+  end
+
+  def group_relation_model_identifier
+    @group_relation_model_identifier ||= self.class.group_relation_model_identifier
+  end
+
+  def touch_related
+    # rubocop:disable Rails/SkipsModelValidations
+    group.touch if group&.persisted?
+    group_relation_instance.touch if group_relation_instance&.persisted?
+    # rubocop:enable Rails/SkipsModelValidations
+  end
+
+  def validate_access
+    query = self.class.where(
+      group_relation_model_identifier => group_relation_instance,
+      group: group
+    )
+
+    query = if access == 'full'
+              query.where.not(access: 'full')
+            else
+              query.where(access: 'full')
+            end
+
+    return if !query.exists?
+    errors.add(:access, "#{group_relation_model_identifier.to_s.capitalize} can have full or granular access to group")
+  end
+
+  # methods defined here are going to extend the class, not the instance of it
+  class_methods do
+
+    def group_relation_model_identifier
+      @group_relation_model_identifier ||= model_name.singular.split('_').first.to_sym
+    end
+
+    def ref_key
+      @ref_key ||= "#{group_relation_model_identifier}_id".to_sym
+    end
+  end
+end

app/models/concerns/has_groups.rb

@@ -7,8 +7,12 @@ module HasGroups
 
     attr_accessor :group_access_buffer
 
-    after_create :process_group_access_buffer
-    after_update :process_group_access_buffer
+    after_save :process_group_access_buffer
+
+    # add association to Group, too but ignore it in asset output
+    Group.has_many group_through_identifier
+    Group.has_many model_name.collection.to_sym, through: group_through_identifier, after_add: :cache_update, after_remove: :cache_update, dependent: :destroy
+    Group.association_attributes_ignored group_through_identifier
 
     association_attributes_ignored :groups, group_through_identifier
 

app/models/group.rb

@@ -6,10 +6,10 @@ class Group < ApplicationModel
   include ChecksLatestChangeObserved
   include HasHistory
 
-  has_and_belongs_to_many  :users,         after_add: :cache_update, after_remove: :cache_update
-  belongs_to               :email_address
-  belongs_to               :signature
-  validates                :name, presence: true
+  belongs_to :email_address
+  belongs_to :signature
+
+  validates :name, presence: true
 
   activity_stream_permission 'admin.group'
 end

app/models/object_manager/attribute.rb

@@ -12,6 +12,9 @@ class ObjectManager::Attribute < ApplicationModel
   store :data_option
   store :data_option_new
 
+  before_create :check_datatype
+  before_update :check_datatype, :verify_possible_type_change
+
 =begin
 
 list of all attributes
@@ -29,12 +32,25 @@ list of all attributes
 
   def self.list_full
     result = ObjectManager::Attribute.all.order('position ASC, name ASC')
+    references = ObjectManager::Attribute.attribute_to_references_hash
     attributes = []
     assets = {}
     result.each do |item|
       attribute = item.attributes
       attribute[:object] = ObjectLookup.by_id(item.object_lookup_id)
       attribute.delete('object_lookup_id')
+
+      # an attribute is deletable if it is both editable and not referenced by other Objects (Triggers, Overviews, Schedulers)
+      deletable = true
+      not_deletable_reason = ''
+      if ObjectManager::Attribute.attribute_used_by_references?(attribute[:object], attribute['name'], references)
+        deletable = false
+        not_deletable_reason = ObjectManager::Attribute.attribute_used_by_references_humaniced(attribute[:object], attribute['name'], references)
+      end
+      attribute[:deletable] = attribute['editable'] && deletable == true
+      if not_deletable_reason.present?
+        attribute[:not_deletable_reason] = "This attribute is referenced by #{not_deletable_reason} and thus cannot be deleted!"
+      end
       attributes.push attribute
     end
     attributes
@@ -311,7 +327,6 @@ possible types
         record.check_editable
         record.check_name
       end
-      record.check_datatype
       record.save!
       return record
     end
@@ -331,7 +346,6 @@ possible types
       record.check_editable
       record.check_name
     end
-    record.check_datatype
     record.save!
     record
   end
@@ -354,6 +368,10 @@ use "force: true" to delete also not editable fields
     # lookups
     if data[:object]
       data[:object_lookup_id] = ObjectLookup.by_name(data[:object])
+    elsif data[:object_lookup_id]
+      data[:object] = ObjectLookup.by_id(data[:object_lookup_id])
+    else
+      raise 'ERROR: need object or object_lookup_id param!'
     end
 
     data[:name].downcase!
@@ -371,6 +389,12 @@ use "force: true" to delete also not editable fields
       raise "ERROR: #{data[:object]}.#{data[:name]} can't be removed!"
     end
 
+    # check to make sure that no triggers, overviews, or schedulers references this attribute
+    if ObjectManager::Attribute.attribute_used_by_references?(data[:object], data[:name])
+      text = ObjectManager::Attribute.attribute_used_by_references_humaniced(data[:object], data[:name])
+      raise "ERROR: #{data[:object]}.#{data[:name]} is referenced by #{text} and thus cannot be deleted!"
+    end
+
     # if record is to create, just destroy it
     if record.to_create
       record.destroy
@@ -721,6 +745,109 @@ to send no browser reload event, pass false
     true
   end
 
+=begin
+
+where attributes are used by triggers, overviews or schedulers
+
+  result = ObjectManager::Attribute.attribute_to_references_hash
+
+  result = {
+    ticket.category: {
+      Trigger: ['abc', 'xyz'],
+      Overview: ['abc1', 'abc2'],
+    },
+    ticket.field_b: {
+      Trigger: ['abc'],
+      Overview: ['abc1', 'abc2'],
+    },
+  },
+
+=end
+
+  def self.attribute_to_references_hash
+    objects = Trigger.select(:name, :condition) + Overview.select(:name, :condition) + Job.select(:name, :condition)
+    attribute_list = {}
+    objects.each do |item|
+      item.condition.each do |condition_key, _condition_attributes|
+        attribute_list[condition_key] ||= {}
+        attribute_list[condition_key][item.class.name] ||= []
+        next if attribute_list[condition_key][item.class.name].include?(item.name)
+        attribute_list[condition_key][item.class.name].push item.name
+      end
+    end
+    attribute_list
+  end
+
+=begin
+
+is certain attribute used by triggers, overviews or schedulers
+
+  ObjectManager::Attribute.attribute_used_by_references?('Ticket', 'attribute_name')
+
+=end
+
+  def self.attribute_used_by_references?(object_name, attribute_name, references = attribute_to_references_hash)
+    references.each do |reference_key, _relations|
+      local_object, local_attribute = reference_key.split('.')
+      next if local_object != object_name.downcase
+      next if local_attribute != attribute_name
+      return true
+    end
+    false
+  end
+
+=begin
+
+is certain attribute used by triggers, overviews or schedulers
+
+  result = ObjectManager::Attribute.attribute_used_by_references('Ticket', 'attribute_name')
+
+  result = {
+    Trigger: ['abc', 'xyz'],
+    Overview: ['abc1', 'abc2'],
+  }
+
+=end
+
+  def self.attribute_used_by_references(object_name, attribute_name, references = attribute_to_references_hash)
+    result = {}
+    references.each do |reference_key, relations|
+      local_object, local_attribute = reference_key.split('.')
+      next if local_object != object_name.downcase
+      next if local_attribute != attribute_name
+      relations.each do |relation, relation_names|
+        result[relation] ||= []
+        result[relation].push relation_names.sort
+      end
+      break
+    end
+    result
+  end
+
+=begin
+
+is certain attribute used by triggers, overviews or schedulers
+
+  text = ObjectManager::Attribute.attribute_used_by_references_humaniced('Ticket', 'attribute_name', references)
+
+=end
+
+  def self.attribute_used_by_references_humaniced(object_name, attribute_name, references = nil)
+    result = if references.present?
+               ObjectManager::Attribute.attribute_used_by_references(object_name, attribute_name, references)
+             else
+               ObjectManager::Attribute.attribute_used_by_references(object_name, attribute_name)
+             end
+    not_deletable_reason = ''
+    result.each do |relation, relation_names|
+      if not_deletable_reason.present?
+        not_deletable_reason += '; '
+      end
+      not_deletable_reason += "#{relation}: #{relation_names.sort.join(',')}"
+    end
+    not_deletable_reason
+  end
+
   def self.reset_database_info(model)
     model.connection.schema_cache.clear!
     model.reset_column_information
@@ -743,6 +870,7 @@ to send no browser reload event, pass false
 
     record = object_lookup.name.constantize.new
     return true if !record.respond_to?(name.to_sym)
+    raise "#{name} already exists!" if record.attributes.key?(name) && new_record?
     return true if record.attributes.key?(name)
     raise "#{name} is a reserved word, please choose a different one"
   end
@@ -752,7 +880,13 @@ to send no browser reload event, pass false
     raise 'Attribute not editable!'
   end
 
+  private
+
   def check_datatype
+    local_data_option = data_option
+    if to_config == true
+      local_data_option = data_option_new
+    end
     if !data_type
       raise 'Need data_type param'
     end
@@ -760,62 +894,76 @@ to send no browser reload event, pass false
       raise "Invalid data_type param '#{data_type}'"
     end
 
-    if !data_option
-      raise 'Need data_type param'
+    if local_data_option.blank?
+      raise 'Need data_option param'
     end
-    if data_option[:null].nil?
+    if local_data_option[:null].nil?
       raise 'Need data_option[:null] param with true or false'
     end
 
     # validate data_option
     if data_type == 'input'
-      raise 'Need data_option[:type] param e. g. (text|password|tel|fax|email|url)' if !data_option[:type]
-      raise "Invalid data_option[:type] param '#{data_option[:type]}' (text|password|tel|fax|email|url)" if data_option[:type] !~ /^(text|password|tel|fax|email|url)$/
-      raise 'Need data_option[:maxlength] param' if !data_option[:maxlength]
-      raise "Invalid data_option[:maxlength] param #{data_option[:maxlength]}" if data_option[:maxlength].to_s !~ /^\d+?$/
+      raise 'Need data_option[:type] param e. g. (text|password|tel|fax|email|url)' if !local_data_option[:type]
+      raise "Invalid data_option[:type] param '#{local_data_option[:type]}' (text|password|tel|fax|email|url)" if local_data_option[:type] !~ /^(text|password|tel|fax|email|url)$/
+      raise 'Need data_option[:maxlength] param' if !local_data_option[:maxlength]
+      raise "Invalid data_option[:maxlength] param #{local_data_option[:maxlength]}" if local_data_option[:maxlength].to_s !~ /^\d+?$/
     end
 
     if data_type == 'richtext'
-      raise 'Need data_option[:maxlength] param' if !data_option[:maxlength]
-      raise "Invalid data_option[:maxlength] param #{data_option[:maxlength]}" if data_option[:maxlength].to_s !~ /^\d+?$/
+      raise 'Need data_option[:maxlength] param' if !local_data_option[:maxlength]
+      raise "Invalid data_option[:maxlength] param #{local_data_option[:maxlength]}" if local_data_option[:maxlength].to_s !~ /^\d+?$/
     end
 
     if data_type == 'integer'
       %i[min max].each do |item|
-        raise "Need data_option[#{item.inspect}] param" if !data_option[item]
-        raise "Invalid data_option[#{item.inspect}] param #{data_option[item]}" if data_option[item].to_s !~ /^\d+?$/
+        raise "Need data_option[#{item.inspect}] param" if !local_data_option[item]
+        raise "Invalid data_option[#{item.inspect}] param #{data_option[item]}" if local_data_option[item].to_s !~ /^\d+?$/
       end
     end
 
     if data_type == 'select' || data_type == 'tree_select' || data_type == 'checkbox'
-      raise 'Need data_option[:default] param' if !data_option.key?(:default)
-      raise 'Invalid data_option[:options] or data_option[:relation] param' if data_option[:options].nil? && data_option[:relation].nil?
-      if !data_option.key?(:maxlength)
-        data_option[:maxlength] = 255
+      raise 'Need data_option[:default] param' if !local_data_option.key?(:default)
+      raise 'Invalid data_option[:options] or data_option[:relation] param' if local_data_option[:options].nil? && local_data_option[:relation].nil?
+      if !local_data_option.key?(:maxlength)
+        local_data_option[:maxlength] = 255
       end
-      if !data_option.key?(:nulloption)
-        data_option[:nulloption] = true
+      if !local_data_option.key?(:nulloption)
+        local_data_option[:nulloption] = true
       end
     end
 
     if data_type == 'boolean'
-      raise 'Need data_option[:default] param true|false|undefined' if !data_option.key?(:default)
-      raise 'Invalid data_option[:options] param' if data_option[:options].nil?
+      raise 'Need data_option[:default] param true|false|undefined' if !local_data_option.key?(:default)
+      raise 'Invalid data_option[:options] param' if local_data_option[:options].nil?
     end
 
     if data_type == 'datetime'
-      raise 'Need data_option[:future] param true|false' if data_option[:future].nil?
-      raise 'Need data_option[:past] param true|false' if data_option[:past].nil?
-      raise 'Need data_option[:diff] param in hours' if data_option[:diff].nil?
+      raise 'Need data_option[:future] param true|false' if local_data_option[:future].nil?
+      raise 'Need data_option[:past] param true|false' if local_data_option[:past].nil?
+      raise 'Need data_option[:diff] param in hours' if local_data_option[:diff].nil?
     end
 
     if data_type == 'date'
-      raise 'Need data_option[:future] param true|false' if data_option[:future].nil?
-      raise 'Need data_option[:past] param true|false' if data_option[:past].nil?
-      raise 'Need data_option[:diff] param in days' if data_option[:diff].nil?
+      raise 'Need data_option[:future] param true|false' if local_data_option[:future].nil?
+      raise 'Need data_option[:past] param true|false' if local_data_option[:past].nil?
+      raise 'Need data_option[:diff] param in days' if local_data_option[:diff].nil?
     end
 
     true
   end
 
+  def verify_possible_type_change
+    return true if changes_to_save['data_type'].blank?
+
+    possible = {
+      'select' => %w[tree_select select input checkbox],
+      'tree_select' => %w[tree_select select input checkbox],
+      'checkbox' => %w[tree_select select input checkbox],
+      'input' => %w[tree_select select input checkbox],
+    }
+
+    return true if possible[changes_to_save['data_type'][0]]&.include?(changes_to_save['data_type'][1])
+
+    raise 'Can\'t be changed data_type of attribute. Drop the attribute and recreate it with new data_type.'
+  end
 end

app/models/observer/sla/ticket_rebuild_escalation.rb

@@ -3,15 +3,7 @@
 class Observer::Sla::TicketRebuildEscalation < ActiveRecord::Observer
   observe 'sla', 'calendar'
 
-  def after_create(record)
-    _rebuild(record)
-  end
-
-  def after_update(record)
-    _check(record)
-  end
-
-  def after_delete(record)
+  def after_commit(record)
     _rebuild(record)
   end
 

app/models/observer/ticket/article/fillup_from_email.rb

@@ -24,7 +24,7 @@ class Observer::Ticket::Article::FillupFromEmail < ActiveRecord::Observer
     return true if type['name'] != 'email'
 
     # set subject if empty
-    ticket = Ticket.lookup(id: record.ticket_id)
+    ticket = record.ticket
     if !record.subject || record.subject == ''
       record.subject = ticket.title
     end
@@ -44,7 +44,7 @@ class Observer::Ticket::Article::FillupFromEmail < ActiveRecord::Observer
     # set sender
     email_address = ticket.group.email_address
     if !email_address
-      raise "No email address found for group '#{ticket.group.name}'"
+      raise "No email address found for group '#{ticket.group.name}' (#{ticket.group_id})"
     end
 
     # remember email address for background job

app/models/observer/ticket/article_changes.rb

@@ -27,7 +27,7 @@ class Observer::Ticket::ArticleChanges < ActiveRecord::Observer
       record.ticket.touch # rubocop:disable Rails/SkipsModelValidations
       return
     end
-    record.ticket.save
+    record.ticket.save!
   end
 
   def after_destroy(record)

app/models/observer/ticket/escalation_update.rb

@@ -3,11 +3,7 @@
 class Observer::Ticket::EscalationUpdate < ActiveRecord::Observer
   observe 'ticket'
 
-  def after_create(record)
-    _check(record)
-  end
-
-  def after_update(record)
+  def after_commit(record)
     _check(record)
   end
 
@@ -16,10 +12,10 @@ class Observer::Ticket::EscalationUpdate < ActiveRecord::Observer
   def _check(record)
 
     # return if we run import mode
-    return false if Setting.get('import_mode')
-
-    return false if !record.saved_changes?
+    return true if Setting.get('import_mode')
 
+    return true if !Ticket.exists?(record.id)
+    record.reload
     record.escalation_calculation
   end
 end

app/models/observer/ticket/reset_new_state.rb

@@ -6,10 +6,10 @@ class Observer::Ticket::ResetNewState < ActiveRecord::Observer
   def after_create(record)
 
     # return if we run import mode
-    return if Setting.get('import_mode')
+    return true if Setting.get('import_mode')
 
     # only change state if not processed via postmaster
-    return if ApplicationHandleInfo.postmaster?
+    return true if ApplicationHandleInfo.postmaster?
 
     # if article in internal
     return true if record.internal
@@ -21,12 +21,13 @@ class Observer::Ticket::ResetNewState < ActiveRecord::Observer
     return true if !Ticket::Article::Type.lookup(id: record.type_id).communication
 
     # if current ticket state is still new
-    ticket = Ticket.lookup(id: record.ticket_id)
+    ticket = Ticket.find_by(id: record.ticket_id)
+    return true if !ticket
     new_state = Ticket::State.find_by(default_create: true)
     return true if ticket.state_id != new_state.id
 
     state = Ticket::State.find_by(default_follow_up: true)
-    return if !state
+    return true if !state
 
     # set ticket to open
     ticket.state_id = state.id

app/models/observer/ticket/user_ticket_counter.rb

@@ -3,20 +3,18 @@
 class Observer::Ticket::UserTicketCounter < ActiveRecord::Observer
   observe 'ticket'
 
-  def after_create(record)
-    user_ticket_counter_update(record)
-  end
-
-  def after_update(record)
+  def after_commit(record)
     user_ticket_counter_update(record)
   end
 
   def user_ticket_counter_update(record)
 
     # return if we run import mode
-    return if Setting.get('import_mode')
+    return true if Setting.get('import_mode')
 
-    return if !record.customer_id
+    return true if record.destroyed?
+
+    return true if !record.customer_id
 
     # send background job
     Delayed::Job.enqueue(

app/models/observer/ticket/user_ticket_counter/background_job.rb

@@ -26,6 +26,7 @@ class Observer::Ticket::UserTicketCounter::BackgroundJob
 
     # check if update is needed
     customer = User.lookup(id: @customer_id)
+    return true if !customer
     need_update = false
     if customer[:preferences][:tickets_open] != tickets_open
       need_update = true
@@ -36,7 +37,7 @@ class Observer::Ticket::UserTicketCounter::BackgroundJob
       customer[:preferences][:tickets_closed] = tickets_closed
     end
 
-    return if !need_update
+    return true if !need_update
     customer.updated_by_id = @updated_by_id
     customer.save
   end

app/models/role.rb

@@ -16,6 +16,8 @@ class Role < ApplicationModel
   before_create  :validate_permissions, :check_default_at_signup_permissions
   before_update  :validate_permissions, :last_admin_check_by_attribute, :validate_agent_limit_by_attributes, :check_default_at_signup_permissions
 
+  # ignore Users because this will lead to huge
+  # results for e.g. the Customer role
   association_attributes_ignored :users
 
   activity_stream_permission 'admin.role'

app/models/role_group.rb

@@ -1,13 +1,10 @@
 # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/
 
 class RoleGroup < ApplicationModel
-  self.table_name   = 'roles_groups'
-  self.primary_keys = :role_id, :group_id, :access
-  belongs_to          :role
-  belongs_to          :group
-  validates           :access, presence: true
+  include HasGroupRelationDefinition
 
-  def self.ref_key
-    :role_id
-  end
+  self.table_name = 'roles_groups'
+
+  # don't list roles in Group association result
+  Group.association_attributes_ignored :roles
 end

app/models/text_module.rb

@@ -13,6 +13,8 @@ class TextModule < ApplicationModel
 
   sanitized_html :content
 
+  csv_delete_possible true
+
 =begin
 
 load text modules from online

app/models/ticket.rb

@@ -557,11 +557,11 @@ condition example
 
       # get attributes
       attributes = attribute.split(/\./)
-      attribute = "#{attributes[0]}s.#{attributes[1]}"
+      attribute = "#{ActiveRecord::Base.connection.quote_table_name("#{attributes[0]}s")}.#{ActiveRecord::Base.connection.quote_column_name(attributes[1])}"
 
       # magic selectors
       if attributes[0] == 'ticket' && attributes[1] == 'out_of_office_replacement_id'
-        attribute = "#{attributes[0]}s.owner_id"
+        attribute = "#{ActiveRecord::Base.connection.quote_table_name("#{attributes[0]}s")}.#{ActiveRecord::Base.connection.quote_column_name('owner_id')}"
       end
 
       if attributes[0] == 'ticket' && attributes[1] == 'tags'
@@ -575,7 +575,7 @@ condition example
       if selector['operator'] == 'is'
         if selector['pre_condition'] == 'not_set'
           if attributes[1].match?(/^(created_by|updated_by|owner|customer|user)_id/)
-            query += "#{attribute} IN (?)"
+            query += "(#{attribute} IS NULL OR #{attribute} IN (?))"
             bind_params.push 1
           else
             query += "#{attribute} IS NULL"
@@ -610,20 +610,20 @@ condition example
       elsif selector['operator'] == 'is not'
         if selector['pre_condition'] == 'not_set'
           if attributes[1].match?(/^(created_by|updated_by|owner|customer|user)_id/)
-            query += "#{attribute} NOT IN (?)"
+            query += "(#{attribute} IS NOT NULL AND #{attribute} NOT IN (?))"
             bind_params.push 1
           else
             query += "#{attribute} IS NOT NULL"
           end
         elsif selector['pre_condition'] == 'current_user.id'
-          query += "#{attribute} NOT IN (?)"
+          query += "(#{attribute} IS NULL OR #{attribute} NOT IN (?))"
           if attributes[1] == 'out_of_office_replacement_id'
             bind_params.push User.find(current_user_id).out_of_office_agent_of.pluck(:id)
           else
             bind_params.push current_user_id
           end
         elsif selector['pre_condition'] == 'current_user.organization_id'
-          query += "#{attribute} NOT IN (?)"
+          query += "(#{attribute} IS NULL OR #{attribute} NOT IN (?))"
           user = User.find_by(id: current_user_id)
           bind_params.push user.organization_id
         else
@@ -631,7 +631,7 @@ condition example
           if selector['value'].nil?
             query += "#{attribute} IS NOT NULL"
           else
-            query += "#{attribute} NOT IN (?)"
+            query += "(#{attribute} IS NULL OR #{attribute} NOT IN (?))"
             if attributes[1] == 'out_of_office_replacement_id'
               bind_params.push User.find(selector['value']).out_of_office_agent_of.pluck(:id)
             else
@@ -824,219 +824,15 @@ perform changes on ticket
       end
     end
 
+    perform_notification = {}
     changed = false
     perform.each do |key, value|
       (object_name, attribute) = key.split('.', 2)
       raise "Unable to update object #{object_name}.#{attribute}, only can update tickets and send notifications!" if object_name != 'ticket' && object_name != 'notification'
 
-      # send notification
+      # send notification (after changes are done)
       if object_name == 'notification'
-
-        # value['recipient'] was a string in the past (single-select) so we convert it to array if needed
-        value_recipient = value['recipient']
-        if !value_recipient.is_a?(Array)
-          value_recipient = [value_recipient]
-        end
-
-        recipients_raw = []
-        value_recipient.each do |recipient|
-          if recipient == 'article_last_sender'
-            if article.present?
-              if article.reply_to.present?
-                recipients_raw.push(article.reply_to)
-              elsif article.from.present?
-                recipients_raw.push(article.from)
-              elsif article.origin_by_id
-                email = User.find_by(id: article.origin_by_id).email
-                recipients_raw.push(email)
-              elsif article.created_by_id
-                email = User.find_by(id: article.created_by_id).email
-                recipients_raw.push(email)
-              end
-            end
-          elsif recipient == 'ticket_customer'
-            email = User.find_by(id: customer_id).email
-            recipients_raw.push(email)
-          elsif recipient == 'ticket_owner'
-            email = User.find_by(id: owner_id).email
-            recipients_raw.push(email)
-          elsif recipient == 'ticket_agents'
-            User.group_access(group_id, 'full').sort_by(&:login).each do |user|
-              recipients_raw.push(user.email)
-            end
-          else
-            logger.error "Unknown email notification recipient '#{recipient}'"
-            next
-          end
-        end
-
-        recipients_checked = []
-        recipients_raw.each do |recipient_email|
-
-          skip_user = false
-          users = User.where(email: recipient_email)
-          users.each do |user|
-            next if user.preferences[:mail_delivery_failed] != true
-            next if !user.preferences[:mail_delivery_failed_data]
-            till_blocked = ((user.preferences[:mail_delivery_failed_data] - Time.zone.now - 60.days) / 60 / 60 / 24).round
-            next if till_blocked.positive?
-            logger.info "Send no trigger based notification to #{recipient_email} because email is marked as mail_delivery_failed for #{till_blocked} days"
-            skip_user = true
-            break
-          end
-          next if skip_user
-
-          # send notifications only to email adresses
-          next if recipient_email.blank?
-          next if recipient_email !~ /@/
-
-          # check if address is valid
-          begin
-            Mail::AddressList.new(recipient_email).addresses.each do |address|
-              recipient_email = address.address
-              break if recipient_email.present? && recipient_email =~ /@/ && !recipient_email.match?(/\s/)
-            end
-          rescue
-            if recipient_email.present?
-              if recipient_email !~ /^(.+?)<(.+?)@(.+?)>$/
-                next # no usable format found
-              end
-              recipient_email = "#{$2}@#{$3}"
-            end
-            next if recipient_email.blank?
-            next if recipient_email !~ /@/
-            next if recipient_email.match?(/\s/)
-          end
-
-          # do not sent notifications to this recipients
-          send_no_auto_response_reg_exp = Setting.get('send_no_auto_response_reg_exp')
-          begin
-            next if recipient_email.match?(/#{send_no_auto_response_reg_exp}/i)
-          rescue => e
-            logger.error "ERROR: Invalid regex '#{send_no_auto_response_reg_exp}' in setting send_no_auto_response_reg_exp"
-            logger.error 'ERROR: ' + e.inspect
-            next if recipient_email.match?(/(mailer-daemon|postmaster|abuse|root|noreply|noreply.+?|no-reply|no-reply.+?)@.+?/i)
-          end
-
-          # check if notification should be send because of customer emails
-          if article.present? && article.preferences.fetch('is-auto-response', false) == true && article.from && article.from =~ /#{Regexp.quote(recipient_email)}/i
-            logger.info "Send no trigger based notification to #{recipient_email} because of auto response tagged incoming email"
-            next
-          end
-
-          # loop protection / check if maximal count of trigger mail has reached
-          map = {
-            10 => 10,
-            30 => 15,
-            60 => 25,
-            180 => 50,
-            600 => 100,
-          }
-          skip = false
-          map.each do |minutes, count|
-            already_sent = Ticket::Article.where(
-              ticket_id: id,
-              sender: Ticket::Article::Sender.find_by(name: 'System'),
-              type: Ticket::Article::Type.find_by(name: 'email'),
-            ).where('ticket_articles.created_at > ? AND ticket_articles.to LIKE ?', Time.zone.now - minutes.minutes, "%#{recipient_email.strip}%").count
-            next if already_sent < count
-            logger.info "Send no trigger based notification to #{recipient_email} because already sent #{count} for this ticket within last #{minutes} minutes (loop protection)"
-            skip = true
-            break
-          end
-          next if skip
-          map = {
-            10 => 30,
-            30 => 60,
-            60 => 120,
-            180 => 240,
-            600 => 360,
-          }
-          skip = false
-          map.each do |minutes, count|
-            already_sent = Ticket::Article.where(
-              sender: Ticket::Article::Sender.find_by(name: 'System'),
-              type: Ticket::Article::Type.find_by(name: 'email'),
-            ).where('ticket_articles.created_at > ? AND ticket_articles.to LIKE ?', Time.zone.now - minutes.minutes, "%#{recipient_email.strip}%").count
-            next if already_sent < count
-            logger.info "Send no trigger based notification to #{recipient_email} because already sent #{count} in total within last #{minutes} minutes (loop protection)"
-            skip = true
-            break
-          end
-          next if skip
-
-          email = recipient_email.downcase.strip
-          next if recipients_checked.include?(email)
-          recipients_checked.push(email)
-        end
-
-        next if recipients_checked.blank?
-        recipient_string = recipients_checked.join(', ')
-
-        group = self.group
-        next if !group
-        email_address = group.email_address
-        if !email_address
-          logger.info "Unable to send trigger based notification to #{recipient_string} because no email address is set for group '#{group.name}'"
-          next
-        end
-
-        if !email_address.channel_id
-          logger.info "Unable to send trigger based notification to #{recipient_string} because no channel is set for email address '#{email_address.email}' (id: #{email_address.id})"
-          next
-        end
-
-        # articles.last breaks (returns the wrong article)
-        # if another email notification trigger preceded this one
-        # (see https://github.com/zammad/zammad/issues/1543)
-        objects = {
-          ticket: self,
-          article: article || articles.last
-        }
-
-        # get subject
-        subject = NotificationFactory::Mailer.template(
-          templateInline: value['subject'],
-          locale: 'en-en',
-          objects: objects,
-          quote: false,
-        )
-        subject = subject_build(subject)
-
-        body = NotificationFactory::Mailer.template(
-          templateInline: value['body'],
-          locale: 'en-en',
-          objects: objects,
-          quote: true,
-        )
-
-        (body, attachments_inline) = HtmlSanitizer.replace_inline_images(body, id)
-
-        message = Ticket::Article.create(
-          ticket_id: id,
-          to: recipient_string,
-          subject: subject,
-          content_type: 'text/html',
-          body: body,
-          internal: false,
-          sender: Ticket::Article::Sender.find_by(name: 'System'),
-          type: Ticket::Article::Type.find_by(name: 'email'),
-          preferences: {
-            perform_origin: perform_origin,
-          },
-          updated_by_id: 1,
-          created_by_id: 1,
-        )
-
-        attachments_inline.each do |attachment|
-          Store.add(
-            object: 'Ticket::Article',
-            o_id: message.id,
-            data: attachment[:data],
-            filename: attachment[:filename],
-            preferences: attachment[:preferences],
-          )
-        end
+        perform_notification[key] = value
         next
       end
 
@@ -1081,10 +877,227 @@ perform changes on ticket
       changed = true
 
       self[attribute] = value['value']
-      logger.debug { "set #{object_name}.#{attribute} = #{value['value'].inspect}" }
+      logger.debug { "set #{object_name}.#{attribute} = #{value['value'].inspect} for ticket_id #{id}" }
+    end
+
+    if changed
+      save!
+    end
+
+    perform_notification.each do |key, value|
+      perform_changes_notification(key, value, perform_origin, article)
+    end
+
+    true
+  end
+
+  def perform_changes_notification(_key, value, perform_origin, article)
+
+    # value['recipient'] was a string in the past (single-select) so we convert it to array if needed
+    value_recipient = value['recipient']
+    if !value_recipient.is_a?(Array)
+      value_recipient = [value_recipient]
+    end
+
+    recipients_raw = []
+    value_recipient.each do |recipient|
+      if recipient == 'article_last_sender'
+        if article.present?
+          if article.reply_to.present?
+            recipients_raw.push(article.reply_to)
+          elsif article.from.present?
+            recipients_raw.push(article.from)
+          elsif article.origin_by_id
+            email = User.find_by(id: article.origin_by_id).email
+            recipients_raw.push(email)
+          elsif article.created_by_id
+            email = User.find_by(id: article.created_by_id).email
+            recipients_raw.push(email)
+          end
+        end
+      elsif recipient == 'ticket_customer'
+        email = User.find_by(id: customer_id).email
+        recipients_raw.push(email)
+      elsif recipient == 'ticket_owner'
+        email = User.find_by(id: owner_id).email
+        recipients_raw.push(email)
+      elsif recipient == 'ticket_agents'
+        User.group_access(group_id, 'full').sort_by(&:login).each do |user|
+          recipients_raw.push(user.email)
+        end
+      else
+        logger.error "Unknown email notification recipient '#{recipient}'"
+        next
+      end
+    end
+
+    recipients_checked = []
+    recipients_raw.each do |recipient_email|
+
+      skip_user = false
+      users = User.where(email: recipient_email)
+      users.each do |user|
+        next if user.preferences[:mail_delivery_failed] != true
+        next if !user.preferences[:mail_delivery_failed_data]
+        till_blocked = ((user.preferences[:mail_delivery_failed_data] - Time.zone.now - 60.days) / 60 / 60 / 24).round
+        next if till_blocked.positive?
+        logger.info "Send no trigger based notification to #{recipient_email} because email is marked as mail_delivery_failed for #{till_blocked} days"
+        skip_user = true
+        break
+      end
+      next if skip_user
+
+      # send notifications only to email adresses
+      next if recipient_email.blank?
+      next if recipient_email !~ /@/
+
+      # check if address is valid
+      begin
+        Mail::AddressList.new(recipient_email).addresses.each do |address|
+          recipient_email = address.address
+          break if recipient_email.present? && recipient_email =~ /@/ && !recipient_email.match?(/\s/)
+        end
+      rescue
+        if recipient_email.present?
+          if recipient_email !~ /^(.+?)<(.+?)@(.+?)>$/
+            next # no usable format found
+          end
+          recipient_email = "#{$2}@#{$3}"
+        end
+        next if recipient_email.blank?
+        next if recipient_email !~ /@/
+        next if recipient_email.match?(/\s/)
+      end
+
+      # do not sent notifications to this recipients
+      send_no_auto_response_reg_exp = Setting.get('send_no_auto_response_reg_exp')
+      begin
+        next if recipient_email.match?(/#{send_no_auto_response_reg_exp}/i)
+      rescue => e
+        logger.error "ERROR: Invalid regex '#{send_no_auto_response_reg_exp}' in setting send_no_auto_response_reg_exp"
+        logger.error 'ERROR: ' + e.inspect
+        next if recipient_email.match?(/(mailer-daemon|postmaster|abuse|root|noreply|noreply.+?|no-reply|no-reply.+?)@.+?/i)
+      end
+
+      # check if notification should be send because of customer emails
+      if article.present? && article.preferences.fetch('is-auto-response', false) == true && article.from && article.from =~ /#{Regexp.quote(recipient_email)}/i
+        logger.info "Send no trigger based notification to #{recipient_email} because of auto response tagged incoming email"
+        next
+      end
+
+      # loop protection / check if maximal count of trigger mail has reached
+      map = {
+        10 => 10,
+        30 => 15,
+        60 => 25,
+        180 => 50,
+        600 => 100,
+      }
+      skip = false
+      map.each do |minutes, count|
+        already_sent = Ticket::Article.where(
+          ticket_id: id,
+          sender: Ticket::Article::Sender.find_by(name: 'System'),
+          type: Ticket::Article::Type.find_by(name: 'email'),
+        ).where('ticket_articles.created_at > ? AND ticket_articles.to LIKE ?', Time.zone.now - minutes.minutes, "%#{recipient_email.strip}%").count
+        next if already_sent < count
+        logger.info "Send no trigger based notification to #{recipient_email} because already sent #{count} for this ticket within last #{minutes} minutes (loop protection)"
+        skip = true
+        break
+      end
+      next if skip
+      map = {
+        10 => 30,
+        30 => 60,
+        60 => 120,
+        180 => 240,
+        600 => 360,
+      }
+      skip = false
+      map.each do |minutes, count|
+        already_sent = Ticket::Article.where(
+          sender: Ticket::Article::Sender.find_by(name: 'System'),
+          type: Ticket::Article::Type.find_by(name: 'email'),
+        ).where('ticket_articles.created_at > ? AND ticket_articles.to LIKE ?', Time.zone.now - minutes.minutes, "%#{recipient_email.strip}%").count
+        next if already_sent < count
+        logger.info "Send no trigger based notification to #{recipient_email} because already sent #{count} in total within last #{minutes} minutes (loop protection)"
+        skip = true
+        break
+      end
+      next if skip
+
+      email = recipient_email.downcase.strip
+      next if recipients_checked.include?(email)
+      recipients_checked.push(email)
+    end
+
+    return if recipients_checked.blank?
+    recipient_string = recipients_checked.join(', ')
+
+    group_id = self.group_id
+    return if !group_id
+    email_address = Group.find(group_id).email_address
+    if !email_address
+      logger.info "Unable to send trigger based notification to #{recipient_string} because no email address is set for group '#{group.name}'"
+      return
+    end
+
+    if !email_address.channel_id
+      logger.info "Unable to send trigger based notification to #{recipient_string} because no channel is set for email address '#{email_address.email}' (id: #{email_address.id})"
+      return
+    end
+
+    # articles.last breaks (returns the wrong article)
+    # if another email notification trigger preceded this one
+    # (see https://github.com/zammad/zammad/issues/1543)
+    objects = {
+      ticket: self,
+      article: article || articles.last
+    }
+
+    # get subject
+    subject = NotificationFactory::Mailer.template(
+      templateInline: value['subject'],
+      locale: 'en-en',
+      objects: objects,
+      quote: false,
+    )
+    subject = subject_build(subject)
+
+    body = NotificationFactory::Mailer.template(
+      templateInline: value['body'],
+      locale: 'en-en',
+      objects: objects,
+      quote: true,
+    )
+
+    (body, attachments_inline) = HtmlSanitizer.replace_inline_images(body, id)
+
+    message = Ticket::Article.create(
+      ticket_id: id,
+      to: recipient_string,
+      subject: subject,
+      content_type: 'text/html',
+      body: body,
+      internal: false,
+      sender: Ticket::Article::Sender.find_by(name: 'System'),
+      type: Ticket::Article::Type.find_by(name: 'email'),
+      preferences: {
+        perform_origin: perform_origin,
+      },
+      updated_by_id: 1,
+      created_by_id: 1,
+    )
+
+    attachments_inline.each do |attachment|
+      Store.add(
+        object: 'Ticket::Article',
+        o_id: message.id,
+        data: attachment[:data],
+        filename: attachment[:filename],
+        preferences: attachment[:preferences],
+      )
     end
-    return if !changed
-    save
 
     true
   end
@@ -1136,6 +1149,8 @@ perform active triggers on ticket
 
     Transaction.execute(local_options) do
       triggers.each do |trigger|
+        logger.debug { "Probe trigger (#{trigger.name}/#{trigger.id}) for this object (Ticket:#{ticket.id}/Loop:#{local_options[:loop_count]})" }
+
         condition = trigger.condition
 
         # check if one article attribute is used

app/models/ticket/overviews.rb

@@ -47,6 +47,8 @@ returns
   {
     overview: {
       id: 123,
+      name: 'some name',
+      view: 'some_view',
       updated_at: ...,
     },
     count: 3,
@@ -99,7 +101,7 @@ returns
                      'created_at'
                    end
       end
-      order_by = "tickets.#{order_by}"
+      order_by = "#{ActiveRecord::Base.connection.quote_table_name('tickets')}.#{ActiveRecord::Base.connection.quote_column_name(order_by)}"
 
       # check if group by exists
       if overview.group_by.present?
@@ -110,7 +112,7 @@ returns
                      end
         end
         if group_by
-          order_by = "tickets.#{group_by}, #{order_by}"
+          order_by = "#{ActiveRecord::Base.connection.quote_table_name('tickets')}.#{ActiveRecord::Base.connection.quote_column_name(group_by)}, #{order_by}"
         end
       end
 

app/models/ticket/screen_options.rb

@@ -115,10 +115,13 @@ returns
       group_agent_roles_ids = Role.joins(', roles_groups').where("roles.id = roles_groups.role_id AND roles_groups.access = 'full' AND roles_groups.group_id = ? AND roles.id IN (?)", group.id, agent_role_ids).pluck(:id)
       group_agent_role_user_ids = User.joins(:roles).where(roles: { id: group_agent_roles_ids }).pluck(:id)
 
-      User.where(id: group_agent_user_ids.concat(group_agent_role_user_ids).uniq, active: true).each do |user|
-        dependencies[:group_id][group.id][:owner_id].push user.id
-        next if agents[user.id]
-        agents[user.id] = true
+      User.where(id: group_agent_user_ids.concat(group_agent_role_user_ids).uniq, active: true).pluck(:id).each do |user_id|
+        dependencies[:group_id][group.id][:owner_id].push user_id
+        next if agents[user_id]
+        agents[user_id] = true
+        next if assets[:User] && assets[:User][user_id]
+        user = User.lookup(id: user_id)
+        next if !user
         assets = user.assets(assets)
       end
 

app/models/user_group.rb

@@ -1,41 +1,7 @@
 # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/
 
 class UserGroup < ApplicationModel
-  self.table_name   = 'groups_users'
-  self.primary_keys = :user_id, :group_id, :access
-  belongs_to          :user
-  belongs_to          :group
-  validates           :access, presence: true
+  include HasGroupRelationDefinition
 
-  def self.ref_key
-    :user_id
-  end
-
-  def cache_update
-    group.cache_update(nil)
-    user.cache_update(nil)
-    super
-  end
-
-  def cache_delete
-    group.cache_update(nil)
-    user.cache_update(nil) if user.present?
-    super
-  end
-
-  private
-
-  def validate_access
-    query = self.class.where(group: group, user: user)
-
-    query = if access == 'full'
-              query.where.not(access: 'full')
-            else
-              query.where(access: 'full')
-            end
-
-    errors.add(:access, 'User can have full or granular access to group') if query.exists?
-  end
-
-  validate :validate_access
+  self.table_name = 'groups_users'
 end

app/views/tests/html_utils.html.erb

@@ -13,3 +13,5 @@ body {
 </script>
 
 <div id="qunit" class="u-dontfold"></div>
+
+<div id="image2text"></div>

config/initializers/db_preferences_mysql.rb

@@ -61,7 +61,7 @@ end
 
 if connection.execute("SHOW tables LIKE 'settings';").any? &&
    Setting.get('postmaster_max_size').present? &&
-   Setting.get('postmaster_max_size') > max_allowed_packet_mb
+   Setting.get('postmaster_max_size').to_i > max_allowed_packet_mb
   printf "\e[33m" # ANSI yellow
   puts <<~MSG
   Warning: Database config value 'max_allowed_packet' less than Zammad setting 'Maximum Email Size'

config/routes/auth.rb

@@ -3,6 +3,7 @@ Zammad::Application.routes.draw do
 
   # omniauth
   match '/auth/:provider/callback',         to: 'sessions#create_omniauth',      via: %i[post get puts delete]
+  match '/auth/failure',                    to: 'sessions#failure_omniauth',     via: %i[post get]
 
   # sso
   match '/auth/sso',                        to: 'sessions#create_sso',           via: %i[post get]

db/migrate/20180628000001_fixed_twitter_ticket_article_preferences5.rb

@@ -0,0 +1,36 @@
+class FixedTwitterTicketArticlePreferences5 < ActiveRecord::Migration[5.0]
+  def up
+
+    # return if it's a new setup
+    return if !Setting.find_by(name: 'system_init_done')
+
+    # find article preferences with Twitter::NullObject and replace it with nill to prevent elasticsearch index issue
+    article_type_ids = Ticket::Article::Type.where(name: ['twitter status', 'twitter direct-message']).pluck(:id)
+    article_ids = Ticket::Article.where(type_id: article_type_ids).pluck(:id)
+    article_ids.each do |article_id|
+      article = Ticket::Article.find(article_id)
+      next if !article.preferences
+      changed = false
+      article.preferences.each_value do |value|
+        next if value.class != ActiveSupport::HashWithIndifferentAccess
+        value.each do |sub_key, sub_level|
+          if sub_level.class == NilClass
+            value[sub_key] = nil
+            next
+          end
+          if sub_level.class == Twitter::Place || sub_level.class == Twitter::Geo
+            value[sub_key] = sub_level.attrs
+            changed = true
+            next
+          end
+          next if sub_level.class != Twitter::NullObject
+          value[sub_key] = nil
+          changed = true
+        end
+      end
+      next if !changed
+      article.save!
+    end
+
+  end
+end

db/migrate/20180716060129_issue_2019_fix_double_domain_links_in_trigger_emails.rb

@@ -0,0 +1,18 @@
+class Issue2019FixDoubleDomainLinksInTriggerEmails < ActiveRecord::Migration[5.1]
+  DOUBLE_DOMAIN_REGEX = %r{(?<=<a href=")https?://[^"]+(?=(https?|\#{config\.http_type})://.+?".*?>)}
+
+  def up
+    return if !Setting.find_by(name: 'system_init_done')
+
+    Trigger.where('perform LIKE ?', '%notification.email: %')
+           .find_each do |t|
+             email_response = t.perform['notification.email']
+             next if email_response.blank? || !email_response['body']&.match(DOUBLE_DOMAIN_REGEX)
+
+             email_response['body'] = email_response['body'].gsub(DOUBLE_DOMAIN_REGEX, '')
+             next if !t.perform_changed?
+
+             t.save
+           end
+  end
+end

db/migrate/20180719104748_add_confidential_to_doorkeeper_application.rb

@@ -0,0 +1,11 @@
+class AddConfidentialToDoorkeeperApplication < ActiveRecord::Migration[5.1]
+  def change
+    add_column(
+      :oauth_applications,
+      :confidential,
+      :boolean,
+      null: false,
+      default: true # maintaining backwards compatibility: require secrets
+    )
+  end
+end

lib/html_sanitizer.rb

@@ -1,4 +1,5 @@
 class HtmlSanitizer
+  LINKABLE_URL_SCHEMES = URI.scheme_list.keys.map(&:downcase) - ['mailto'] + ['tel']
 
 =begin
 
@@ -25,23 +26,14 @@ satinize html string based on whiltelist
 
     scrubber_link = Loofah::Scrubber.new do |node|
 
-      # check if text has urls which need to be clickable
-      if node&.name != 'a' && node.parent && node.parent.name != 'a' && (!node.parent.parent || node.parent.parent.name != 'a')
-        if node.class == Nokogiri::XML::Text
-          urls = []
-          node.content.scan(%r{((http|https|ftp|tel)://.+?)([[:space:]]|\.[[:space:]]|,[[:space:]]|\.$|,$|\)|\(|$)}mxi).each do |match|
-            if match[0]
-              urls.push match[0].to_s.strip
-            end
-          end
-          node.content.scan(/(^|:|;|\s)(www\..+?)([[:space:]]|\.[[:space:]]|,[[:space:]]|\.$|,$|\)|\(|$)/mxi).each do |match|
-            if match[1]
-              urls.push match[1].to_s.strip
-            end
-          end
-          next if urls.blank?
-          add_link(node.content, urls, node)
-        end
+      # wrap plain-text URLs in <a> tags
+      if node.is_a?(Nokogiri::XML::Text) && node.ancestors.map(&:name).exclude?('a')
+        urls = URI.extract(node.content, LINKABLE_URL_SCHEMES)
+                  .map { |u| u.sub(/[,.]$/, '') }      # URI::extract captures trailing dots/commas
+                  .reject { |u| u.match?(/^[^:]+:$/) } # URI::extract will match, e.g., 'tel:'
+
+        next if urls.blank?
+        add_link(node.content, urls, node)
       end
 
       # prepare links
@@ -400,7 +392,7 @@ cleanup html string:
                 end
 
     uri.to_s
-  rescue URI::InvalidURIError
+  rescue URI::Error
     url
   end
 
@@ -427,14 +419,20 @@ reolace inline images with cid images
 
   def self.replace_inline_images(string, prefix = rand(999_999_999))
     attachments_inline = []
+    filename_counter = 0
     scrubber = Loofah::Scrubber.new do |node|
       if node.name == 'img'
         if node['src'] && node['src'] =~ %r{^(data:image/(jpeg|png);base64,.+?)$}i
+          filename_counter += 1
           file_attributes = StaticAssets.data_url_attributes($1)
           cid = "#{prefix}.#{rand(999_999_999)}@#{Setting.get('fqdn')}"
+          filename = cid
+          if file_attributes[:file_extention].present?
+            filename = "image#{filename_counter}.#{file_attributes[:file_extention]}"
+          end
           attachment = {
             data: file_attributes[:content],
-            filename: cid,
+            filename: filename,
             preferences: {
               'Content-Type' => file_attributes[:mime_type],
               'Mime-Type' => file_attributes[:mime_type],
@@ -485,6 +483,7 @@ satinize style of img tags
   end
 
   private_class_method :cleanup_target
+  private_class_method :sanitize_attachment_disposition
   private_class_method :add_link
   private_class_method :url_same?
   private_class_method :html_decode

lib/search_index_backend.rb

@@ -332,18 +332,10 @@ return search result
     data['query']['bool'] ||= {}
     data['query']['bool']['must'] ||= []
 
-    # add * on simple query like "somephrase23" or "attribute: somephrase23"
-    if query.present?
-      query.strip!
-      if query.match?(/^([[:alpha:],0-9]+|[[:alpha:],0-9]+\:\s+[[:alpha:],0-9]+)$/)
-        query += '*'
-      end
-    end
-
     # real search condition
     condition = {
       'query_string' => {
-        'query' => query,
+        'query' => append_wildcard_to_simple_query(query),
         'default_operator' => 'AND',
       }
     }
@@ -489,25 +481,82 @@ get count of tickets and tickets which match on selector
   def self.selector2query(selector, _current_user, aggs_interval, limit)
     query_must = []
     query_must_not = []
+    relative_map = {
+      day: 'd',
+      year: 'y',
+      month: 'M',
+      hour: 'h',
+      minute: 'm',
+    }
     if selector.present?
       selector.each do |key, data|
         key_tmp = key.sub(/^.+?\./, '')
         t = {}
-        if data['value'].class == Array
-          t[:terms] = {}
-          t[:terms][key_tmp] = data['value']
+
+        # is/is not/contains/contains not
+        if data['operator'] == 'is' || data['operator'] == 'is not' || data['operator'] == 'contains' || data['operator'] == 'contains not'
+          if data['value'].class == Array
+            t[:terms] = {}
+            t[:terms][key_tmp] = data['value']
+          else
+            t[:term] = {}
+            t[:term][key_tmp] = data['value']
+          end
+
+        # within last/within next (relative)
+        elsif data['operator'] == 'within last (relative)' || data['operator'] == 'within next (relative)'
+          range = relative_map[data['range'].to_sym]
+          if range.blank?
+            raise "Invalid relative_map for range '#{data['range']}'."
+          end
+          t[:range] = {}
+          t[:range][key_tmp] = {}
+          if data['operator'] == 'within last (relative)'
+            t[:range][key_tmp][:gte] = "now-#{data['value']}#{range}"
+          else
+            t[:range][key_tmp][:lt] = "now+#{data['value']}#{range}"
+          end
+
+        # before/after (relative)
+        elsif data['operator'] == 'before (relative)' || data['operator'] == 'after (relative)'
+          range = relative_map[data['range'].to_sym]
+          if range.blank?
+            raise "Invalid relative_map for range '#{data['range']}'."
+          end
+          t[:range] = {}
+          t[:range][key_tmp] = {}
+          if data['operator'] == 'before (relative)'
+            t[:range][key_tmp][:lt] = "now-#{data['value']}#{range}"
+          else
+            t[:range][key_tmp][:gt] = "now+#{data['value']}#{range}"
+          end
+
+        # before/after (absolute)
+        elsif data['operator'] == 'before (absolute)' || data['operator'] == 'after (absolute)'
+          t[:range] = {}
+          t[:range][key_tmp] = {}
+          if data['operator'] == 'before (absolute)'
+            t[:range][key_tmp][:lt] = (data['value']).to_s
+          else
+            t[:range][key_tmp][:gt] = (data['value']).to_s
+          end
         else
-          t[:term] = {}
-          t[:term][key_tmp] = data['value']
+          raise "unknown operator '#{data['operator']}' for #{key}"
         end
         if data['operator'] == 'is'
           query_must.push t
         elsif data['operator'] == 'is not'
           query_must_not.push t
         elsif data['operator'] == 'contains'
-          query_must.push t
+          query_must_not.push t
         elsif data['operator'] == 'contains not'
           query_must_not.push t
+        elsif data['operator'] == 'within last (relative)' || data['operator'] == 'within next (relative)'
+          query_must.push t
+        elsif data['operator'] == 'before (relative)' || data['operator'] == 'after (relative)'
+          query_must.push t
+        elsif data['operator'] == 'before (absolute)' || data['operator'] == 'after (absolute)'
+          query_must.push t
         else
           raise "unknown operator '#{data['operator']}' for #{key}"
         end
@@ -616,4 +665,11 @@ return true if backend is configured
     Rails.logger.error result.first(40_000)
     result
   end
+
+  # add * on simple query like "somephrase23" or "attribute: somephrase23"
+  def self.append_wildcard_to_simple_query(query)
+    query.strip!
+    query += '*' if query.match?(/^([[:alnum:]._]+|[[:alnum:]]+\:\s*[[:alnum:]._]+)$/)
+    query
+  end
 end

lib/sequencer/unit/import/common/model/associations/assign.rb

@@ -15,40 +15,47 @@ class Sequencer
               def process
                 return if dry_run
                 return if instance.blank?
+                return if associations.blank? && log_associations_error
 
+                register_changes
                 instance.assign_attributes(associations)
-
-                # execute associations check only if needed for performance reasons
-                return if action != :unchanged
-                return if !changed?
-                state.provide(:action, :changed)
               rescue => e
                 handle_failure(e)
               end
 
               private
 
-              def changed?
+              # always returns true
+              def log_associations_error
+                return true if %i[failed deactivated].include?(action)
+                logger.error { 'associations cannot be nil' } if associations.nil?
+                true
+              end
+
+              def register_changes
+                return if !(action == :unchanged && changes.any?)
                 logger.debug { "Changed instance associations: #{changes.inspect}" }
-                changes.present?
+                state.provide(:action, :updated)
               end
 
+              # Why not just use instance.changes?
+              # Because it doesn't include associations
+              # stored on OTHER TABLES (has-one, has-many, HABTM)
               def changes
-                @changes ||= begin
-                  return {} if associations.blank?
-                  associations.collect do |association, value|
-                    before = compareable(instance.send(association))
-                    after  = compareable(value)
-                    next if before == after
-                    [association, [before, after]]
-                  end.compact.to_h.with_indifferent_access
-                end
+                @changes ||= unfiltered_changes.reject(&method(:no_diff?))
               end
 
-              def compareable(value)
-                return nil if value.blank?
-                return value.sort if value.respond_to?(:sort)
-                value
+              def unfiltered_changes
+                attrs  = associations.keys
+                before = attrs.map(&instance.method(:send))
+                after  = associations.values
+                attrs.zip(before.zip(after)).to_h.with_indifferent_access
+              end
+
+              def no_diff?(_, values)
+                values.map!(&:sort) if values.all? { |val| val.respond_to?(:sort) }
+                values.map!(&:presence) # [nil, []] -> [nil, nil]
+                values.uniq.length == 1
               end
             end
           end

lib/sequencer/unit/import/ldap/users/lost/deactivate.rb

@@ -10,16 +10,11 @@ class Sequencer
               def process
                 return if dry_run
 
-                # we need to update in slices since some DBs
-                # have a limit for IN length
-                lost_ids.each_slice(5000) do |slice|
-
-                  # we need to instanciate every entry and set
-                  # the active state this way to send notifications
-                  # to the client
-                  ::User.where(id: slice).each do |user|
-                    user.update!(active: false)
-                  end
+                # Why not use `#update_all`?
+                # It bypasses validations/callbacks
+                # (which are used to send notifications to the client)
+                ::User.where(id: lost_ids).find_each do |user|
+                  user.update!(active: false, updated_by_id: 1)
                 end
               end
             end

lib/sessions/backend/activity_stream.rb

@@ -1,5 +1,7 @@
 class Sessions::Backend::ActivityStream
 
+  attr_writer :user
+
   def initialize(user, asset_lookup, client = nil, client_id = nil, ttl = 25)
     @user         = user
     @client       = client
@@ -29,8 +31,13 @@ class Sessions::Backend::ActivityStream
     assets = {}
     item_ids = []
     activity_stream.each do |item|
+      begin
+        assets = item.assets(assets)
+      rescue ActiveRecord::RecordNotFound
+        next
+      end
+
       item_ids.push item.id
-      assets = item.assets(assets)
     end
 
     {

lib/sessions/backend/base.rb

@@ -1,5 +1,8 @@
 class Sessions::Backend::Base
 
+  attr_writer :user
+  attr_writer :time_now
+
   def initialize(user, asset_lookup, client, client_id, ttl = 10)
     @user         = user
     @client       = client
@@ -7,6 +10,7 @@ class Sessions::Backend::Base
     @ttl          = ttl
     @asset_lookup = asset_lookup
     @last_change  = nil
+    @time_now     = Time.zone.now.to_i
   end
 
   def asset_push(record, assets)
@@ -14,7 +18,7 @@ class Sessions::Backend::Base
     @asset_lookup[class_name] ||= {}
     @asset_lookup[class_name][record.id] = {
       updated_at: record.updated_at,
-      pushed_at: Time.zone.now,
+      pushed_at: @time_now,
     }
     record.assets(assets)
   end
@@ -30,7 +34,7 @@ class Sessions::Backend::Base
     return true if @asset_lookup[class_name][record_id].blank?
     return true if @asset_lookup[class_name][record_id][:updated_at] < updated_at
     return true if @asset_lookup[class_name][record_id][:pushed_at].blank?
-    return true if @asset_lookup[class_name][record_id][:pushed_at] < Time.zone.now - 2.hours
+    return true if @asset_lookup[class_name][record_id][:pushed_at] < @time_now - 7200
     false
   end
 

lib/sessions/backend/collections.rb

@@ -7,6 +7,7 @@ class Sessions::Backend::Collections < Sessions::Backend::Base
     @ttl          = ttl
     @asset_lookup = asset_lookup
     @backends     = backend
+    @time_now     = Time.zone.now.to_i
   end
 
   def push
@@ -22,6 +23,15 @@ class Sessions::Backend::Collections < Sessions::Backend::Base
     results
   end
 
+  def user=(user)
+    @user = user
+
+    # update stored user in backends, too
+    @backends.each do |backend|
+      backend.user = user
+    end
+  end
+
   def backend
 
     # auto population collections

lib/sessions/backend/collections/base.rb

@@ -1,6 +1,9 @@
 class Sessions::Backend::Collections::Base < Sessions::Backend::Base
   class << self; attr_accessor :model, :permissions end
 
+  attr_writer :user
+  attr_writer :time_now
+
   def initialize(user, asset_lookup, client, client_id, ttl)
     @user         = user
     @client       = client
@@ -8,6 +11,7 @@ class Sessions::Backend::Collections::Base < Sessions::Backend::Base
     @ttl          = ttl
     @asset_lookup = asset_lookup
     @last_change  = nil
+    @time_now     = Time.zone.now.to_i
   end
 
   def load
@@ -51,6 +55,7 @@ class Sessions::Backend::Collections::Base < Sessions::Backend::Base
     end
 
     # collect assets
+    @time_now = Time.zone.now.to_i
     assets = {}
     items.each do |item|
       next if !asset_needed?(item)

lib/sessions/backend/ticket_create.rb

@@ -1,7 +1,6 @@
 class Sessions::Backend::TicketCreate < Sessions::Backend::Base
 
   def load
-
     # get attributes to update
     ticket_create_attributes = Ticket::ScreenOptions.attributes_to_change(
       current_user: @user,

lib/sessions/backend/ticket_overview_list.rb

@@ -15,6 +15,7 @@ class Sessions::Backend::TicketOverviewList < Sessions::Backend::Base
     @last_overview        = {}
     @last_overview_change = nil
     @last_ticket_change   = nil
+    @time_now             = Time.zone.now.to_i
   end
 
   def load
@@ -87,6 +88,8 @@ class Sessions::Backend::TicketOverviewList < Sessions::Backend::Base
       )
     end
 
+    @time_now = Time.zone.now.to_i
+
     # push overviews
     results = []
     index_and_lists.each do |data|
@@ -114,7 +117,6 @@ class Sessions::Backend::TicketOverviewList < Sessions::Backend::Base
         }
         results.push result
       else
-
         @client.log "push overview_list #{overview.link} for user #{@user.id}"
 
         # send update to browser

lib/sessions/client.rb

@@ -17,10 +17,11 @@ class Sessions::Client
       'Sessions::Backend::TicketCreate',
     ]
 
-    asset_lookup = {}
-    backend_pool = []
-    user_id_last_run = nil
-    loop_count = 0
+    asset_lookup             = {}
+    backend_pool             = []
+    user_id_last_run         = nil
+    user_updated_at_last_run = nil
+    loop_count               = 0
     loop do
 
       # check if session still exists
@@ -50,6 +51,14 @@ class Sessions::Client
           item = backend.constantize.new(user, asset_lookup, self, @client_id)
           backend_pool.push item
         end
+      # update user if required
+      elsif user_updated_at_last_run != user.updated_at
+        user_updated_at_last_run = user.updated_at
+
+        log "---client - updating user #{user.id} - #{user_updated_at_last_run}"
+        backend_pool.each do |backend|
+          backend.user = user
+        end
       end
 
       loop_count += 1

lib/static_assets.rb

@@ -9,6 +9,7 @@ returns
   {
     mime_type: 'image/png',
     content: image_bin_content,
+    file_extention: 'png',
   }
 
 =end
@@ -18,6 +19,9 @@ returns
     if data_url =~ /^data:(.+?);base64,(.+?)$/
       data[:mime_type] = $1
       data[:content]   = Base64.decode64($2)
+      if data[:mime_type] =~ %r{/(.+?)$}
+        data[:file_extention] = $1
+      end
       return data
     end
     raise "Unable to parse data url: #{data_url.substr(0, 100)}"

lib/tweet_base.rb

@@ -373,7 +373,7 @@ class TweetBase
           value[sub_key] = nil
           next
         end
-        if sub_level.class == Twitter::Place
+        if sub_level.class == Twitter::Place || sub_level.class == Twitter::Geo
           value[sub_key] = sub_level.attrs
           next
         end

public/401.html

@@ -5,6 +5,9 @@
 <link rel="stylesheet" href="/assets/error/style.css">
 <body <% if @traceback %>class="error-message"<% end %>>
   <h1>401: Unauthorized</h1>
+<% if @message.present? %>
+  <div><%= @message %></div>
+<% end %>
 <% if !@traceback %>
   <div class="error-image" style="background-image: url(/assets/error/error-1.svg)"></div>
   <p>Sorry, but you're not allowed to access this page. If you're registered please log in and refresh this page.</p>

public/404.html

@@ -5,6 +5,9 @@
 <link rel="stylesheet" href="/assets/error/style.css">
 <body <% if @traceback %>class="error-message"<% end %>>
   <h1>404: Requested Ressource was not found.</h1>
+<% if @message.present? %>
+  <div><%= @message %></div>
+<% end %>
 <% if !@traceback %>
   <div class="error-image" style="background-image: url(/assets/error/error-2.svg)"></div>
   <p>Sorry, but the Phoenix is not able to find your ressource. Try checking the URL for errors.</p>

public/422.html

@@ -5,6 +5,9 @@
 <link rel="stylesheet" href="/assets/error/style.css">
 <body <% if @traceback %>class="error-message"<% end %>>
   <h1>422: The change you wanted was rejected.</h1>
+<% if @message.present? %>
+  <div><%= @message %></div>
+<% end %>
 <% if !@traceback %>
   <div class="error-image" style="background-image: url(/assets/error/error-1.svg)"></div>
   <p>Maybe you tried to change something you didn't have access to.</p>

public/assets/tests/html_utils.js

@@ -651,6 +651,12 @@ test("htmlCleanup", function() {
   result = App.Utils.htmlCleanup(source)
   equal(result.get(0).outerHTML, should, source)
 
+  // strip out browser-inserted (broken) link (see https://github.com/zammad/zammad/issues/2019)
+  source = "<div><a href=\"https://example.com/#{config.http_type}://#{config.fqdn}/#ticket/zoom/#{ticket.id}\">test</a></div>"
+  should = "<a href=\"#{config.http_type}://#{config.fqdn}/#ticket/zoom/#{ticket.id}\">test</a>"
+  result = App.Utils.htmlCleanup(source)
+  equal(result.html(), should, source)
+
   source = "<table bgcolor=\"green\" aaa=\"1\" style=\"color: red\"><thead><tr style=\"margin-top: 10px\"><th colspan=\"2\" abc=\"a\" style=\"margin-top: 12px\">aaa</th></tr></thead><tbody><tr><td>value</td></tr></tbody></table>"
   should = "<table bgcolor=\"green\" style=\"color:red;\"><thead><tr style=\"margin-top:10px;\"><th colspan=\"2\" style=\"margin-top:12px;\">aaa</th></tr></thead><tbody><tr><td>value</td></tr></tbody></table>"
   result = App.Utils.htmlCleanup(source)
@@ -2644,8 +2650,6 @@ test('check getRecipientArticle format', function() {
 
 });
 
-}
-
 test("contentTypeCleanup", function() {
 
   var source = "image/png"
@@ -2682,4 +2686,51 @@ test("contentTypeCleanup", function() {
   should = "image/gif"
   result = App.Utils.contentTypeCleanup(source)
   equal(result, should, source)
-});
+});
+
+// htmlImage2DataUrl
+test("htmlImage2DataUrl", function() {
+
+  var source = '<div>test 13</div>'
+  var should = '<div>test 13</div>'
+  var result = App.Utils.htmlImage2DataUrl(source)
+  equal(result, should, source)
+
+  source = 'some test'
+  should = 'some test'
+  result = App.Utils.htmlImage2DataUrl(source)
+  equal(result, should, source)
+
+  source = '<img src="some url">some test'
+  should = '<img src="data:,">some test'
+  result = App.Utils.htmlImage2DataUrl(source)
+  equal(result, should, source)
+
+  source = '<img src="data:image/jpeg;base64,/9j/4AAQSkZJRgABAQAAAQABAAD/2wBDAAEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQH/2wBDAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQH/wAARCAADAAEDAREAAhEBAxEB/8QAFAABAAAAAAAAAAAAAAAAAAAACv/EABQQAQAAAAAAAAAAAAAAAAAAAAD/xAAUAQEAAAAAAAAAAAAAAAAAAAAF/8QAFBEBAAAAAAAAAAAAAAAAAAAAAP/aAAwDAQACEQMRAD8AbgQDv//Z">some test'
+  should = '<img src="data:image/jpeg;base64,/9j/4AAQSkZJRgABAQAAAQABAAD/2wBDAAEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQH/2wBDAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQH/wAARCAADAAEDAREAAhEBAxEB/8QAFAABAAAAAAAAAAAAAAAAAAAACv/EABQQAQAAAAAAAAAAAAAAAAAAAAD/xAAUAQEAAAAAAAAAAAAAAAAAAAAF/8QAFBEBAAAAAAAAAAAAAAAAAAAAAP/aAAwDAQACEQMRAD8AbgQDv//Z">some test'
+  result = App.Utils.htmlImage2DataUrl(source)
+  equal(result, should, source)
+
+  source = '<img src="data:image/jpeg;base64,some_data_123">some <img src="some url">test'
+  should = '<img src="data:image/jpeg;base64,some_data_123">some <img src="data:,">test'
+  result = App.Utils.htmlImage2DataUrl(source)
+  equal(result, should, source)
+
+});
+
+source = '<img src="/assets/images/avatar-bg.png">some test'
+$('#image2text').html(source)
+var htmlImage2DataUrlTest = function() {
+
+  var result = App.Utils.htmlImage2DataUrl(source)
+  test("htmlImage2DataUrl async", function() {
+    var result = App.Utils.htmlImage2DataUrl(source)
+    ok(result.match(/some test/), source)
+    ok(!result.match(/avatar-bg.png/), source)
+    ok(result.match(/^\<img src=\"data:image\/png;base64,/), source)
+  });
+
+}
+$('#image2text img').one('load', htmlImage2DataUrlTest)
+
+}

script/build/test_slice_tests.sh

@@ -37,6 +37,7 @@ if [ "$LEVEL" == '1' ]; then
   rm test/browser/agent_ticket_macro_test.rb
   rm test/browser/agent_ticket_merge_test.rb
   rm test/browser/agent_ticket_online_notification_test.rb
+  rm test/browser/agent_ticket_overview_group_by_organization_test.rb
   rm test/browser/agent_ticket_overview_level0_test.rb
   rm test/browser/agent_ticket_overview_level1_test.rb
   rm test/browser/agent_ticket_overview_tab_test.rb
@@ -49,6 +50,7 @@ if [ "$LEVEL" == '1' ]; then
   rm test/browser/agent_ticket_update4_test.rb
   rm test/browser/agent_ticket_update5_test.rb
   rm test/browser/agent_ticket_update_and_reload_test.rb
+  rm test/browser/agent_ticket_zoom_hide_test.rb
   rm test/browser/agent_user_manage_test.rb
   rm test/browser/agent_user_profile_test.rb
   # test/browser/auth_test.rb
@@ -111,6 +113,7 @@ elif [ "$LEVEL" == '2' ]; then
   rm test/browser/agent_ticket_macro_test.rb
   # test/browser/agent_ticket_merge_test.rb
   rm test/browser/agent_ticket_online_notification_test.rb
+  rm test/browser/agent_ticket_overview_group_by_organization_test.rb
   rm test/browser/agent_ticket_overview_level0_test.rb
   rm test/browser/agent_ticket_overview_level1_test.rb
   rm test/browser/agent_ticket_overview_tab_test.rb
@@ -123,6 +126,7 @@ elif [ "$LEVEL" == '2' ]; then
   # test/browser/agent_ticket_update4_test.rb
   # rm test/browser/agent_ticket_update5_test.rb
   # test/browser/agent_ticket_update_and_reload_test.rb
+  # test/browser/agent_ticket_zoom_hide_test.rb
   rm test/browser/agent_user_manage_test.rb
   rm test/browser/agent_user_profile_test.rb
   rm test/browser/auth_test.rb
@@ -185,6 +189,7 @@ elif [ "$LEVEL" == '3' ]; then
   # test/browser/agent_ticket_macro_test.rb
   rm test/browser/agent_ticket_merge_test.rb
   rm test/browser/agent_ticket_online_notification_test.rb
+  rm test/browser/agent_ticket_overview_group_by_organization_test.rb
   rm test/browser/agent_ticket_overview_level0_test.rb
   rm test/browser/agent_ticket_overview_level1_test.rb
   rm test/browser/agent_ticket_overview_tab_test.rb
@@ -197,6 +202,7 @@ elif [ "$LEVEL" == '3' ]; then
   rm test/browser/agent_ticket_update4_test.rb
   rm test/browser/agent_ticket_update5_test.rb
   rm test/browser/agent_ticket_update_and_reload_test.rb
+  rm test/browser/agent_ticket_zoom_hide_test.rb
   rm test/browser/agent_user_manage_test.rb
   rm test/browser/agent_user_profile_test.rb
   rm test/browser/auth_test.rb
@@ -259,6 +265,7 @@ elif [ "$LEVEL" == '4' ]; then
   rm test/browser/agent_ticket_macro_test.rb
   rm test/browser/agent_ticket_merge_test.rb
   # test/browser/agent_ticket_online_notification_test.rb
+  # test/browser/agent_ticket_overview_group_by_organization_test.rb
   # test/browser/agent_ticket_overview_level0_test.rb
   # test/browser/agent_ticket_overview_level1_test.rb
   # test/browser/agent_ticket_overview_tab_test.rb
@@ -271,6 +278,7 @@ elif [ "$LEVEL" == '4' ]; then
   rm test/browser/agent_ticket_update4_test.rb
   rm test/browser/agent_ticket_update5_test.rb
   rm test/browser/agent_ticket_update_and_reload_test.rb
+  rm test/browser/agent_ticket_zoom_hide_test.rb
   rm test/browser/agent_user_manage_test.rb
   rm test/browser/agent_user_profile_test.rb
   rm test/browser/auth_test.rb
@@ -332,6 +340,7 @@ elif [ "$LEVEL" == '5' ]; then
   rm test/browser/agent_ticket_macro_test.rb
   rm test/browser/agent_ticket_merge_test.rb
   rm test/browser/agent_ticket_online_notification_test.rb
+  rm test/browser/agent_ticket_overview_group_by_organization_test.rb
   rm test/browser/agent_ticket_overview_level0_test.rb
   rm test/browser/agent_ticket_overview_level1_test.rb
   rm test/browser/agent_ticket_overview_tab_test.rb
@@ -344,6 +353,7 @@ elif [ "$LEVEL" == '5' ]; then
   rm test/browser/agent_ticket_update4_test.rb
   rm test/browser/agent_ticket_update5_test.rb
   rm test/browser/agent_ticket_update_and_reload_test.rb
+  rm test/browser/agent_ticket_zoom_hide_test.rb
   # test/browser/agent_user_manage_test.rb
   # test/browser/agent_user_profile_test.rb
   rm test/browser/auth_test.rb
@@ -408,6 +418,7 @@ elif [ "$LEVEL" == '6' ]; then
   rm test/browser/agent_ticket_macro_test.rb
   rm test/browser/agent_ticket_merge_test.rb
   rm test/browser/agent_ticket_online_notification_test.rb
+  rm test/browser/agent_ticket_overview_group_by_organization_test.rb
   rm test/browser/agent_ticket_overview_level0_test.rb
   rm test/browser/agent_ticket_overview_level1_test.rb
   rm test/browser/agent_ticket_overview_tab_test.rb
@@ -420,6 +431,7 @@ elif [ "$LEVEL" == '6' ]; then
   rm test/browser/agent_ticket_update4_test.rb
   rm test/browser/agent_ticket_update5_test.rb
   rm test/browser/agent_ticket_update_and_reload_test.rb
+  rm test/browser/agent_ticket_zoom_hide_test.rb
   rm test/browser/agent_user_manage_test.rb
   rm test/browser/agent_user_profile_test.rb
   rm test/browser/auth_test.rb

spec/controllers/integration/exchange_spec.rb

@@ -0,0 +1,46 @@
+require 'rails_helper'
+
+RSpec.describe 'Exchange integration endpoint', type: :request do
+  before { authenticated_as(admin_with_admin_user_permissions) }
+
+  let(:admin_with_admin_user_permissions) do
+    create(:user, roles: [role_with_admin_user_permissions])
+  end
+
+  let(:role_with_admin_user_permissions) do
+    create(:role).tap { |role| role.permission_grant('admin.integration') }
+  end
+
+  describe 'EWS folder retrieval' do
+    # see https://github.com/zammad/zammad/issues/1802
+    context 'when no folders found (#1802)' do
+      let(:empty_folder_list) { { folders: {} } }
+
+      it 'responds with an error message' do
+        allow(Sequencer).to receive(:process).with(any_args).and_return(empty_folder_list)
+
+        post api_v1_integration_exchange_folders_path,
+             params: {}, as: :json
+
+        expect(json_response).to include('result' => 'failed').and include('message')
+      end
+    end
+  end
+
+  describe 'autodiscovery' do
+    # see https://github.com/zammad/zammad/issues/2065
+    context 'when Autodiscover gem raises Errno::EADDRNOTAVAIL (#2065)' do
+      let(:client) { instance_double('Autodiscover::Client') }
+
+      it 'rescues and responds with an empty hash (to proceed to manual configuration)' do
+        allow(Autodiscover::Client).to receive(:new).with(any_args).and_return(client)
+        allow(client).to receive(:autodiscover).and_raise(Errno::EADDRNOTAVAIL)
+
+        post api_v1_integration_exchange_autodiscover_path,
+             params: {}, as: :json
+
+        expect(json_response).to eq('result' => 'ok')
+      end
+    end
+  end
+end

spec/db/migrate/check_for_object_attributes_spec.rb

@@ -41,33 +41,16 @@ RSpec.describe CheckForObjectAttributes, type: :db_migration do
     end
   end
 
-  context '[:data_option]' do
-
-    it 'ensures an empty Hash' do
-      attribute = create(:object_manager_attribute_text, data_option: nil)
-      migrate
-      attribute.reload
-
-      expect(attribute[:data_option]).to be_a(Hash)
-    end
-  end
-
   context '[:data_option][:options]' do
 
-    it 'ensures an empty Hash' do
-      attribute = create(:object_manager_attribute_text, data_option: {})
-      migrate
-      attribute.reload
-
-      expect(attribute[:data_option][:options]).to be_a(Hash)
-    end
-
     it 'converts String to Hash' do
       wrong = {
-        default:  '',
-        options:  '',
-        relation: '',
-        null:     true
+        default:   '',
+        options:   '',
+        relation:  '',
+        type:      'text',
+        maxlength: 255,
+        null:      true
       }
 
       attribute = create(:object_manager_attribute_text, data_option: wrong)
@@ -83,9 +66,11 @@ RSpec.describe CheckForObjectAttributes, type: :db_migration do
 
     it 'ensures an empty String' do
       wrong = {
-        default: '',
-        options: {},
-        null:    true
+        default:   '',
+        options:   {},
+        type:      'text',
+        maxlength: 255,
+        null:      true
       }
 
       attribute = create(:object_manager_attribute_text, data_option: wrong)
@@ -97,10 +82,12 @@ RSpec.describe CheckForObjectAttributes, type: :db_migration do
 
     it 'converts Hash to String' do
       wrong = {
-        default:  '',
-        options:  {},
-        relation: {},
-        null:     true
+        default:   '',
+        options:   {},
+        relation:  {},
+        type:      'text',
+        maxlength: 255,
+        null:      true
       }
 
       attribute = create(:object_manager_attribute_text, data_option: wrong)

spec/db/migrate/issue_1660_fix_tree_select_configurations_spec.rb

@@ -175,7 +175,7 @@ RSpec.describe Issue1660FixTreeSelectConfigurations, type: :db_migration do
       }
     ]
 
-    attribute = create(:object_manager_attribute_tree_select, data_option: { options: broken })
+    attribute = create(:object_manager_attribute_tree_select, data_option: { options: broken, null: true, default: '' })
 
     expect do
       migrate
@@ -192,18 +192,8 @@ RSpec.describe Issue1660FixTreeSelectConfigurations, type: :db_migration do
     end
   end
 
-  it 'skips blank data_option' do
-    attribute = create(:object_manager_attribute_tree_select, data_option: {})
-
-    expect do
-      migrate
-    end.not_to change {
-      attribute.reload.data_option
-    }
-  end
-
   it 'skips blank data_option options' do
-    attribute = create(:object_manager_attribute_tree_select, data_option: { options: [] })
+    attribute = create(:object_manager_attribute_tree_select, data_option: { options: [], null: true, default: '' })
 
     expect do
       migrate

spec/db/migrate/issue_2019_fix_double_domain_links_in_trigger_emails_spec.rb

@@ -0,0 +1,22 @@
+require 'rails_helper'
+
+RSpec.describe Issue2019FixDoubleDomainLinksInTriggerEmails, type: :db_migration do
+  subject { create(:trigger, perform: { 'notification.email' => { 'body' => faulty_link } }) }
+
+  let(:faulty_link) do
+    '<a href="https://example.com/#{config.http_type}://#{config.fqdn}/#ticket/zoom/#{ticket.id}">' \
+      'View ticket' \
+    '</a>'
+  end
+
+  let(:fixed_link) do
+    '<a href="#{config.http_type}://#{config.fqdn}/#ticket/zoom/#{ticket.id}">' \
+      'View ticket' \
+    '</a>'
+  end
+
+  it "removes duplicate domains from Trigger records' notification.email bodies" do
+    expect { migrate }.to change { subject.reload.perform['notification.email']['body'] }
+      .from(faulty_link).to(fixed_link)
+  end
+end

spec/factories/trigger.rb

@@ -0,0 +1,10 @@
+FactoryBot.define do
+  factory :trigger do
+    name          { "Test trigger #{rand(2**16)}" } # prevent unique name conflicts
+    condition     { { 'ticket.state_id' => { 'operator' => 'is not', 'value' => 4 } } }
+    perform       { { 'ticket.state_id' => { 'value' => 4 } } }
+    active        true
+    created_by_id 1
+    updated_by_id 1
+  end
+end

spec/lib/import/otrs/customer_user_spec.rb

@@ -108,10 +108,6 @@ RSpec.describe Import::OTRS::CustomerUser do
       travel_to DateTime.current
     end
 
-    after(:each) do
-      travel_back
-    end
-
     it 'creates' do
       creates_with(zammad_structure)
     end

spec/lib/sequencer/unit/import/common/model/associations/assign_spec.rb

@@ -0,0 +1,95 @@
+require 'rails_helper'
+
+RSpec.describe Sequencer::Unit::Import::Common::Model::Associations::Assign, sequencer: :unit do
+  let(:parameters) do
+    {
+      instance:     instance,
+      associations: associations,
+      action:       action,
+      dry_run:      false
+    }
+  end
+
+  context 'when given an `associations` hash that changes the instance' do
+    let(:instance)     { create(:user) }
+    let(:action)       { :created }
+    let(:associations) do
+      alt_org = Organization.where('id <> ?', instance.organization_id.to_i).pluck(:id).sample
+      { organization_id: alt_org }
+    end
+
+    it 'assigns (NOT updates) the associations' do
+      process(parameters)
+      expect(instance.changes).to include(:organization_id)
+    end
+
+    it 'changes `:action => :unchanged` to `:action => :updated`' do
+      parameters[:action] = :unchanged
+      expect(process(parameters)).to include(action: :updated)
+    end
+  end
+
+  context 'when given a `associations` hash that does NOT change the instance' do
+    let(:instance)     { create(:user) }
+    let(:associations) { { organization_id: instance.organization_id } }
+    let(:action) { :unchanged }
+
+    it 'keeps `:action => :unchanged`' do
+      expect(process(parameters)).to include(action: :unchanged)
+    end
+  end
+
+  context 'when given an empty `associations` hash' do
+    let(:instance)     { create(:user) }
+    let(:action)       { :created }
+    let(:associations) { {} }
+
+    it 'makes no changes' do
+      provided = process(parameters)
+
+      expect(provided).to include(action: action)
+      expect(instance.changed?).to be(false)
+    end
+  end
+
+  context 'when given nil for `associations`' do
+    let(:instance)     { create(:user) }
+    let(:associations) { nil }
+
+    context 'and `action == :failed`' do
+      let(:action) { :failed }
+
+      it 'makes no changes' do
+        provided = process(parameters)
+
+        expect(provided).to include(action: action)
+        expect(instance.changed?).to be(false)
+      end
+    end
+
+    context 'and `action == :deactivated`' do
+      let(:action) { :deactivated }
+
+      it 'makes no changes' do
+        provided = process(parameters)
+
+        expect(provided).to include(action: action)
+        expect(instance.changed?).to be(false)
+      end
+    end
+
+    context 'and any other value of `action`' do
+      let(:action) { :created }
+
+      it 'makes no changes and logs an error' do
+        allow(Rails.logger).to receive(:error).with(any_args)
+
+        provided = process(parameters)
+
+        expect(provided).to include(action: action)
+        expect(instance.changed?).to be(false)
+        expect(Rails.logger).to have_received(:error)
+      end
+    end
+  end
+end

spec/lib/sequencer/unit/import/ldap/users/lost/deactivate_spec.rb

@@ -0,0 +1,29 @@
+require 'rails_helper'
+
+RSpec.describe Sequencer::Unit::Import::Ldap::Users::Lost::Deactivate, sequencer: :unit do
+  let!(:lost_users) { create_list(:user, sample_length, attributes) }
+  let(:sample_length) { 2 }
+
+  context 'when provided ids of active users' do
+    let(:attributes) { { active: true } }
+
+    it 'deactivates them' do
+      expect { process(lost_ids: lost_users.pluck(:id), dry_run: false) }
+        .to change { lost_users.each(&:reload).pluck(:active) }.to(Array.new(sample_length, false))
+    end
+  end
+
+  context 'when provided ids of users with any `updated_by_id`' do
+    # ordinarily, a History log's created_by_id is based on this value (or UserInfo.current_user_id),
+    # but this Sequencer unit is expected to override it
+    let(:attributes) { { updated_by_id: 2 } }
+
+    it 'enforces created_by_id => 1 in newly created History logs' do
+      expect { process(lost_ids: lost_users.pluck(:id), dry_run: false) }
+        .to change { History.count }.by(sample_length)
+
+      expect(History.last(sample_length).pluck(:created_by_id))
+        .to eq(Array.new(sample_length, 1))
+    end
+  end
+end

spec/lib/sessions/backend/activity_stream_spec.rb

@@ -0,0 +1,24 @@
+require 'rails_helper'
+
+RSpec.describe Sessions::Backend::ActivityStream do
+  context 'when async processes affect associated objects / DB records (#2066)' do
+    let(:subject)            { described_class.new(user, {}) }
+    let(:user)               { create(:agent_user, groups: [group]) }
+    let(:group)              { Group.find_by(name: 'Users') }
+    let(:associated_tickets) { create_list(:ticket, ticket_count, group: group) }
+    let(:ticket_count)       { 20 }
+
+    before do
+      Setting.set('system_init_done', true)
+
+      # these records must be created before the example begins
+      # (same as `let!`, but harder to miss)
+      associated_tickets
+    end
+
+    it 'manages race condition' do
+      Thread.new { associated_tickets.each(&:destroy) }
+      expect { subject.load }.not_to raise_error
+    end
+  end
+end

spec/models/concerns/has_group_relation_definition_examples.rb

@@ -0,0 +1,62 @@
+# Requires: let!(:group_relation_instance) { ... }
+RSpec.shared_examples 'HasGroupRelationDefinition' do
+
+  let(:group_relation_model_key) { group_relation_instance.model_name.element }
+
+  context 'relation creation' do
+
+    it 'refreshes updated_at of related instances' do
+      group = create(:group)
+
+      travel 1.minute
+
+      expect do
+        described_class.create!(
+          group_relation_model_key => group_relation_instance,
+          group: group
+        )
+      end.to change {
+        group.reload.updated_at
+      }.and change {
+        group_relation_instance.reload.updated_at
+      }
+    end
+  end
+
+  context 'related instance deletion' do
+
+    it 'refreshes updated_at of group instance' do
+      group = create(:group)
+
+      described_class.create!(
+        group_relation_model_key => group_relation_instance,
+        group: group
+      )
+
+      travel 1.minute
+
+      expect do
+        group.destroy
+      end.to change {
+        group_relation_instance.reload.updated_at
+      }
+    end
+
+    it 'refreshes updated_at of relation instance' do
+      group = create(:group)
+
+      described_class.create!(
+        group_relation_model_key => group_relation_instance,
+        group: group
+      )
+
+      travel 1.minute
+
+      expect do
+        group_relation_instance.destroy
+      end.to change {
+        group.reload.updated_at
+      }
+    end
+  end
+end

spec/models/concerns/has_groups_examples.rb

@@ -242,6 +242,24 @@ RSpec.shared_examples 'HasGroups' do
             described_class.group_through.klass.count
           }.by(-3)
         end
+
+        it 'prevents having full and other privilege at the same time' do
+
+          invalid_combination = %w[full read change]
+          exception           = ActiveRecord::RecordInvalid
+
+          expect do
+            group_access_instance.group_names_access_map = {
+              group_full.name => invalid_combination,
+            }
+          end.to raise_error(exception)
+
+          expect do
+            group_access_instance.group_names_access_map = {
+              group_full.name => invalid_combination.reverse,
+            }
+          end.to raise_error(exception)
+        end
       end
 
       context 'new instance' do

spec/models/role_group_spec.rb

@@ -0,0 +1,21 @@
+require 'rails_helper'
+require 'models/concerns/has_group_relation_definition_examples'
+
+RSpec.describe RoleGroup do
+
+  let!(:group_relation_instance) { create(:role) }
+
+  include_examples 'HasGroupRelationDefinition'
+
+  it 'prevents roles from beeing in Group assets' do
+
+    group = create(:group)
+
+    described_class.create!(
+      group: group,
+      role:  create(:role)
+    )
+    expect(group.assets({})[:Group][group.id]).not_to include('role_ids')
+  end
+
+end