From cc2bc4f188f6a96e96846d6225992574633f2201 Mon Sep 17 00:00:00 2001 From: Mantas Masalskis Date: Wed, 13 May 2020 20:01:42 +0300 Subject: [PATCH] Fixes #2867 - KB links are in the header and footer of the public KB, Fixes #2834 - unclear meaning of "Public Menu" tab in KB admin --- .rubocop_todo.rspec.yml | 1 + .../controllers/_manage/knowledge_base.coffee | 2 +- .../knowledge_base/public_menu_form.coffee | 78 ++++++++++++++--- .../public_menu_form_item.coffee | 84 +++++-------------- .../knowledge_base/public_menu_manager.coffee | 49 +++++++++++ .../models/knowledge_base_menu_item.coffee | 5 ++ .../public_menu_form_item.jst.eco | 8 +- .../public_menu_manager.jst.eco | 40 +++++++++ app/assets/stylesheets/zammad.scss | 38 +++++++++ .../knowledge_base/manage_controller.rb | 7 +- .../knowledge_base/public/base_controller.rb | 4 +- app/models/knowledge_base/menu_item.rb | 17 ++-- app/views/layouts/knowledge_base.html.erb | 9 +- ...0190531180304_initialize_knowledge_base.rb | 1 + ...53_issue_2867_footer_header_public_link.rb | 14 ++++ lib/knowledge_base/menu_item_update_action.rb | 47 ++++++++++- ...sue_2867_footer_header_public_link_spec.rb | 39 +++++++++ spec/factories/knowledge_base/menu_item.rb | 16 +++- spec/models/knowledge_base/menu_item_spec.rb | 65 +++++++++----- .../admin/knowledge_base/public_menu_spec.rb | 67 +++++++++++++++ spec/support/db_migration.rb | 21 +++++ spec/support/knowledge_base_contexts.rb | 8 ++ .../admin/knowledge_base/public_menu_spec.rb | 74 ++++++++++++++++ .../knowledge_base_public/menu_items_spec.rb | 38 +++++++++ 24 files changed, 607 insertions(+), 125 deletions(-) create mode 100644 app/assets/javascripts/app/controllers/knowledge_base/public_menu_manager.coffee create mode 100644 app/assets/javascripts/app/views/knowledge_base/public_menu_manager.jst.eco create mode 100644 db/migrate/20190918114553_issue_2867_footer_header_public_link.rb create mode 100644 spec/db/migrate/issue_2867_footer_header_public_link_spec.rb create mode 100644 spec/requests/admin/knowledge_base/public_menu_spec.rb create mode 100644 spec/system/admin/knowledge_base/public_menu_spec.rb create mode 100644 spec/system/knowledge_base_public/menu_items_spec.rb diff --git a/.rubocop_todo.rspec.yml b/.rubocop_todo.rspec.yml index 638f35dd9..4e2087e32 100644 --- a/.rubocop_todo.rspec.yml +++ b/.rubocop_todo.rspec.yml @@ -91,6 +91,7 @@ RSpec/FilePath: - 'spec/db/migrate/issue_2460_fix_corrupted_twitter_ids_spec.rb' - 'spec/db/migrate/issue_2715_fix_broken_twitter_urls_spec.rb' - 'spec/jobs/issue_2715_fix_broken_twitter_urls_job_spec.rb' + - 'spec/db/migrate/issue_2867_footer_header_public_link_spec.rb' - 'spec/lib/import/base_factory_spec.rb' # Offense count: 60 diff --git a/app/assets/javascripts/app/controllers/_manage/knowledge_base.coffee b/app/assets/javascripts/app/controllers/_manage/knowledge_base.coffee index 6cdfcb82b..da290cb66 100644 --- a/app/assets/javascripts/app/controllers/_manage/knowledge_base.coffee +++ b/app/assets/javascripts/app/controllers/_manage/knowledge_base.coffee @@ -109,7 +109,7 @@ class App.ManageKnowledgeBase extends App.ControllerTabs },{ name: 'Public Menu' target: 'public_menu' - controller: App.KnowledgeBasePublicMenuForm + controller: App.KnowledgeBasePublicMenuManager params: _.extend({}, params, { screen: 'public_menu' }) },{ name: 'Delete' diff --git a/app/assets/javascripts/app/controllers/knowledge_base/public_menu_form.coffee b/app/assets/javascripts/app/controllers/knowledge_base/public_menu_form.coffee index f30ad420a..99d11fd58 100644 --- a/app/assets/javascripts/app/controllers/knowledge_base/public_menu_form.coffee +++ b/app/assets/javascripts/app/controllers/knowledge_base/public_menu_form.coffee @@ -1,17 +1,69 @@ -class App.KnowledgeBasePublicMenuForm extends App.Controller - events: - 'show.bs.tab': 'willShow' +class App.KnowledgeBasePublicMenuForm extends App.ControllerModal + autoFocusOnFirstInput: false + includeForm: true - willShow: -> - @el.empty() + constructor: (params) -> + @formItems = [] + @head = params.location.headline + super - for kb_locale in App.KnowledgeBase.find(@knowledge_base_id).kb_locales() - menu_items = App.KnowledgeBaseMenuItem.using_kb_locale(kb_locale) + formParams: => + @formItems.map (elem) -> elem.buildData() - form_item = new App.KnowledgeBasePublicMenuFormItem( - knowledge_base_id: @knowledge_base_id, - kb_locale: kb_locale, - menu_items: menu_items - ) + content: -> + @formItems = App.KnowledgeBase + .find(@knowledge_base_id) + .kb_locales() + .map (kb_locale) => + menu_items = App.KnowledgeBaseMenuItem.using_kb_locale_location(kb_locale, @location.identifier) - @el.append form_item.el + new App.KnowledgeBasePublicMenuFormItem( + parent: @, + knowledge_base_id: @knowledge_base_id, + location: @location.identifier, + kb_locale: kb_locale, + menu_items: menu_items + ) + + @formItems.map (elem) -> elem.el + + hasError: -> + @formItems + .map (elem) -> elem.hasError() + .filter((elem) -> elem) + .pop() + + onSubmit: (e) -> + @preventDefaultAndStopPropagation(e) + + if error = @hasError() + @showAlert(error) + return + + @clearAlerts() + @formItems.forEach (elem) -> elem.toggleUserInteraction(false) + + kb = App.KnowledgeBase.find(@knowledge_base_id) + + @ajax( + id: 'update_menu_items' + type: 'PATCH' + url: kb.manageUrl('update_menu_items') + data: JSON.stringify(menu_items_sets: @formParams()) + processData: true + success: @onSuccess + error: @onError + ) + + onSuccess: (data, status, xhr) => + for formItem in @formItems + for menuItem in App.KnowledgeBaseMenuItem.using_kb_locale_location(formItem.kb_locale, formItem.location) + menuItem.remove(clear: true) + + App.Collection.loadAssets(data.assets) + App.KnowledgeBaseMenuItem.trigger('kb_data_change_loaded') + @close() + + onError: (xhr) => + @showAlert(xhr.responseJSON?.error_human || 'Couldn\'t save changes') + @formItems.forEach (elem) -> elem.toggleUserInteraction(true) diff --git a/app/assets/javascripts/app/controllers/knowledge_base/public_menu_form_item.coffee b/app/assets/javascripts/app/controllers/knowledge_base/public_menu_form_item.coffee index 1c403cabf..96e63c7de 100644 --- a/app/assets/javascripts/app/controllers/knowledge_base/public_menu_form_item.coffee +++ b/app/assets/javascripts/app/controllers/knowledge_base/public_menu_form_item.coffee @@ -3,7 +3,6 @@ class App.KnowledgeBasePublicMenuFormItem extends App.Controller 'click .js-add': 'add' 'click .js-remove': 'remove' 'input input': 'input' - 'submit form': 'submit' elements: '.js-alert': 'alert' @@ -14,30 +13,29 @@ class App.KnowledgeBasePublicMenuFormItem extends App.Controller render: -> @html App.view('knowledge_base/public_menu_form_item')( - kb_locale_id: @kb_locale.id - rows: @menu_items - title: @kb_locale.systemLocale().name + rows: @menu_items + title: @kb_locale.systemLocale().name ) @applySortable() applySortable: -> dndOptions = - tolerance: 'pointer' - distance: 15 - opacity: 0.6 - items: 'tr.sortable' - start: (e, ui) -> + tolerance: 'pointer' + distance: 15 + opacity: 0.6 + items: 'tr.sortable' + start: (e, ui) -> ui.placeholder.height( ui.item.height() ) - helper: (e, tr) -> + helper: (e, tr) -> originals = tr.children() helper = tr helper.children().each (index, el) -> # Set helper cell sizes to match the original sizes $(@).width( originals.eq(index).width() ) return helper - update: @dndCallback - stop: (e, ui) -> + update: @dndCallback + stop: (e, ui) -> ui.item.children().each (index, element) -> element.style.width = '' @@ -66,13 +64,14 @@ class App.KnowledgeBasePublicMenuFormItem extends App.Controller } { - kb_locale_id: @$('form').data('kb-locale-id'), - menu_items: items + kb_locale_id: @kb_locale.id, + location: @location, + menu_items: items } input: -> - if @validateForm(false) - @hideAlert() + if !@hasError() + @parent.clearAlerts() add: -> el = App.view('knowledge_base/public_menu_form_item_row')() @@ -88,57 +87,14 @@ class App.KnowledgeBasePublicMenuFormItem extends App.Controller else row.remove() - showAlert: (message) -> - translated = App.i18n.translatePlain(message) - - @alert - .text(translated) - .removeClass('hidden') - - hideAlert: -> - @alert.addClass('hidden') - - emptyFields: -> + findEmptyFields: -> @$('tr.sortable:not(.js-deleted)') .find('input[data-name]') .toArray() .filter (elem) -> $(elem).val().length == 0 - validateForm: (showAlert = true) -> - if @emptyFields().length == 0 - return true + hasError: -> + if @findEmptyFields().length == 0 + return false - if showAlert - @showAlert('Please fill in all fields') - - false - - submit: (e) -> - @preventDefaultAndStopPropagation(e) - - if !@validateForm() - return - - @hideAlert() - @toggleUserInteraction(false) - - kb = App.KnowledgeBase.find(@knowledge_base_id) - - @ajax( - id: 'update_menu_items' - type: 'PATCH' - url: kb.manageUrl('update_menu_items') - data: JSON.stringify(@buildData()) - processData: true - success: (data, status, xhr) => - for menu_item in App.KnowledgeBaseMenuItem.using_kb_locale(@kb_locale) - menu_item.remove(clear: true) - - App.Collection.loadAssets(data.assets) - - @menu_items = App.KnowledgeBaseMenuItem.using_kb_locale(@kb_locale) - @render() - error: (xhr) => - @showAlert(xhr.responseJSON?.error_human || 'Couldn\'t save changes') - @toggleUserInteraction(true) - ) + 'Please fill in all fields' diff --git a/app/assets/javascripts/app/controllers/knowledge_base/public_menu_manager.coffee b/app/assets/javascripts/app/controllers/knowledge_base/public_menu_manager.coffee new file mode 100644 index 000000000..879497e6d --- /dev/null +++ b/app/assets/javascripts/app/controllers/knowledge_base/public_menu_manager.coffee @@ -0,0 +1,49 @@ +class App.KnowledgeBasePublicMenuManager extends App.Controller + events: + 'show.bs.tab': 'willShow' + 'click .js-edit': 'edit' + + constructor: -> + super + + @listenTo App.KnowledgeBaseMenuItem, 'kb_data_change_loaded', => + @render() + + willShow: -> + @render() + + render: -> + kb = App.KnowledgeBase.find(@knowledge_base_id) + + @html App.view('knowledge_base/public_menu_manager')( + locations: @locations(), + locales: kb.kb_locales() + ) + + locations: -> + kb = App.KnowledgeBase.find(@knowledge_base_id) + + [ + { + headline: 'Header menu', + identifier: 'header', + color: kb.color_header + }, + { + headline: 'Footer menu', + identifier: 'footer' + } + ] + + + edit: (e) => + @preventDefaultAndStopPropagation(e) + + identifier = $(e.target).data('target-location') + location = _.find @locations(), (elem) -> elem.identifier == identifier + + new App.KnowledgeBasePublicMenuForm( + location: location, + knowledge_base_id: @knowledge_base_id + container: @el.closest('.main') + ) diff --git a/app/assets/javascripts/app/models/knowledge_base_menu_item.coffee b/app/assets/javascripts/app/models/knowledge_base_menu_item.coffee index acb7acfa9..d8977d16a 100644 --- a/app/assets/javascripts/app/models/knowledge_base_menu_item.coffee +++ b/app/assets/javascripts/app/models/knowledge_base_menu_item.coffee @@ -5,3 +5,8 @@ class App.KnowledgeBaseMenuItem extends App.Model items = @findAllByAttribute('kb_locale_id', kb_locale.id) items.sort( (a, b) -> if a.position < b.position then -1 else 1) items + + @using_kb_locale_location: (kb_locale, location) -> + items = @all().filter (elem) -> elem.kb_locale_id is kb_locale.id and elem.location is location + items.sort( (a, b) -> if a.position < b.position then -1 else 1) + items diff --git a/app/assets/javascripts/app/views/knowledge_base/public_menu_form_item.jst.eco b/app/assets/javascripts/app/views/knowledge_base/public_menu_form_item.jst.eco index 96d5e0b4e..f9b320370 100644 --- a/app/assets/javascripts/app/views/knowledge_base/public_menu_form_item.jst.eco +++ b/app/assets/javascripts/app/views/knowledge_base/public_menu_form_item.jst.eco @@ -1,4 +1,4 @@ -
+

<%= @title %>

@@ -28,8 +28,4 @@
- -
- -
-
+ diff --git a/app/assets/javascripts/app/views/knowledge_base/public_menu_manager.jst.eco b/app/assets/javascripts/app/views/knowledge_base/public_menu_manager.jst.eco new file mode 100644 index 000000000..3ccb6b454 --- /dev/null +++ b/app/assets/javascripts/app/views/knowledge_base/public_menu_manager.jst.eco @@ -0,0 +1,40 @@ +

+ <%- @T 'Public Menu' %> +

+ +

+ <%- @T 'Here you can add further links to your public FAQ page, which will be displayed either in the header or footer.' %> +

+ +<% for location in @locations: %> +
+

<%= @T(location.headline) %>

+ + <% for kb_locale in @locales: %> +
+
<%= kb_locale.systemLocale().name %>
+ +
+ <% menu_items = App.KnowledgeBaseMenuItem.using_kb_locale_location(kb_locale, location.identifier) %> + + <% if menu_items.length == 0: %> + <%= @T 'Empty' %> + <% else: %> + <% for item in menu_items: %> + <%= item.title %> + <% end %> + <% end %> +
+
+ <% end %> + + + + <%= @T 'Edit' %> + +
+<% end %> diff --git a/app/assets/stylesheets/zammad.scss b/app/assets/stylesheets/zammad.scss index 0efd396ea..c0001482a 100644 --- a/app/assets/stylesheets/zammad.scss +++ b/app/assets/stylesheets/zammad.scss @@ -2472,6 +2472,36 @@ input.has-error { } } +.kb-menu-preview { + margin-bottom: 1em; + + &-container { + display: flex; + justify-content: flex-end; + border: 1px solid hsl(213,14%,91%); + + &--footer { + justify-content: center; + } + } + + a, span { + font-size: 14px; + padding: .5em 1em; + white-space: nowrap; + text-decoration: none; + line-height: 2em; + } + + a { + color: hsl(206,8%,50%); + } + + .label { + text-transform: none; + } +} + .modified-icon { position: relative; line-height: 1; @@ -11684,3 +11714,11 @@ span.is-disabled { width: 100%; height: 100%; } + +.btn-manage-public-menu-edit { + margin-top: 0 +} + +.kb-menu-settings-entry { + margin-bottom: 12px +} diff --git a/app/controllers/knowledge_base/manage_controller.rb b/app/controllers/knowledge_base/manage_controller.rb index 16a181f14..799d732a2 100644 --- a/app/controllers/knowledge_base/manage_controller.rb +++ b/app/controllers/knowledge_base/manage_controller.rb @@ -31,13 +31,10 @@ class KnowledgeBase::ManageController < KnowledgeBase::BaseController def update_menu_items kb = KnowledgeBase.find params[:id] - kb_locale = kb.kb_locales.find params[:kb_locale_id] - KnowledgeBase::MenuItemUpdateAction - .new(kb_locale, params[:menu_items]) - .perform! + affected_items = KnowledgeBase::MenuItemUpdateAction.update_using_params! kb, params_for_permission[:menu_items_sets] - render json: { assets: ApplicationModel::CanAssets.reduce(kb_locale.menu_items.reload, {}) } + render json: { assets: ApplicationModel::CanAssets.reduce(affected_items || [], {}) } end def destroy diff --git a/app/controllers/knowledge_base/public/base_controller.rb b/app/controllers/knowledge_base/public/base_controller.rb index ca1f17010..8f1deeb3d 100644 --- a/app/controllers/knowledge_base/public/base_controller.rb +++ b/app/controllers/knowledge_base/public/base_controller.rb @@ -24,9 +24,7 @@ class KnowledgeBase::Public::BaseController < ApplicationController end def menu_items - @menu_items ||= KnowledgeBase::MenuItem - .sorted - .using_locale(guess_locale_via_uri || filter_primary_kb_locale) + @menu_items ||= KnowledgeBase::MenuItem.using_locale(guess_locale_via_uri || filter_primary_kb_locale) end def system_locale_via_uri diff --git a/app/models/knowledge_base/menu_item.rb b/app/models/knowledge_base/menu_item.rb index 81ccd6a3a..8c89867c4 100644 --- a/app/models/knowledge_base/menu_item.rb +++ b/app/models/knowledge_base/menu_item.rb @@ -1,17 +1,24 @@ class KnowledgeBase::MenuItem < ApplicationModel belongs_to :kb_locale, class_name: 'KnowledgeBase::Locale', inverse_of: :menu_items, touch: true - validates :title, presence: true, length: { maximum: 100 } - validates :url, presence: true, length: { maximum: 100 } + validates :title, presence: true, length: { maximum: 100 } + validates :url, presence: true, length: { maximum: 500 } + validates :location, presence: true, inclusion: { in: %w[header footer] } - acts_as_list scope: :kb_locale, top_of_list: 0 + acts_as_list scope: %i[kb_locale_id location], top_of_list: 0 - scope :sorted, -> { order(position: :asc) } - scope :using_locale, ->(locale) { locale.present? ? joins(:kb_locale).where(knowledge_base_locales: { system_locale_id: locale.id } ) : none } + scope :sorted, -> { order(position: :asc) } + scope :using_locale, ->(locale) { locale.present? ? joins(:kb_locale).where(knowledge_base_locales: { system_locale_id: locale.id } ) : none } + scope :location, ->(location) { sorted.where(location: location) } + + scope :location_header, -> { location(:header) } + scope :location_footer, -> { location(:footer) } private def add_protocol_prefix + return if url.blank? + url.strip! return if url.match? %r{^\S+\:\/\/} diff --git a/app/views/layouts/knowledge_base.html.erb b/app/views/layouts/knowledge_base.html.erb index ede5e249d..2a1df3589 100644 --- a/app/views/layouts/knowledge_base.html.erb +++ b/app/views/layouts/knowledge_base.html.erb @@ -25,7 +25,7 @@ <% end %> @@ -51,6 +51,13 @@
<%= @knowledge_base.translation.footer_note %>
+ + +