From aa53c4e69777565ab5fcd5dceac714108546b907 Mon Sep 17 00:00:00 2001 From: Thorsten Eckel Date: Thu, 27 Apr 2017 10:46:49 +0200 Subject: [PATCH 01/12] Improved association change tracking for broader use cases. --- lib/import/base_resource.rb | 28 +++++++++++++++++++++++----- lib/import/ldap/user.rb | 2 +- 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/lib/import/base_resource.rb b/lib/import/base_resource.rb index 1072a918e..08fc7c245 100644 --- a/lib/import/base_resource.rb +++ b/lib/import/base_resource.rb @@ -6,7 +6,10 @@ module Import def initialize(resource, *args) handle_args(resource, *args) + initialize_associations_states import(resource, *args) + return if @resource.blank? + store_associations(:after, @resource) end def import_class @@ -30,8 +33,7 @@ module Import end def attributes_changed? - return true if changed_attributes.present? - @associations_init != associations_state(@resource) + changed_attributes.present? || changed_associations.present? end def changed_attributes @@ -42,6 +44,15 @@ module Import @resource.previous_changes end + def changed_associations + changes = {} + tracked_associations.each do |association| + next if @associations[:before][association] == @associations[:after][association] + changes[association] = [@associations[:before][association], @associations[:after][association]] + end + changes + end + def created? return false if @resource.blank? # dry run @@ -52,6 +63,13 @@ module Import private + def initialize_associations_states + @associations = {} + %i(before after).each do |state| + @associations[state] ||= {} + end + end + def import(resource, *args) create_or_update(map(resource, *args), *args) rescue => e @@ -96,13 +114,13 @@ module Import return if !synced_instance instance = import_class.find_by(id: synced_instance.o_id) - store_associations_state(instance) + store_associations(:before, instance) instance end - def store_associations_state(instance) - @associations_init = associations_state(instance) + def store_associations(state, instance) + @associations[state] = associations_state(instance) end def associations_state(instance) diff --git a/lib/import/ldap/user.rb b/lib/import/ldap/user.rb index 26525b449..7d2376ed7 100644 --- a/lib/import/ldap/user.rb +++ b/lib/import/ldap/user.rb @@ -94,7 +94,7 @@ module Import remote: resource, ) - store_associations_state(instance) + store_associations(:before, instance) end instance From 591881ccf64a0cb060ea1086715fe470ce497c27 Mon Sep 17 00:00:00 2001 From: Thorsten Eckel Date: Thu, 27 Apr 2017 10:49:03 +0200 Subject: [PATCH 02/12] Working on issue #350 - issuecomment-295252402 - Users lock theirselfs out if they provide no role mapping. --- lib/import/ldap/user.rb | 49 +++++++++++++++++++++++++++---- spec/lib/import/ldap/user_spec.rb | 15 +++++++++- 2 files changed, 58 insertions(+), 6 deletions(-) diff --git a/lib/import/ldap/user.rb b/lib/import/ldap/user.rb index 7d2376ed7..a71415aea 100644 --- a/lib/import/ldap/user.rb +++ b/lib/import/ldap/user.rb @@ -31,7 +31,6 @@ module Import def create_or_update(resource, *args) return if skip?(resource) - resource[:role_ids] = role_ids(resource) result = super(resource, *args) ldap_log( @@ -58,14 +57,49 @@ module Import return @signup_role_ids if !dn # check if roles are mapped for the found dn roles = @dn_roles[ dn.downcase ] - # use signup roles if no mapped roles were found - return @signup_role_ids if !roles # return found roles - roles + return roles if roles + # return signup roles if there is a role mapping in general + # this makes the LDAP the leading source of roles + return @signup_role_ids if @dn_roles.present? + + # special case: there is no LDAP role mapping configured + # + # if there is no role mapping in general the signup roles + # should only get used for newly created users (see .create method) + # otherwise users might overwrite their local assigned + # roles with less privileged roles and lock theirselfs out + # + # see issue 350#issuecomment-295252402 + # + # this method only gets called from the .updated? method + # so we won't return any roles which will keep the current + # role assignment for the User + # the signup roles will be taken in this case direcly in + # the .create method + nil end def updated?(resource, *_args) - super + + # this is needed for the special case described in the role_ids method + # + # assign roles only if there are any found in the mapping + # otherwise keep those stored to the User + update_roles = role_ids(resource) + resource[:role_ids] = update_roles if update_roles + + user_found = super + + # in case a User was found and we had no roles + # to set/update we have to note the currently + # assigned roles so that our action check won't + # falsly detect changes to the User + if user_found && update_roles.blank? + resource[:role_ids] = @resource.role_ids + end + + user_found rescue => e ldap_log( action: "update -> #{resource[:login]}", @@ -122,6 +156,11 @@ module Import end def create(resource, *_args) + + # this is needed for the special case described in the role_ids method + # + # in case we have no role IDs yet we have to fall back to the signup roles + resource[:role_ids] ||= @signup_role_ids super rescue => e ldap_log( diff --git a/spec/lib/import/ldap/user_spec.rb b/spec/lib/import/ldap/user_spec.rb index 69f46ad22..f112b4ecd 100644 --- a/spec/lib/import/ldap/user_spec.rb +++ b/spec/lib/import/ldap/user_spec.rb @@ -99,7 +99,12 @@ RSpec.describe Import::Ldap::User do context 'update' do before(:each) do - user = create(:user, login: uid) + user = create(:user, + login: uid, + role_ids: [ + Role.find_by(name: 'Agent').id, + Role.find_by(name: 'Admin').id + ]) ExternalSync.create( source: 'Ldap::User', @@ -119,6 +124,14 @@ RSpec.describe Import::Ldap::User do } end + it "doesn't change roles if no role mapping is configured" do + expect do + described_class.new(user_entry, ldap_config, {}, signup_role_ids) + end.to not_change { + User.last.role_ids + } + end + it 'creates an HTTP Log entry' do expect do described_class.new(user_entry, ldap_config, user_roles, signup_role_ids) From ed78be7d50311d41e35739a46ca3c5f7662721c6 Mon Sep 17 00:00:00 2001 From: Martin Edenhofer Date: Thu, 27 Apr 2017 09:34:40 +0200 Subject: [PATCH 03/12] Load default gems based on bundle config. --- script/scheduler.rb | 16 ++++++++++++---- script/websocket-server.rb | 4 ++++ 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/script/scheduler.rb b/script/scheduler.rb index b4252b866..8ea47d270 100755 --- a/script/scheduler.rb +++ b/script/scheduler.rb @@ -3,8 +3,18 @@ $LOAD_PATH << './lib' require 'rubygems' -require 'daemons' + +# load rails env dir = File.expand_path(File.join(File.dirname(__FILE__), '..')) +Dir.chdir dir +RAILS_ENV = ENV['RAILS_ENV'] || 'development' + +require 'rails/all' +require 'bundler' +Bundler.require(:default, Rails.env) +require File.join(dir, 'config', 'environment') + +require 'daemons' daemon_options = { multiple: false, @@ -22,12 +32,10 @@ Daemons.run_proc(name, daemon_options) do end Dir.chdir dir - RAILS_ENV = ARGV.first || ENV['RAILS_ENV'] || 'development' $stdout.reopen( dir + '/log/' + name + '_out.log', 'w') $stderr.reopen( dir + '/log/' + name + '_err.log', 'w') - require File.join(dir, 'config', 'environment') - require 'scheduler' + require 'scheduler' Scheduler.threads end diff --git a/script/websocket-server.rb b/script/websocket-server.rb index 7affdb355..e72c5d994 100755 --- a/script/websocket-server.rb +++ b/script/websocket-server.rb @@ -14,6 +14,10 @@ require 'daemons' dir = File.expand_path(File.join(File.dirname(__FILE__), '..')) Dir.chdir dir RAILS_ENV = ENV['RAILS_ENV'] || 'development' + +require 'rails/all' +require 'bundler' +Bundler.require(:default, Rails.env) require File.join(dir, 'config', 'environment') require 'sessions' From bfb9e1b1333e9d75504324fe1538385a36052ac3 Mon Sep 17 00:00:00 2001 From: Martin Edenhofer Date: Thu, 27 Apr 2017 11:07:31 +0200 Subject: [PATCH 04/12] Improved alignment of lists. --- app/assets/stylesheets/zammad.scss | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/assets/stylesheets/zammad.scss b/app/assets/stylesheets/zammad.scss index 40e675df4..70c8e0e9e 100644 --- a/app/assets/stylesheets/zammad.scss +++ b/app/assets/stylesheets/zammad.scss @@ -129,8 +129,8 @@ blockquote { font-size: inherit; } -ul { - padding-left: 10px; +ol, ul { + padding-left: 20px; } #app { From 3bb2f98ab4d353acbc29c38d3a144960d1cb2a83 Mon Sep 17 00:00:00 2001 From: Felix Niklas Date: Thu, 27 Apr 2017 11:26:45 +0200 Subject: [PATCH 05/12] Ticket Zoom - give editable header a background on hover, see #954 - clean out outdated code, fix layout bug when in translate mode --- app/assets/stylesheets/zammad.scss | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/app/assets/stylesheets/zammad.scss b/app/assets/stylesheets/zammad.scss index 70c8e0e9e..83261efd9 100644 --- a/app/assets/stylesheets/zammad.scss +++ b/app/assets/stylesheets/zammad.scss @@ -262,7 +262,7 @@ ol, ul { } [contenteditable]:hover, [contenteditable]:focus { - background: #f8f9fa; + background: hsl(210,17%,98%); } [contenteditable]:focus { text-overflow: clip !important; @@ -4523,7 +4523,12 @@ footer { white-space: normal; margin-top: 15px; margin-bottom: 8px; + padding: 0 7px; text-align: center; + + .ticketZoom-header &:hover { + background: hsl(210,17%,93%); + } } .task-subline { @@ -5061,15 +5066,13 @@ footer { cursor: text; } - .article-new .textBubble [contenteditable], - .article-new textarea, .articleNewEdit-body { width: 100%; position: relative; min-height: 20px; vertical-align: bottom; border: none; - background: none; + background: none !important; // overwrite [contenteditable]:hover outline: none; resize: none; } From 35ac76a6f6a3623381028a96d5e913e4be504cbb Mon Sep 17 00:00:00 2001 From: Rolf Schmidt Date: Thu, 27 Apr 2017 11:26:20 +0200 Subject: [PATCH 06/12] Removed open ticket from list of related tickets (#655). --- app/controllers/tickets_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/tickets_controller.rb b/app/controllers/tickets_controller.rb index b50a40c9e..74d913bd3 100644 --- a/app/controllers/tickets_controller.rb +++ b/app/controllers/tickets_controller.rb @@ -299,7 +299,7 @@ class TicketsController < ApplicationController } ticket_ids_recent_viewed = [] - recent_views = RecentView.list(current_user, 8, 'Ticket') + recent_views = RecentView.list(current_user, 8, 'Ticket').delete_if{|object| object['o_id'] == ticket.id } recent_views.each { |recent_view| next if recent_view['object'] != 'Ticket' ticket_ids_recent_viewed.push recent_view['o_id'] From 883c3ae18bc6fcb6f3bce13fa52771c50bdd2c5c Mon Sep 17 00:00:00 2001 From: Felix Niklas Date: Thu, 27 Apr 2017 11:28:49 +0200 Subject: [PATCH 07/12] Ticket Zoom - addon: also change background of the title on focus, see #954 --- app/assets/stylesheets/zammad.scss | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/app/assets/stylesheets/zammad.scss b/app/assets/stylesheets/zammad.scss index 83261efd9..247df3c7c 100644 --- a/app/assets/stylesheets/zammad.scss +++ b/app/assets/stylesheets/zammad.scss @@ -4526,8 +4526,11 @@ footer { padding: 0 7px; text-align: center; - .ticketZoom-header &:hover { - background: hsl(210,17%,93%); + .ticketZoom-header & { + &:hover, + &:focus { + background: hsl(210,17%,93%); + } } } From 6cd7cd3a86e7bfd56d20bef811990fcb268fa5b9 Mon Sep 17 00:00:00 2001 From: Rolf Schmidt Date: Thu, 27 Apr 2017 11:31:23 +0200 Subject: [PATCH 08/12] Tidied. --- app/controllers/tickets_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/tickets_controller.rb b/app/controllers/tickets_controller.rb index 74d913bd3..81506f393 100644 --- a/app/controllers/tickets_controller.rb +++ b/app/controllers/tickets_controller.rb @@ -299,7 +299,7 @@ class TicketsController < ApplicationController } ticket_ids_recent_viewed = [] - recent_views = RecentView.list(current_user, 8, 'Ticket').delete_if{|object| object['o_id'] == ticket.id } + recent_views = RecentView.list(current_user, 8, 'Ticket').delete_if { |object| object['o_id'] == ticket.id } recent_views.each { |recent_view| next if recent_view['object'] != 'Ticket' ticket_ids_recent_viewed.push recent_view['o_id'] From a00580322d84b3c768fbbd501aaabb2395c089ad Mon Sep 17 00:00:00 2001 From: Felix Niklas Date: Thu, 27 Apr 2017 11:53:44 +0200 Subject: [PATCH 09/12] z-index - condense z-index range: several smaller z-indexes were unused like zIndex-3,4,5,6 - push notifications to the top on z-index: 10 to prevent it from staying behind modal windows, see #655 --- app/assets/stylesheets/zammad.scss | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/app/assets/stylesheets/zammad.scss b/app/assets/stylesheets/zammad.scss index 247df3c7c..9da7f127a 100644 --- a/app/assets/stylesheets/zammad.scss +++ b/app/assets/stylesheets/zammad.scss @@ -1912,7 +1912,7 @@ input.has-error { } .selected-clue { - @extend .zIndex-9; + @extend .zIndex-6; pointer-events: none; } @@ -1920,7 +1920,7 @@ input.has-error { display: flex; align-items: center; justify-content: center; - @extend .zIndex-8; + @extend .zIndex-5; .modal-backdrop { bottom: 0; @@ -3157,7 +3157,7 @@ footer { Safari font rendering bugfix while animating http://stackoverflow.com/questions/9733011/safari-changing-font-weights-when-unrelated-animations-are-running */ - z-index: 899; // stay beneath #global-search-results + @extend .zIndex-5; // stay beneath .global-search-menu position: relative; } @@ -3331,7 +3331,7 @@ footer { .search .logo { position: relative; - @extend .u-clickable, .zIndex-5; + @extend .u-clickable, .zIndex-3; margin: -4px 10px 0 12px; transition: 240ms; } @@ -3362,13 +3362,13 @@ footer { } .global-search-menu { + @extend .zIndex-6; background: #26272e; position: absolute; left: 0; right: 0; bottom: 0; top: 53px; - z-index: 900; display: none; overflow: auto; @@ -3852,7 +3852,7 @@ footer { border: none; color: hsl(206,7%,28%); box-shadow: 0 1px 14px rgba(0,8,14,.25); - z-index: 900; + @extend .zIndex-6; } .popover-body { @@ -3955,7 +3955,7 @@ footer { min-width: 350px; position: absolute; flex-direction: column; - @extend .zIndex-5; + @extend .zIndex-2; &.is-visible { display: flex; @@ -4858,7 +4858,7 @@ footer { .editControls-item { position: absolute; top: 43px; - @extend .u-clickable, .zIndex-7; + @extend .u-clickable, .zIndex-4; &.is-hidden { display: none; @@ -5725,7 +5725,7 @@ footer { /* allow/show autocomplete in modal dialog */ .ui-autocomplete.ui-widget-content { - z-index: 1100; + @extend .zIndex-9; } .drox { @@ -5856,7 +5856,7 @@ footer { .modal { position: fixed; - z-index: 1000; + @extend .zIndex-9; } .modal-dialog { @@ -6363,7 +6363,7 @@ footer { } .scrollPageHeader { - @extend .zIndex-8; + @extend .zIndex-5; display: flex; align-items: center; background: white; @@ -8469,8 +8469,7 @@ output { } .batch-overlay { - @extend .fit; - z-index: 100; + @extend .fit, .zIndex-1; color: white; text-transform: uppercase; text-align: center; From 3ac29c8102fda3088f4f0b8777d32f906f159800 Mon Sep 17 00:00:00 2001 From: Felix Niklas Date: Thu, 27 Apr 2017 12:00:22 +0200 Subject: [PATCH 10/12] Adjust limited article width to a max of 1280px see #957 --- app/assets/stylesheets/zammad.scss | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/assets/stylesheets/zammad.scss b/app/assets/stylesheets/zammad.scss index 9da7f127a..45b25a7c2 100644 --- a/app/assets/stylesheets/zammad.scss +++ b/app/assets/stylesheets/zammad.scss @@ -4511,10 +4511,13 @@ footer { .ticket-article, .article-new { + max-width: 1280px; + margin: 0 auto; padding: 0 21px; } .ticket-title { + max-width: 1280px; padding: 0 81px; } From 5362642f78ce1e470de834c0e7abea306da007eb Mon Sep 17 00:00:00 2001 From: Felix Niklas Date: Thu, 27 Apr 2017 12:11:24 +0200 Subject: [PATCH 11/12] Limited article width: settle for a max of 1080px see #957 --- app/assets/stylesheets/zammad.scss | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/assets/stylesheets/zammad.scss b/app/assets/stylesheets/zammad.scss index 45b25a7c2..d9c25e461 100644 --- a/app/assets/stylesheets/zammad.scss +++ b/app/assets/stylesheets/zammad.scss @@ -4511,13 +4511,13 @@ footer { .ticket-article, .article-new { - max-width: 1280px; + max-width: 1080px; margin: 0 auto; padding: 0 21px; } .ticket-title { - max-width: 1280px; + max-width: 1080px; padding: 0 81px; } From a1c4c5198573a1c29f072d8e6ebd18f1546c3189 Mon Sep 17 00:00:00 2001 From: Felix Niklas Date: Thu, 27 Apr 2017 12:26:27 +0200 Subject: [PATCH 12/12] Article View - remeasure see-more when images got loaded, see #950 - add top-margin to attachments --- .../app/controllers/ticket_zoom/article_view.coffee | 8 ++++++++ app/assets/stylesheets/zammad.scss | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/app/assets/javascripts/app/controllers/ticket_zoom/article_view.coffee b/app/assets/javascripts/app/controllers/ticket_zoom/article_view.coffee index 3d7e15a54..d66ed3c15 100644 --- a/app/assets/javascripts/app/controllers/ticket_zoom/article_view.coffee +++ b/app/assets/javascripts/app/controllers/ticket_zoom/article_view.coffee @@ -73,6 +73,7 @@ class ArticleViewItem extends App.ObserverController elements: '.textBubble-content': 'textBubbleContent' + '.textBubble-content img': 'textBubbleImages' '.textBubble-overflowContainer': 'textBubbleOverflowContainer' events: @@ -209,6 +210,13 @@ class ArticleViewItem extends App.ObserverController return if @shown @shown = true + @textBubbleImages.each (i, el) => + if !el.complete + $(el).one 'load', @measureSeeMore + + @measureSeeMore() + + measureSeeMore: => maxHeight = 560 minHeight = 90 bubbleContent = @textBubbleContent diff --git a/app/assets/stylesheets/zammad.scss b/app/assets/stylesheets/zammad.scss index d9c25e461..9dacadfea 100644 --- a/app/assets/stylesheets/zammad.scss +++ b/app/assets/stylesheets/zammad.scss @@ -4763,7 +4763,7 @@ footer { .attachments.attachments--list:not(:empty) { border-top: 1px solid rgba(0,0,0,.04); white-space: normal; - margin: 0 -20px; + margin: 10px -20px 0; padding: 26px 20px 7px 72px; position: relative; }