Merge branch 'develop' into private-te-refactoring-constantize

.rubocop.yml

@@ -234,15 +234,23 @@ Style/NumericPredicate:
 
 Lint/AmbiguousBlockAssociation:
   Description: >-
-                 Checks for ambiguous block association with method when param passed without
-                 parentheses.
+                 Checks for ambiguous block association with method when param
+                 passed without parentheses.
   StyleGuide: '#syntax'
   Enabled: true
   Exclude:
     - "spec/support/*.rb"
     - "**/*_spec.rb"
     - "**/*_examples.rb"
 
+Layout/MultilineMethodCallIndentation:
+  Description: >-
+                 Checks the indentation of the method name part in method calls
+                 that span more than one line.
+  EnforcedStyle: indented
+  Include:
+    - "**/*_spec.rb"
+
 # Special exceptions
 
 Style/HashSyntax:

app/assets/javascripts/app/views/channel/email_account_overview.jst.eco

@@ -46,7 +46,9 @@
       <div class="action-block action-block--flex">
         <div class="horizontal">
           <h3><%- @Icon('status', channel.status_in + " inline") %> <%- @T('Inbound') %></h3>
-          <div class="js-editInbound btn btn--text space-left"><%- @T('Edit') %></div>
+          <% if channel.preferences.editable isnt false: %>
+            <div class="js-editInbound btn btn--text space-left"><%- @T('Edit') %></div>
+          <% end %>
         </div>
         <table class="key-value">
           <tr>
@@ -89,7 +91,9 @@
       <div class="action-block action-block--flex">
         <div class="horizontal">
           <h3><%- @Icon('status', channel.status_out + " inline") %> <%- @T('Outbound') %></h3>
-          <div class="js-editOutbound btn btn--text space-left"><%- @T('Edit') %></div>
+          <% if channel.preferences.editable isnt false: %>
+            <div class="js-editOutbound btn btn--text space-left"><%- @T('Edit') %></div>
+          <% end %>
         </div>
         <table class="key-value">
         <% if channel.options.outbound && channel.options.outbound.options: %>

app/models/channel/driver/null.rb

@@ -0,0 +1,22 @@
+# Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/
+class Channel::Driver::Null
+  def fetchable?(_channel)
+    false
+  end
+
+  def fetch(*)
+    {
+      result:  'ok',
+      fetched: 0,
+      notice:  '',
+    }
+  end
+
+  def disconnect
+    true
+  end
+
+  def self.streamable?
+    false
+  end
+end

app/models/permission.rb

@@ -31,4 +31,8 @@ def self.with_parents(key)
     names
   end
 
+  def to_s
+    name
+  end
+
 end

app/models/role.rb

@@ -9,12 +9,16 @@ class Role < ApplicationModel
   include Role::Assets
 
   has_and_belongs_to_many :users, after_add: :cache_update, after_remove: :cache_update
-  has_and_belongs_to_many :permissions, after_add: :cache_update, after_remove: :cache_update, before_update: :cache_update, after_update: :cache_update, before_add: :validate_agent_limit_by_permission, before_remove: :last_admin_check_by_permission
+  has_and_belongs_to_many :permissions,
+                          before_add:    %i[validate_agent_limit_by_permission validate_permissions],
+                          after_add:     :cache_update,
+                          before_remove: :last_admin_check_by_permission,
+                          after_remove:  :cache_update
   validates               :name,  presence: true
   store                   :preferences
 
-  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
+  before_create  :check_default_at_signup_permissions
+  before_update  :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
@@ -150,23 +154,17 @@ def self.permission_ids_by_name(keys)
 
   private
 
-  def validate_permissions
-    Rails.logger.debug { "self permission: #{self.permission_ids}" }
-    return true if !self.permission_ids
+  def validate_permissions(permission)
+    Rails.logger.debug { "self permission: #{permission.id}" }
 
-    permission_ids.each do |permission_id|
-      permission = Permission.lookup(id: permission_id)
-      raise "Unable to find permission for id #{permission_id}" if !permission
-      raise "Permission #{permission.name} is disabled" if permission.preferences[:disabled] == true
-      next if !permission.preferences[:not]
+    raise "Permission #{permission.name} is disabled" if permission.preferences[:disabled]
 
-      permission.preferences[:not].each do |local_permission_name|
-        local_permission = Permission.lookup(name: local_permission_name)
-        next if !local_permission
-        raise "Permission #{permission.name} conflicts with #{local_permission.name}" if permission_ids.include?(local_permission.id)
-      end
-    end
-    true
+    permission.preferences[:not]
+              &.find { |name| name.in?(permissions.map(&:name)) }
+              &.tap { |conflict| raise "Permission #{permission} conflicts with #{conflict}" }
+
+    permissions.find { |p| p.preferences[:not]&.include?(permission.name) }
+               &.tap { |conflict| raise "Permission #{permission} conflicts with #{conflict}" }
   end
 
   def last_admin_check_by_attribute

lib/cache.rb

@@ -25,18 +25,14 @@ def self.delete(key)
 =end
 
   def self.write(key, data, params = {})
-    if !params[:expires_in]
-      params[:expires_in] = 7.days
-    end
+    params[:expires_in] ||= 7.days
 
     # in certain cases, caches are deleted by other thread at same
     # time, just log it
-    begin
-      Rails.cache.write(key.to_s, data, params)
-    rescue => e
-      Rails.logger.error "Can't write cache #{key}: #{e.inspect}"
-      Rails.logger.error e
-    end
+    Rails.cache.write(key.to_s, data, params)
+  rescue => e
+    Rails.logger.error "Can't write cache #{key}: #{e.inspect}"
+    Rails.logger.error e
   end
 
 =begin
@@ -60,9 +56,6 @@ def self.get(key)
 =end
 
   def self.clear
-    # workaround, set test cache before clear whole cache, Rails.cache.clear complains about not existing cache dir
-    Cache.write('test', 1)
-
     Rails.cache.clear
   end
 end

spec/factories/channel.rb

@@ -8,6 +8,20 @@
     updated_by_id 1
     created_by_id 1
 
+    factory :email_channel do
+      area 'Email::Account'
+      options do
+        {
+          inbound:  {
+            adapter: 'null', options: {}
+          },
+          outbound: {
+            adapter: 'sendmail'
+          }
+        }
+      end
+    end
+
     factory :twitter_channel do
       transient do
         custom_options { {} }

spec/factories/permission.rb

@@ -0,0 +1,5 @@
+FactoryBot.define do
+  factory :permission do
+    name { Faker::Job.unique.position.downcase }
+  end
+end

spec/factories/role.rb

@@ -5,10 +5,13 @@
 end
 
 FactoryBot.define do
-
   factory :role do
     name { generate(:test_role_name) }
     created_by_id 1
     updated_by_id 1
+
+    factory :agent_role do
+      permissions { Permission.where(name: 'ticket.agent') }
+    end
   end
 end

spec/lib/cache_spec.rb

@@ -0,0 +1,100 @@
+require 'rails_helper'
+
+RSpec.describe Cache do
+  describe '.get' do
+    before { allow(Rails.cache).to receive(:read) }
+
+    it 'wraps Rails.cache.read' do
+      Cache.get('foo')
+
+      expect(Rails.cache).to have_received(:read).with('foo')
+    end
+
+    context 'with a non-string argument' do
+      it 'passes a string' do
+        Cache.get(:foo)
+
+        expect(Rails.cache).to have_received(:read).with('foo')
+      end
+    end
+  end
+
+  describe '.write' do
+    it 'stores string values' do
+      expect { Cache.write('123', 'some value') }
+        .to change { Cache.get('123') }.to('some value')
+    end
+
+    it 'stores hash values' do
+      expect { Cache.write('123', { key: 'some value' }) }
+        .to change { Cache.get('123') }.to({ key: 'some value' })
+    end
+
+    it 'overwrites previous values' do
+      Cache.write('123', 'some value')
+
+      expect { Cache.write('123', { key: 'some value' }) }
+        .to change { Cache.get('123') }.to({ key: 'some value' })
+    end
+
+    it 'stores hash values with non-ASCII content' do
+      expect { Cache.write('123', { key: 'some valueöäüß' }) }
+        .to change { Cache.get('123') }.to({ key: 'some valueöäüß' })
+    end
+
+    it 'defaults to expires_in: 7.days' do
+      Cache.write('123', 'some value')
+
+      expect { travel 7.days }.not_to change { Cache.get('123') }
+      expect { travel 1.second }.to change { Cache.get('123') }.to(nil)
+    end
+
+    it 'accepts a custom :expires_in option' do
+      Cache.write('123', 'some value', expires_in: 3.seconds)
+
+      expect { travel 4.seconds }.to change { Cache.get('123') }.to(nil)
+    end
+  end
+
+  describe '.delete' do
+    it 'deletes stored values' do
+      Cache.write('123', 'some value')
+
+      expect { Cache.delete('123') }
+        .to change { Cache.get('123') }.to(nil)
+    end
+
+    it 'is idempotent' do
+      Cache.write('123', 'some value')
+      Cache.delete('123')
+
+      expect { Cache.delete('123') }.not_to raise_error
+    end
+  end
+
+  describe '.clear' do
+    it 'deletes all stored values' do
+      Cache.write('123', 'some value')
+      Cache.write('456', 'some value')
+
+      expect { Cache.clear }
+        .to change { Cache.get('123') }.to(nil)
+        .and change { Cache.get('456') }.to(nil)
+    end
+
+    it 'is idempotent' do
+      Cache.write('123', 'some value')
+      Cache.clear
+
+      expect { Cache.clear }.not_to raise_error
+    end
+
+    context 'when cache directory is not present on disk' do
+      before { FileUtils.rm_rf(Rails.cache.cache_path) }
+
+      it 'does not raise an error' do
+        expect { Cache.clear }.not_to raise_error
+      end
+    end
+  end
+end

spec/models/http_log_spec.rb

@@ -9,11 +9,9 @@
       subject.request[:content]  = 'foo'.force_encoding('ascii-8bit')
       subject.response[:content] = 'bar'.force_encoding('ascii-8bit')
 
-      # rubocop:disable Layout/MultilineMethodCallIndentation
       expect { subject.save }
         .to change { subject.request[:content].encoding.name }.from('ASCII-8BIT').to('UTF-8')
         .and change { subject.response[:content].encoding.name }.from('ASCII-8BIT').to('UTF-8')
-      # rubocop:enable Layout/MultilineMethodCallIndentation
     end
   end
 end