Working on issue #981 - Error message about insufficient LDAP configuration is confusing and should be an information instead.

This commit is contained in:
Thorsten Eckel 2017-05-10 16:43:44 +02:00
parent 75fb615f3d
commit ea926d1095
4 changed files with 77 additions and 9 deletions

View file

@ -498,9 +498,9 @@ class ConnectionWizard extends App.WizardModal
finished: true
processData: true
success: (job, status, xhr) =>
if job.result && job.result.error
if job.result && (job.result.error || job.result.info)
@showSlide('js-error')
@showAlert('js-error', job.result.error)
@showAlert('js-error', (job.result.error || job.result.info))
return
if job.result && job.result.sum

View file

@ -3,6 +3,8 @@
<% if _.isEmpty(@job.started_at): %>
<% if @job.result && @job.result.error: %>
<div class="alert alert--danger" role="alert"><%- @T('An error occurred: %s', @job.result.error) %></div>
<% else if @job.result && @job.result.info: %>
<div class="alert alert--info" role="alert"><%- @T('Info: %s', @job.result.info) %></div>
<% else: %>
<p><%- @T('Job is waiting to get started...') %></p>
<% end %>
@ -11,6 +13,8 @@
<p><%- @Ttimestamp(@job.started_at) %> - <%- @Ttimestamp(@job.finished_at) %></p>
<% if @job.result && @job.result.error: %>
<div class="alert alert--danger" role="alert"><%- @T('An error occurred: %s', @job.result.error) %></div>
<% else if @job.result && @job.result.info: %>
<div class="alert alert--info" role="alert"><%- @T('Info: %s', @job.result.info) %></div>
<% end %>
<% else: %>
<% if @job.result && @job.result.error: %>

View file

@ -28,10 +28,7 @@ module Import
#
# return [nil]
def start
if !Setting.get('ldap_integration') && !@import_job.dry_run
raise "LDAP integration deactivated, check Setting 'ldap_integration'."
end
return if !requirements_completed?
start_import
end
@ -48,5 +45,23 @@ module Import
@import_job.result = Import::Ldap::UserFactory.statistics
end
def requirements_completed?
return true if @import_job.dry_run
if !Setting.get('ldap_integration')
message = 'Sync cancelled. LDAP integration deactivated. Activate via the switch.'
elsif Setting.get('ldap_config').blank? && @import_job.payload.blank?
message = 'Sync cancelled. LDAP configration or ImportJob payload missing.'
end
return true if !message
@import_job.update_attribute(:result, {
info: message
})
false
end
end
end

View file

@ -4,7 +4,7 @@ require 'lib/import/import_job_backend_examples'
RSpec.describe Import::Ldap do
it_behaves_like 'ImportJob backend'
describe '#queueable?' do
describe '::queueable?' do
it 'is queueable if LDAP integration is activated and configured' do
allow(Setting).to receive(:get).with('ldap_integration').and_return(true)
@ -25,16 +25,65 @@ RSpec.describe Import::Ldap do
end
end
describe '.start' do
describe '#start' do
it 'starts LDAP import resource factories' do
import_job = create(:import_job)
instance = described_class.new(import_job)
allow(Setting).to receive(:get).with('ldap_integration').and_return(true)
allow(Setting).to receive(:get).with('ldap_config').and_return(true)
expect(Import::Ldap::UserFactory).to receive(:import)
instance.start
end
context 'requirements' do
it 'lets dry runs always start' do
import_job = create(:import_job, dry_run: true)
instance = described_class.new(import_job)
expect(Import::Ldap::UserFactory).to receive(:import)
instance.start
end
it 'informs about deactivated ldap_integration' do
import_job = create(:import_job)
instance = described_class.new(import_job)
allow(Setting).to receive(:get).with('ldap_integration').and_return(false)
expect(Import::Ldap::UserFactory).not_to receive(:import)
expect do
instance.start
import_job.reload
end.to change {
import_job.result
}
expect(import_job.result.key?(:info)).to be true
end
it 'informs about blank ldap_config' do
import_job = create(:import_job)
instance = described_class.new(import_job)
allow(Setting).to receive(:get).with('ldap_integration').and_return(true)
allow(Setting).to receive(:get).with('ldap_config').and_return({})
expect(Import::Ldap::UserFactory).not_to receive(:import)
expect do
instance.start
import_job.reload
end.to change {
import_job.result
}
expect(import_job.result.key?(:info)).to be true
end
end
end
end