From f7a59c6bbd2c0373b13241e291abdf253bb1e49a Mon Sep 17 00:00:00 2001
From: Billy Zhou <bz@zammad.com>
Date: Fri, 4 Jan 2019 22:26:43 +0800
Subject: [PATCH] Fixed #2242, required attribute not being enforeced, for the
 frontend only.

---
 .../app/controllers/ticket_zoom.coffee        |  8 +++-
 spec/factories/object_manager_attribute.rb    | 48 +++++++++++++++++++
 spec/support/capybara/common_actions.rb       | 29 +++++++++++
 spec/system/ticket/update_spec.rb             | 25 ++++++++++
 4 files changed, 109 insertions(+), 1 deletion(-)
 create mode 100644 spec/system/ticket/update_spec.rb

diff --git a/app/assets/javascripts/app/controllers/ticket_zoom.coffee b/app/assets/javascripts/app/controllers/ticket_zoom.coffee
index 9a632722f..73fc47fc1 100644
--- a/app/assets/javascripts/app/controllers/ticket_zoom.coffee
+++ b/app/assets/javascripts/app/controllers/ticket_zoom.coffee
@@ -807,7 +807,13 @@ class App.TicketZoom extends App.Controller
     # stop autosave
     @autosaveStop()
 
-    # validate ticket
+    # validate ticket form using HTML5 validity check
+    if !@$('.edit').parent().get(0).reportValidity()
+      @submitEnable(e)
+      @autosaveStart()
+      return
+
+    # validate ticket by model
     errors = ticket.validate(
       screen: 'edit'
     )
diff --git a/spec/factories/object_manager_attribute.rb b/spec/factories/object_manager_attribute.rb
index 9ec41443b..07e82943f 100644
--- a/spec/factories/object_manager_attribute.rb
+++ b/spec/factories/object_manager_attribute.rb
@@ -220,4 +220,52 @@ FactoryBot.define do
       }
     end
   end
+
+  factory :required_screen, class: Hash do
+    create_middle do
+      {
+        'ticket.customer'    => {
+          shown:      true,
+          required:   true,
+          item_class: 'column'
+        },
+        'ticket.agent'       => {
+          shown:      true,
+          required:   true,
+          item_class: 'column'
+        },
+        'admin.organization' => {
+          shown:    true,
+          required: true,
+        },
+        'admin.group'        => {
+          shown:      true,
+          required:   true,
+          item_class: 'column'
+        },
+      }
+    end
+
+    edit          do
+      {
+        'ticket.customer'    => {
+          shown:    true,
+          required: true
+        },
+        'ticket.agent'       => {
+          shown:    true,
+          required: true
+        },
+        'admin.organization' => {
+          shown:    true,
+          required: true,
+        },
+        'admin.group'        => {
+          shown:      true,
+          required:   true,
+          item_class: 'column'
+        },
+      }
+    end
+  end
 end
diff --git a/spec/support/capybara/common_actions.rb b/spec/support/capybara/common_actions.rb
index acf85a141..f9194a39b 100644
--- a/spec/support/capybara/common_actions.rb
+++ b/spec/support/capybara/common_actions.rb
@@ -125,6 +125,35 @@ module CommonActions
   def expect_current_route(route, **options)
     expect(page).to have_current_route(route, **options)
   end
+
+  # Create and migrate an object manager attribute and verify that it exists. Returns the newly attribute.
+  #
+  # Create a select attribute:
+  # @example
+  #  attribute = setup_attribute :object_manager_attribute_select
+  #
+  # Create a required text attribute:
+  # @example
+  #  attribute = setup_attribute :object_manager_attribute_text,
+  #                               screens: attributes_for(:required_screen)
+  #
+  # Create a date attribute with custom parameters:
+  # @example
+  #  attribute = setup_attribute :object_manager_attribute_date,
+  #                              data_option: {
+  #                                'future' => true,
+  #                                'past'   => false,
+  #                                'diff'   => 24,
+  #                                'null'   => true,
+  #                              }
+  #
+  # return [attribute]
+  def create_attribute(attribute_name, attribute_parameters = {})
+    attribute = create(attribute_name, attribute_parameters)
+    ObjectManager::Attribute.migration_execute
+    page.driver.browser.navigate.refresh
+    attribute
+  end
 end
 
 RSpec.configure do |config|
diff --git a/spec/system/ticket/update_spec.rb b/spec/system/ticket/update_spec.rb
new file mode 100644
index 000000000..0395548c9
--- /dev/null
+++ b/spec/system/ticket/update_spec.rb
@@ -0,0 +1,25 @@
+require 'rails_helper'
+
+RSpec.describe 'Ticket Create', type: :system do
+
+  let(:group) { Group.find_by(name: 'Users') }
+
+  # Regression test for issue #2242 - mandatory fields can be empty (or "-") on ticket update
+  context 'when updating a ticket without its required select attributes' do
+    scenario 'frontend checks reject the update', db_strategy: :reset do
+      # setup and migrate a required select attribute
+      attribute = create_attribute :object_manager_attribute_select,
+                                   screens: attributes_for(:required_screen)
+
+      # create a new ticket and attempt to update its state without the required select attribute
+      ticket = create :ticket, group: group
+      visit "#ticket/zoom/#{ticket.id}"
+      select 'closed', from: 'state_id'
+      click('.content.active .js-attributeBar .js-submit')
+      expect(page).to have_css('.content.active')
+
+      # the update should have failed and thus the ticket is still in the new state
+      expect(ticket.reload.state.name).to eq('new')
+    end
+  end
+end