Followup 12e75243 - Improve Checklist handling

Co-authored-by: Mantas Masalskis <mm@zammad.com>
Co-authored-by: Benjamin Scharf <bs@zammad.com>

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

@@ -57,11 +57,10 @@ class SidebarChecklist extends App.Controller
     }
 
   subscribe: =>
-    @subscriptions ||= []
-
     @unsubscribe()
 
-    # ticket subscriptions
+    # This is reusing ticket subscription which is used in ZoomTitle and ZoomMeta
+    # Double network call happens because TicketZoom has a duplicate request with all=true
     sid = App.Ticket.subscribeItem(
       @ticket.id,
       (ticket) =>
@@ -69,7 +68,7 @@ class SidebarChecklist extends App.Controller
           @badgeRenderLocal()
           return if ticket.updated_by_id is App.Session.get().id
 
-          @shown() if !@widget?.actionController
+          @renderWidget() if !@widget?.actionController
     )
     @subscriptions.push(
       object: 'Ticket',
@@ -90,7 +89,7 @@ class SidebarChecklist extends App.Controller
       (item) =>
         return if item.updated_by_id is App.Session.get().id
         return if @widget?.actionController
-        @shown()
+        @renderWidget()
     )
     @subscriptions.push(
       object: 'Checklist',
@@ -107,7 +106,7 @@ class SidebarChecklist extends App.Controller
         item.ticket_id,
         (item) =>
           return if @widget?.actionController
-          @shown()
+          @renderWidget()
       )
       @subscriptions.push(
         object: 'Ticket',
@@ -136,23 +135,27 @@ class SidebarChecklist extends App.Controller
       url:  "#{@apiPath}/checklists/by_ticket/#{@ticket.id}"
       processData: true
       success: (data, status, xhr) =>
-        @clearWidget()
         @stopLoading()
 
-        if data.id
-          App.Collection.loadAssets(data.assets)
+        App.Collection.loadAssets(data.assets)
 
-          @checklist = App.Checklist.find(data.id)
+        @delay => @renderWidget(enterEditMode)
+    )
 
-          @widget = new App.SidebarChecklistShow(el: @elSidebar, parentVC: @, checklist: @checklist, readOnly: !@changeable, enterEditMode: enterEditMode)
-        else
-          @widget = new App.SidebarChecklistStart(el: @elSidebar, parentVC: @, readOnly: !@changeable)
+  renderWidget: (enterEditMode = false) =>
+    @clearWidget()
 
-        @subscribe()
+    @checklist = App.Checklist.find(@ticket.checklist_id)
 
-        @renderActions()
-        @badgeRenderLocal()
-    )
+    if @checklist
+      @widget = new App.SidebarChecklistShow(el: @elSidebar, parentVC: @, checklist: @checklist, readOnly: !@changeable, enterEditMode: enterEditMode)
+    else
+      @widget = new App.SidebarChecklistStart(el: @elSidebar, parentVC: @, readOnly: !@changeable)
+
+    @subscribe()
+
+    @renderActions()
+    @badgeRenderLocal()
 
   clearWidget: =>
     @widget?.el.empty()

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

@@ -7,7 +7,7 @@ class App.SidebarChecklistShow extends App.Controller
     'click .js-action':               'onAction'
     'change .js-checkbox':            'onCheckboxClick'
     'click .js-title':                'onTitleChange'
-    'click .js-checklist-item-edit':  'onActionButtonClicked'
+    'click .js-checklist-item-edit':  'onEntryTextClicked'
 
   elements:
     '.js-reorder':      'reorderButton'
@@ -18,6 +18,7 @@ class App.SidebarChecklistShow extends App.Controller
 
   constructor: ->
     super
+
     @render()
 
   render: ->
@@ -27,15 +28,6 @@ class App.SidebarChecklistShow extends App.Controller
       readOnly: @readOnly
     )
 
-    $('body').off('click').on('click', (e) =>
-      return if @actionController && @actionController.constructor.name is 'ChecklistReorder'
-      return if $(e.target).closest('.js-actions').length > 0
-      return if $(e.target).closest('button').length > 0
-      return if $(e.target).closest('.checkbox-replacement').length > 0
-
-      @actionController?.releaseController()
-    )
-
     @renderTable()
 
   checklistTitle: =>
@@ -46,20 +38,24 @@ class App.SidebarChecklistShow extends App.Controller
     @actionController?.releaseController()
     @actionController = new ChecklistReorder(parentVC: @)
 
+  setDisabled: (node, id) ->
+    $(node).closest("[data-id='" + id + "']").attr('disabled', true).addClass('u-unclickable u-low-opacity')
+
   onAdd: (e) =>
-    @addButton.attr('disabled', true)
+    addButton = e.target.closest('button')
+
+    $(addButton).attr('disabled', true)
 
     callbackDone = (data) =>
       @enterEditModeId = data.id
       @renderTable()
       @parentVC.subscribe()
-      @addButton.attr('disabled', false)
+      $(addButton).attr('disabled', false)
 
     item = new App.ChecklistItem
     item.checklist_id = @checklist.id
     item.text = ''
 
-
     item.save(
       done: ->
         App.ChecklistItem.full(@id, callbackDone, force: true)
@@ -69,41 +65,46 @@ class App.SidebarChecklistShow extends App.Controller
           msg:  App.i18n.translateContent(details.error)
         )
         @renderTable()
-        @addButton.attr('disabled', false)
     )
 
   onCheckboxClick: (e) =>
     upcomingState = e.currentTarget.checked
     id = parseInt(e.currentTarget.value)
 
-    e.currentTarget.disabled = true
-
     @updateChecklistItem(id, upcomingState, e.currentTarget)
 
-  onCheckOrUncheck: (id, e) =>
-    row  = $(e.currentTarget).closest('tr')
+  onCheckOrUncheck: (e) =>
+    @preventDefaultAndStopPropagation(e)
+
+    row      = $(e.currentTarget).closest('tr')
+    id       = row.data('id')
     checkbox = row.find('.js-checkbox')[0]
 
     upcomingState = !checkbox.checked
 
-    checkbox.disabled = true
-
     @updateChecklistItem(id, upcomingState, checkbox)
 
   updateChecklistItem: (id, upcomingState, checkboxElem) =>
     item = App.ChecklistItem.find(id)
     item.checked = upcomingState
 
+    @setDisabled(checkboxElem, id)
+
     item.save(
       done: =>
-        checkboxElem.disabled = false
         @renderTable()
       fail: =>
         @renderTable()
     )
 
   onSaveOrder: (e) =>
-    @saveOrderButton.attr('disabled', true)
+    saveButton = e.target.closest('button')
+    cancelButton = $(saveButton).prev()
+    checklistTable = $(document).find('.checklistShow tbody')
+
+    $(saveButton).attr('disabled', true)
+    $(cancelButton).attr('disabled', true)
+    $(checklistTable).addClass('u-unclickable u-low-opacity')
 
     sorted_item_ids = @table.find('tbody tr').toArray().map (elem) -> elem.dataset.id
 
@@ -112,12 +113,19 @@ class App.SidebarChecklistShow extends App.Controller
     item.save(
       done: (data) =>
         @actionController?.completed()
+        $(saveButton).attr('disabled', false)
+        $(cancelButton).attr('disabled', false)
+        $(checklistTable).removeClass('u-unclickable u-low-opacity')
+
+
       fail: =>
         @notify(
           type: 'error'
           msg:  App.i18n.translateInline('Failed to save the order of the checklist items. Please try again.')
         )
-        @saveOrderButton.attr('disabled', false)
+        $(cancelButton).attr('disabled', false)
+        $(saveButton).attr('disabled', false)
+        $(checklistTable).removeClass('u-unclickable u-low-opacity')
     )
 
   onResetOrder: (e) =>
@@ -128,12 +136,15 @@ class App.SidebarChecklistShow extends App.Controller
 
     dropdown = $(e.currentTarget).closest('td').find('.js-table-action-menu')
     dropdown.dropdown('toggle')
-    dropdown.off('click.dropdown').on('click.dropdown', '[data-table-action]', @onActionButtonClicked)
-    dropdown.off('keyup.dropdown').on('keyup.dropdown', '[data-table-action]', @onActionButtonKeyup)
+
+    dropdown
+      .off('click.dropdown')
+      .on('click.dropdown', '[data-table-action=delete]', @onDeleteChecklistItem)
+      .on('click.dropdown', '[data-table-action=edit]', @onEditChecklistItem)
+      .on('click.dropdown', '[data-table-action=check]', @onCheckOrUncheck)
 
   onTitleChange: (e) =>
-    e?.stopPropagation()
-    return if @actionController && @actionController.constructor.name is 'ChecklistReorder'
+    @preventDefaultAndStopPropagation(e)
 
     # Close any open dropdowns
     @el.find('.dropdown--actions.open').dropdown('toggle')
@@ -146,57 +157,47 @@ class App.SidebarChecklistShow extends App.Controller
     @actionController?.releaseController()
     @actionController = new ChecklistRenameEdit(el: elem, parentVC: @, originalValue: @checklistTitle())
 
-  onActionButtonKeyup: (e) =>
-    return if e.key isnt 'Enter'
-
-    @onActionButtonClicked(e)
+  onEntryTextClicked: (e) =>
+    return if @actionController instanceof ChecklistItemEdit
 
-  onActionButtonClicked: (e) =>
-    e?.stopPropagation()
-    return if @actionController && @actionController.constructor.name is 'ChecklistReorder'
+    # skip on link openings
+    return if e.target.tagName is 'A'
 
-    id = $(e.currentTarget).parents('tr').data('id')
-    name = e.currentTarget.getAttribute('data-table-action')
-
-    if name is 'edit'
-
-      # skip on link openings
-      return if e.target.tagName is 'A' && $(e.target).parent().hasClass('checklistItemValue')
-      @onEditChecklistItem(id, e)
+    @preventDefaultAndStopPropagation(e)
 
-    else if name is 'delete'
-      @onDeleteChecklistItem(id, e)
+    @onEditChecklistItem(e)
 
-    else if _.contains(['check', 'uncheck'], name)
-      @onCheckOrUncheck(id, e)
+  toggleSortability: (isSorting, disablingCommand = 'disable') =>
+    @table.find('tbody').sortable(if isSorting then 'enable' else disablingCommand)
 
   toggleReorder: (isReordering, disablingCommand = 'disable') =>
-    @table.find('tbody').sortable(if isReordering then 'enable' else disablingCommand)
+    @toggleSortability(isReordering, disablingCommand)
 
     @table.find('.draggable').toggleClass('hide', !isReordering)
     @table.find('.checkbox-replacement').toggleClass('hide', isReordering)
     @table.find('.checkbox-replacement-readonly').toggleClass('hide', !isReordering)
     @table.find('.dropdown').toggleClass('hide', isReordering)
+    @table.find('.checklistItemValue').toggleClass('js-checklist-item-edit u-clickable', !isReordering && !@readonly)
 
     @reorderButton.toggleClass('hide', isReordering)
     @addButton.toggleClass('hide', isReordering)
     @saveOrderButton.toggleClass('hide', !isReordering)
     @resetOrderButton.toggleClass('hide', !isReordering)
 
-  onEditChecklistItem: (id, e) =>
+  onEditChecklistItem: (e) =>
     @preventDefaultAndStopPropagation(e)
 
-    return if @actionController && @actionController.constructor.name is 'ChecklistItemEdit' && @actionController.id == id
-
     row  = $(e.currentTarget).closest('tr')
+    id   = row.data('id')
     cell = row.find('.checklistItemValue')[0]
 
     @activateItemEditMode(cell, row, id)
 
-  onDeleteChecklistItem: (id, e) =>
+  onDeleteChecklistItem: (e) =>
     @preventDefaultAndStopPropagation(e)
 
-    row = $(e.currentTarget).closest('tr')
+    row  = $(e.currentTarget).closest('tr')
+    id   = row.data('id')
 
     dropdown = $(e.currentTarget).closest('td').find('.js-table-action-menu')
     dropdown.dropdown('toggle')
@@ -204,10 +205,16 @@ class App.SidebarChecklistShow extends App.Controller
     item = App.ChecklistItem.find(id)
 
     deleteCallback = =>
+      row.find('.checklistItemValue').css('text-decoration', 'line-through')
+
+      @setDisabled(e.currentTarget, id)
+      
       item.destroy(
         done: =>
           @renderTable()
           @parentVC.subscribe()
+        fail: ->
+          row.find('.checklistItemValue').css('text-decoration', 'auto')
       )
 
     # Skip confirmation dialog if the item has no text.
@@ -283,12 +290,11 @@ class ChecklistItemEdit extends App.Controller
   events:
     'click .js-cancel':             'onCancel'
     'click .js-confirm':            'onConfirm'
-    'blur .js-input':               'onConfirm'
+    'blur .js-input':               'onBlur'
     'keyup #checklistItemEditText': 'onKeyUp'
 
   constructor: ->
     super
-
     @render()
 
   releaseController: =>
@@ -313,6 +319,16 @@ class ChecklistItemEdit extends App.Controller
   object: =>
     App.ChecklistItem.find(@id)
 
+  setDisabled: (node, id) ->
+    $(node).closest("[data-id='" + id + "']").attr('disabled', true).addClass('u-unclickable u-low-opacity')
+
+  onBlur: (e) =>
+    if $(e.originalEvent.relatedTarget).hasClass('js-cancel')
+      @onCancel(e)
+      return
+
+    @onConfirm(e)
+
   onCancel: (e) =>
     @preventDefaultAndStopPropagation(e)
     @releaseController()
@@ -320,12 +336,22 @@ class ChecklistItemEdit extends App.Controller
   onConfirm: (e) =>
     @preventDefaultAndStopPropagation(e)
 
+    newValue = @input.val()
+
+    # Prevent AJAX if user has not changed the value
+    if @originalValue == newValue
+      @releaseController()
+      return
     item = @object()
-    item.text = @input.val()
+    item.text = newValue
+
+    @setDisabled(e.target, item.id)
+
     item.save(
       done: =>
         @parentVC.renderTable()
         @parentVC.parentVC.subscribe()
+        @originalValue = newValue
       fail: (settings, details) =>
         App.ChecklistItem.fetch(id: item.id)
 
@@ -338,7 +364,7 @@ class ChecklistItemEdit extends App.Controller
 
   onKeyUp: (e) =>
     switch e.key
-      when 'Enter' then @onConfirm()
+      when 'Enter' then @onConfirm(e)
       when 'Escape' then @releaseController()
 
 class ChecklistRenameEdit extends App.Controller
@@ -347,6 +373,7 @@ class ChecklistRenameEdit extends App.Controller
   events:
     'click .js-cancel':               'onCancel'
     'click .js-confirm':              'onConfirm'
+    'blur .js-input':                 'onBlur'
     'keyup #checklistTitleEditText':  'onKeyUp'
 
   constructor: ->
@@ -358,6 +385,9 @@ class ChecklistRenameEdit extends App.Controller
     @el.text(@originalValue)
     @parentVC.actionController = undefined
 
+  setDisabled: (node) ->
+    $(node).closest('tr').attr('disabled', true).addClass('u-unclickable u-low-opacity')
+
   render: =>
     @html App.view('ticket_zoom/sidebar_checklist_title_edit')(
       value: @object()?.name
@@ -368,12 +398,20 @@ class ChecklistRenameEdit extends App.Controller
   object: =>
     @parentVC.checklist
 
+  onBlur: (e) =>
+    if $(e.originalEvent.relatedTarget).hasClass('js-cancel')
+      @onCancel(e)
+      return
+
+    @onConfirm(e)
+
   onCancel: (e) =>
     @preventDefaultAndStopPropagation(e)
     @releaseController()
 
   onConfirm: (e) =>
     @preventDefaultAndStopPropagation(e)
+    @setDisabled(e.target)
 
     checklist = @object()
     checklist.name = @input.val()
@@ -384,7 +422,7 @@ class ChecklistRenameEdit extends App.Controller
 
   onKeyUp: (e) =>
     switch e.key
-      when 'Enter' then @onConfirm()
+      when 'Enter' then @onConfirm(e)
       when 'Escape' then @onCancel()
 
 class ChecklistReorder extends App.Controller
@@ -394,12 +432,10 @@ class ChecklistReorder extends App.Controller
 
   releaseController: =>
     @parentVC.toggleReorder(false, 'cancel')
-    @parentVC.saveOrderButton.attr('disabled', false)
     @parentVC.actionController = undefined
 
   completed: =>
     @parentVC.toggleReorder(false)
-    @parentVC.saveOrderButton.attr('disabled', false)
     @parentVC.actionController = undefined
 
   render: =>

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

@@ -37,7 +37,8 @@ class App.SidebarChecklistStart extends App.Controller
       data: JSON.stringify({ ticket_id: @parentVC.ticket.id })
       processData: true
       success: (data, status, xhr) =>
-        @parentVC.shown(true)
+        App.Collection.loadAssets(data.assets)
+        @parentVC.renderWidget(true)
     )
 
   onAddFromTemplate: (e) =>
@@ -56,7 +57,8 @@ class App.SidebarChecklistStart extends App.Controller
       url:  "#{@apiPath}/checklists"
       data: JSON.stringify({ ticket_id: @parentVC.ticket.id, template_id: params.checklist_template_id })
       success: (data, status, xhr) =>
-        @parentVC.shown()
+        App.Collection.loadAssets(data.assets)
+        @parentVC.renderWidget()
     )
 
   onTemplateChange: (e) =>

app/assets/javascripts/app/models/checklist.coffee

@@ -47,6 +47,8 @@ class App.Checklist extends App.Model
     }
 
   @calculateReferences: (ticket) ->
+    return [] if !ticket.referencing_checklist_ids
+
     checklists = App.Checklist
       .findAll(ticket.referencing_checklist_ids)
       .filter (elem) -> !elem.ticket_inaccessible

app/assets/javascripts/app/views/ticket_zoom/sidebar_checklist_show_row.jst.eco

@@ -56,38 +56,26 @@
             </button>
             <ul class="dropdown-menu dropdown-menu-right js-table-action-menu" role="menu">
             <% if !@object.ticket_id: %>
-              <% if !@object.checked: %>
-              <li role="presentation" class="" data-table-action="check">
-                <a role="menuitem" tabindex="-1">
-                  <span class="dropdown-iconSpacer">
-                    <%- @Icon('checkmark') %>
-                  </span>
+              <li role="menuitem" data-table-action="check">
+                <span class="dropdown-iconSpacer">
+                  <%- @Icon('checkmark') %>
+                </span>
+                <% if !@object.checked: %>
                   <%- @T('Check item') %>
-                </a>
-              <% else: %>
-              <li role="presentation" class="" data-table-action="uncheck">
-                <a role="menuitem" tabindex="-1">
-                  <span class="dropdown-iconSpacer">
-                    <%- @Icon('checkmark') %>
-                  </span>
+                <% else: %>
                   <%- @T('Uncheck item') %>
-                </a>
-              <% end %>
-              <li role="presentation" class="" data-table-action="edit">
-                <a role="menuitem" tabindex="-1">
-                  <span class="dropdown-iconSpacer">
-                    <%- @Icon('pen') %>
-                  </span>
-                  <%- @T('Edit item') %>
-                </a>
+                <% end %>
+              <li role="menuitem" data-table-action="edit">
+                <span class="dropdown-iconSpacer">
+                  <%- @Icon('pen') %>
+                </span>
+                <%- @T('Edit item') %>
             <% end %>
-              <li role="presentation" class="danger" data-table-action="delete">
-                <a role="menuitem" tabindex="-1">
-                  <span class="dropdown-iconSpacer">
-                    <%- @Icon('trash') %>
-                  </span>
-                  <%- @T('Remove item') %>
-                </a>
+              <li role="menuitem" class="danger" data-table-action="delete">
+                <span class="dropdown-iconSpacer">
+                  <%- @Icon('trash') %>
+                </span>
+                <%- @T('Remove item') %>
             </ul>
           <div>
         <div>

app/assets/stylesheets/zammad.scss

@@ -386,6 +386,10 @@ ul {
   pointer-events: none;
 }
 
+.u-low-opacity {
+  opacity: 60%;
+}
+
 .u-clickable {
   @extend %clickable;
 }

app/controllers/checklists_controller.rb

@@ -44,10 +44,6 @@ class ChecklistsController < ApplicationController
 
   private
 
-  def new_checklist_params
-    params.permit(:ticket_id, :name)
-  end
-
   def existing_checklist_params
     params.permit(:id, :name, sorted_item_ids: [])
   end

app/frontend/apps/desktop/components/CommonInlineEdit/CommonInlineEdit.vue

@@ -116,6 +116,12 @@ const submitEdit = () => {
   // Needs to be checked, because the 'onSubmit' function is not required.
   if (!props.onSubmitEdit) return undefined
 
+  // Don't trigger a mutation if there is no change
+  if (props.value === newEditValue.value) {
+    stopEditing(false)
+    return
+  }
+
   if (!checkValidity(inputValue.value)) return
 
   const submitEditResult = props.onSubmitEdit(inputValue.value)

app/models/checklist/assets.rb

@@ -10,7 +10,6 @@ class Checklist
       if !data[ app_model ]
         data[ app_model ] = {}
       end
-      Rails.logger.debug [app_model, id, data[app_model]]
       return data if data[ app_model ][ id ]
 
       data[ app_model ][ id ] = attributes_with_association_ids
@@ -20,6 +19,7 @@ class Checklist
       end
 
       items.each { |elem| elem.assets(data) }
+      ticket.assets(data)
 
       data
     end

app/models/checklist/item/assets.rb

@@ -10,7 +10,6 @@ class Checklist::Item
       if !data[ app_model ]
         data[ app_model ] = {}
       end
-      Rails.logger.debug [app_model, id, data[app_model]]
       return data if data[ app_model ][ id ]
 
       data[ app_model ][ id ] = attributes_with_association_ids