Fixed issue #1488 - Role - Group can't be unchecked.

app/assets/javascripts/app/controllers/_application_controller_form.coffee

@@ -460,6 +460,31 @@ class App.ControllerForm extends App.Controller
       else
         param[item.name] = value
 
+    # verify if we have not checked checkboxes
+    uncheckParam = {}
+    lookupForm.find('input[type=checkbox]').each( (index) ->
+      checked = $(@).attr('checked')
+      name = $(@).attr('name')
+      if name && !checked && (!(name of param) || param[name] is '')
+        if !(name of uncheckParam)
+          uncheckParam[name] = undefined
+        else
+          uncheckParam[name] = []
+    )
+
+    # verify if we have not checked radios
+    lookupForm.find('input[type=radio]').each( (index) ->
+      checked = $(@).attr('checked')
+      name = $(@).attr('name')
+      if name && !checked && !(name of param)
+        uncheckParam[name] = undefined
+    )
+
+    # apply empty checkboxes & radio values to params
+    for key, value of uncheckParam
+      if !(key of param)
+        param[key] = value
+
     # data type conversion
     for key of param
 

app/assets/javascripts/app/controllers/_channel/chat.coffee

@@ -348,7 +348,7 @@ class App.ChannelChat extends App.ControllerSubContent
         params[key] = value
     paramString = ''
     for key, value of params
-      if value != ''
+      if !_.isEmpty(value)
         if paramString != ''
           # coffeelint: disable=no_unnecessary_double_quotes
           paramString += ",\n"

app/assets/javascripts/app/controllers/_channel/form.coffee

@@ -55,7 +55,7 @@ class App.ChannelForm extends App.ControllerSubContent
     params = @formParam(@$('.js-paramsDesigner'))
     paramString = ''
     for key, value of params
-      if value != ''
+      if !_.isEmpty(value)
         if paramString != ''
           paramString += ",\n"
         if value == 'true' || value == 'false'

app/models/application_model/can_associations.rb

@@ -22,8 +22,8 @@ returns
       groups:    :group_names_access_map=,
       group_ids: :group_ids_access_map=
     }.each do |param, setter|
+      next if !params.key?(param)
       map = params[param]
-      next if map.blank?
       next if !respond_to?(setter)
       send(setter, map)
     end

app/models/concerns/has_groups.rb

@@ -7,8 +7,8 @@ module HasGroups
 
     attr_accessor :group_access_buffer
 
-    after_create :check_group_access_buffer
+    after_create :process_group_access_buffer
-    after_update :check_group_access_buffer
+    after_update :process_group_access_buffer
 
     association_attributes_ignored :groups, group_through_identifier
 
@@ -211,35 +211,49 @@ module HasGroups
   end
 
   def groups_access_map_store(map)
-    map.each do |group_identifier, accesses|
+    fill_group_access_buffer do
+      Hash(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)
+        Array(accesses).each do |access|
-        accesses = [accesses]
-      end
-
-      accesses.each do |access|
           push_group_access_buffer(
             group_id: group_id,
             access:   access
           )
         end
       end
+    end
+  end
 
-    check_group_access_buffer if id
+  def fill_group_access_buffer
+    @group_access_buffer = []
+    yield
+    process_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
+  def flush_group_access_buffer
-    return if group_access_buffer.blank?
+    # 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
+
+  def process_group_access_buffer
+
+    flush_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
@@ -247,10 +261,8 @@ module HasGroups
       end
 
       group_through.klass.create!(entries)
+    end
 
-    group_access_buffer = nil
-
-    cache_delete
     true
   end
 

public/assets/tests/form.js

@@ -24,6 +24,8 @@ test("form elements check", function() {
     selectmultioption2: [ false, true ],
     richtext2: 'lalu <l> lalu',
     datetime1: Date.parse('2015-01-11T12:40:00Z'),
+    checkbox1: [],
+    checkbox2: '1',
   }
   new App.ControllerForm({
     el:        el,
@@ -45,6 +47,8 @@ test("form elements check", function() {
         { name: 'richtext2', display: 'Richtext2', tag: 'richtext', limit: 100, null: true, upload: true, default: defaults['richtext2'] },
         { name: 'datetime1', display: 'Datetime1', tag: 'datetime', null: true, default: defaults['datetime1'] },
         { name: 'datetime2', display: 'Datetime2', tag: 'datetime', null: false, default: defaults['datetime2'] },
+        { name: 'checkbox1', display: 'Checkbox1', tag: 'checkbox', null: false, default: defaults['checkbox1'], options: { a: 'AA', b: 'BB' } },
+        { name: 'checkbox2', display: 'Checkbox2', tag: 'checkbox', null: false, default: defaults['checkbox2'], options: { 1: '11' } },
       ]
     },
     autofocus: true
@@ -100,6 +104,10 @@ test("form elements check", function() {
   //equal(el.find('[name="richtext2"]').prop('required'), true, 'check textarea2 required')
   equal(el.find('[name="richtext2"]').is(":focus"), false, 'check textarea2 focus')
 
+  equal(el.find('[name="checkbox1"]').first().is(":checked"), false)
+  equal(el.find('[name="checkbox1"]').last().is(":checked"), false)
+  equal(el.find('[name="checkbox2"]').is(":checked"), true)
+
 });
 
 test("form params check", function() {
@@ -134,6 +142,11 @@ test("form params check", function() {
     date3:     '2015-01-11',
     active1:   true,
     active2:   false,
+    checkbox1: [],
+    checkbox2: undefined,
+    checkbox3: 'd',
+    radiobox1: undefined,
+    radiobox2: 'a',
   }
   new App.ControllerForm({
     el:        el,
@@ -173,6 +186,12 @@ test("form params check", function() {
         { name: 'date4',     display: 'Date4',     tag: 'date', null: false, default: defaults['date4'] },
         { name: 'active1',   display: 'Active1',   tag: 'active', default: defaults['active1'] },
         { name: 'active2',   display: 'Active2',   tag: 'active', default: defaults['active2'] },
+        { name: 'checkbox1', display: 'Checkbox1', tag: 'checkbox', null: false, default: defaults['checkbox1'], options: { a: 'AA', b: 'BB' } },
+        { name: 'checkbox2', display: 'Checkbox2', tag: 'checkbox', null: false, default: defaults['checkbox2'], options: { 1: '11' } },
+        { name: 'checkbox3', display: 'Checkbox3', tag: 'checkbox', null: false, default: defaults['checkbox3'], options: { c: 'CC', d: 'DD' } },
+        { name: 'checkbox4', display: 'Checkbox4', tag: 'checkbox', null: false, default: defaults['checkbox4'], options: { aa: 'AA', bb: 'BB' } },
+        { name: 'radiobox1', display: 'Radiobox1', tag: 'radio', null: false, default: defaults['radiobox1'], options: { a: 'AA', b: 'BB' } },
+        { name: 'radiobox2', display: 'Radiobox2', tag: 'radio', null: false, default: defaults['radiobox2'], options: { a: '11' } },
       ],
     },
     params: defaults,
@@ -271,6 +290,12 @@ test("form params check", function() {
     date4: undefined,
     active1: true,
     active2: false,
+    checkbox1: [],
+    checkbox2: undefined,
+    checkbox3: 'd',
+    checkbox4: [],
+    radiobox1: undefined,
+    radiobox2: 'a',
   }
   deepEqual(params, test_params, 'form param check')
 

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<String> 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