diff --git a/app/models/application_model/can_associations.rb b/app/models/application_model/can_associations.rb index 2a625f19e..aca230bc8 100644 --- a/app/models/application_model/can_associations.rb +++ b/app/models/application_model/can_associations.rb @@ -23,7 +23,6 @@ returns group_ids: :group_ids_access_map= }.each do |param, setter| map = params[param] - next if map.blank? next if !respond_to?(setter) send(setter, map) end diff --git a/app/models/concerns/has_groups.rb b/app/models/concerns/has_groups.rb index 84fd3bbbe..8780d3bcf 100644 --- a/app/models/concerns/has_groups.rb +++ b/app/models/concerns/has_groups.rb @@ -211,46 +211,62 @@ module HasGroups end def groups_access_map_store(map) - map.each do |group_identifier, accesses| - # use given key as identifier or look it up - # via the given block which returns the identifier - group_id = block_given? ? yield(group_identifier) : group_identifier + fill_group_access_buffer do + map.each do |group_identifier, accesses| + # use given key as identifier or look it up + # via the given block which returns the identifier + group_id = block_given? ? yield(group_identifier) : group_identifier - if !accesses.is_a?(Array) - accesses = [accesses] - end + if !accesses.is_a?(Array) + accesses = [accesses] + end - accesses.each do |access| - push_group_access_buffer( - group_id: group_id, - access: access - ) + accesses.each do |access| + push_group_access_buffer( + group_id: group_id, + access: access + ) + end end end + end + def fill_group_access_buffer + @group_access_buffer = [] + yield check_group_access_buffer if id end def push_group_access_buffer(entry) - @group_access_buffer ||= [] @group_access_buffer.push(entry) end - def check_group_access_buffer - return if group_access_buffer.blank? - destroy_group_relations + def flushed_group_access_buffer + # group_access_buffer is at least an empty Array + # if changes to the map were performed + # otherwise it's just an update of other attributes + return if group_access_buffer.nil? + yield + group_access_buffer = nil + cache_delete + end - foreign_key = group_through.foreign_key - entries = group_access_buffer.collect do |entry| - entry[foreign_key] = id - entry + def check_group_access_buffer + + flushed_group_access_buffer do + destroy_group_relations + + break if group_access_buffer.blank? + + foreign_key = group_through.foreign_key + entries = group_access_buffer.collect do |entry| + entry[foreign_key] = id + entry + end + + group_through.klass.create!(entries) end - group_through.klass.create!(entries) - - group_access_buffer = nil - - cache_delete true end diff --git a/spec/models/concerns/has_groups_examples.rb b/spec/models/concerns/has_groups_examples.rb index f8bc0beb1..3b2ddaee2 100644 --- a/spec/models/concerns/has_groups_examples.rb +++ b/spec/models/concerns/has_groups_examples.rb @@ -229,6 +229,19 @@ RSpec.shared_examples 'HasGroups' do described_class.group_through.klass.count }.by(3) end + + it 'allows empty Hash value' do + group_access_instance.group_names_access_map = { + group_full.name => 'full', + group_read.name => %w[read change], + } + + expect do + group_access_instance.group_names_access_map = {} + end.to change { + described_class.group_through.klass.count + }.by(-3) + end end context 'new instance' do @@ -256,6 +269,16 @@ RSpec.shared_examples 'HasGroups' do described_class.group_through.klass.count }.by(2) end + + it 'allows empty Hash value' do + expect do + new_group_access_instance.group_names_access_map = {} + + new_group_access_instance.save + end.not_to change { + described_class.group_through.klass.count + } + end end end @@ -284,6 +307,18 @@ RSpec.shared_examples 'HasGroups' do expect(group_access_instance_inactive.group_names_access_map).to be_empty end + + it 'returns empty map if none is stored' do + + group_access_instance.group_names_access_map = { + group_full.name => 'full', + group_read.name => 'read', + } + + group_access_instance.group_names_access_map = {} + + expect(group_access_instance.group_names_access_map).to be_blank + end end context '#group_ids_access_map=' do @@ -305,7 +340,7 @@ RSpec.shared_examples 'HasGroups' do }.by(2) end - it 'stores Hash with String values' do + it 'stores Hash with Array values' do expect do group_access_instance.group_ids_access_map = { group_full.id => 'full', @@ -315,6 +350,19 @@ RSpec.shared_examples 'HasGroups' do described_class.group_through.klass.count }.by(3) end + + it 'allows empty Hash value' do + group_access_instance.group_ids_access_map = { + group_full.id => 'full', + group_read.id => %w[read change], + } + + expect do + group_access_instance.group_ids_access_map = {} + end.to change { + described_class.group_through.klass.count + }.by(-3) + end end context 'new instance' do @@ -342,6 +390,16 @@ RSpec.shared_examples 'HasGroups' do described_class.group_through.klass.count }.by(2) end + + it 'allows empty Hash value' do + expect do + new_group_access_instance.group_ids_access_map = {} + + new_group_access_instance.save + end.not_to change { + described_class.group_through.klass.count + } + end end end @@ -370,6 +428,18 @@ RSpec.shared_examples 'HasGroups' do expect(group_access_instance_inactive.group_ids_access_map).to be_empty end + + it 'returns empty map if none is stored' do + + group_access_instance.group_ids_access_map = { + group_full.id => 'full', + group_read.id => 'read', + } + + group_access_instance.group_ids_access_map = {} + + expect(group_access_instance.group_ids_access_map).to be_blank + end end context '#associations_from_param' do