Preserve checkbox selection in user search dialogs (#43825).

Patch by [Agileware]Kota Uchino (user:uchinokot).


git-svn-id: https://svn.redmine.org/redmine/trunk@24572 e93f8b46-1217-0410-a6f0-8f06a7374b81

app/assets/javascripts/application-legacy.js

@@ -796,22 +796,85 @@ function multipleAutocompleteField(fieldId, url, options) {
   });
 }
 
-function observeSearchfield(fieldId, targetId, url) {
+function observeSearchfield(fieldId, targetId, url, options) {
   $('#'+fieldId).each(function() {
     var $this = $(this);
     $this.addClass('autocomplete');
     $this.attr('data-value-was', $this.val());
+    var checkedValues = {};
+    var cbSelector = options && options.checkboxSelector;
+    var $form = cbSelector ? $this.closest('form') : null;
+    function checkboxName() {
+      if (!cbSelector) return null;
+      return $form.find(cbSelector).first().attr('name');
+    }
+
+    function saveChecked() {
+      if (!cbSelector) return;
+      $form.find(cbSelector).not('.hidden-checked-value').each(function() {
+        if ($(this).prop('checked')) {
+          checkedValues[$(this).val()] = true;
+        } else {
+          delete checkedValues[$(this).val()];
+        }
+      });
+    }
+
+    function restoreChecked() {
+      if (!cbSelector) return;
+      // Restore checkboxes that are visible in the current page
+      $form.find(cbSelector).not('.hidden-checked-value').each(function() {
+        if (checkedValues[$(this).val()]) {
+          $(this).prop('checked', true);
+        }
+      });
+      // Sync hidden inputs for checked values not visible as checkboxes
+      $form.find('input.hidden-checked-value').remove();
+      var cbName = checkboxName();
+      if (!cbName) return;
+      $.each(checkedValues, function(val) {
+        if ($form.find(cbSelector + '[value="' + val + '"]').length === 0) {
+          $form.append(
+            $('<input type="hidden" class="hidden-checked-value">').attr('name', cbName).val(val)
+          );
+        }
+      });
+    }
+
+    if (cbSelector) {
+      // Track checkbox changes via delegation
+      $form.on('change', cbSelector, function() {
+        if ($(this).prop('checked')) {
+          checkedValues[$(this).val()] = true;
+        } else {
+          delete checkedValues[$(this).val()];
+        }
+        restoreChecked();
+      });
+      // Handle pagination (remote links replacing content)
+      $form.on('ajax:before', 'a[data-remote]', function() {
+        saveChecked();
+      });
+      $form.on('ajax:complete', 'a[data-remote]', function() {
+        restoreChecked();
+      });
+    }
+
     var check = function() {
       var val = $this.val();
       if ($this.attr('data-value-was') != val){
         $this.attr('data-value-was', val);
+        saveChecked();
         $.ajax({
           url: url,
           type: 'get',
           data: {q: $this.val()},
           success: function(data){ if(targetId) $('#'+targetId).html(data); },
           beforeSend: function(){ $this.addClass('ajax-loading'); },
-          complete: function(){ $this.removeClass('ajax-loading'); }
+          complete: function(){
+            $this.removeClass('ajax-loading');
+            restoreChecked();
+          }
         });
       }
     };

app/views/groups/_new_users_form.html.erb

@@ -1,7 +1,7 @@
 <fieldset class="box">
   <legend><%= label_tag "user_search", l(:label_user_search) %></legend>
   <p><%= text_field_tag 'user_search', nil %></p>
-  <%= javascript_tag "observeSearchfield('user_search', null, '#{ escape_javascript autocomplete_for_user_group_path(@group) }')" %>
+  <%= javascript_tag "observeSearchfield('user_search', null, '#{ escape_javascript autocomplete_for_user_group_path(@group) }', {checkboxSelector: 'input[name=\"user_ids[]\"]'})" %>
 
   <div id="users">
     <%= render_principals_for_new_group_users(@group) %>

app/views/members/_new_form.html.erb

@@ -1,7 +1,7 @@
 <fieldset class="box">
   <legend><%= label_tag("principal_search", l(:label_principal_search)) %></legend>
   <p><%= text_field_tag('principal_search', nil) %></p>
-  <%= javascript_tag "observeSearchfield('principal_search', null, '#{ escape_javascript autocomplete_project_memberships_path(@project, :format => 'js') }')" %>
+  <%= javascript_tag "observeSearchfield('principal_search', null, '#{ escape_javascript autocomplete_project_memberships_path(@project, :format => 'js') }', {checkboxSelector: 'input[name=\"membership[user_ids][]\"]'})" %>
   <div id="principals_for_new_member">
     <%= render_principals_for_new_members(@project) %>
   </div>

app/views/watchers/_new.html.erb

@@ -34,7 +34,8 @@ title =
                  :object_id => (watchables.present? ? watchables.map(&:id) : nil),
                  :project_id => @project
                )
-             )}'
+             )}',
+          {checkboxSelector: 'input[name=\"watcher[user_ids][]\"]'}
          )"
        ) %>
   <div id="users_for_watcher">

test/system/groups_test.rb

@@ -0,0 +1,163 @@
+# frozen_string_literal: true
+
+# Redmine - project management software
+# Copyright (C) 2006-  Jean-Philippe Lang
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+
+require_relative '../application_system_test_case'
+
+class GroupsSystemTest < ApplicationSystemTestCase
+  fixtures :users, :email_addresses, :groups_users
+
+  def test_add_user_to_group_after_search_preserves_selection
+    group = Group.find(10)  # A Team
+    rhill = User.find_by_login('rhill')
+    someone = User.find_by_login('someone')
+
+    assert_not group.users.include?(rhill)
+    assert_not group.users.include?(someone)
+
+    log_user('admin', 'admin')
+    visit "/groups/#{group.id}/edit?tab=users"
+
+    click_on 'New user'
+
+    within '#ajax-modal' do
+      # Check Robert Hill
+      find('label', text: 'Robert Hill').click
+
+      # Search for 'Some One' - only Some One should remain
+      fill_in 'user_search', with: 'Some One'
+      assert page.has_no_css?('label', text: 'Robert Hill')
+
+      # Check Some One while Robert Hill is hidden
+      find('label', text: 'Some One').click
+
+      # Clear search - both should be visible and Robert Hill should still be checked
+      fill_in 'user_search', with: ''
+      assert page.has_css?('label', text: 'Robert Hill')
+      assert page.has_css?('label', text: 'Some One')
+
+      # Submit
+      click_on 'Add'
+    end
+
+    # Wait for modal to close
+    assert page.has_no_css?('#ajax-modal')
+
+    # Verify both users were added to the group
+    group.reload
+    assert group.users.include?(rhill),
+      "Expected Robert Hill to be added to group"
+    assert group.users.include?(someone),
+      "Expected Some One to be added to group"
+  end
+
+  def test_add_user_to_group_submitting_while_filtered
+    group = Group.find(10)  # A Team
+    rhill = User.find_by_login('rhill')
+    someone = User.find_by_login('someone')
+
+    assert_not group.users.include?(rhill)
+    assert_not group.users.include?(someone)
+
+    log_user('admin', 'admin')
+    visit "/groups/#{group.id}/edit?tab=users"
+
+    click_on 'New user'
+
+    within '#ajax-modal' do
+      # Check Robert Hill
+      find('label', text: 'Robert Hill').click
+
+      # Search for 'Some One' - Robert Hill disappears
+      fill_in 'user_search', with: 'Some One'
+      assert page.has_no_css?('label', text: 'Robert Hill')
+
+      # Check Some One
+      find('label', text: 'Some One').click
+
+      # Verify hidden input exists for Robert Hill before submitting
+      rhill_id = rhill.id.to_s
+      hidden_values = page.evaluate_script(
+        "$('form input.hidden-checked-value').map(function(){return $(this).val()}).get()"
+      )
+      assert_includes hidden_values, rhill_id,
+        "Expected hidden input for Robert Hill (id=#{rhill_id}) but got: #{hidden_values.inspect}"
+
+      # Submit without clearing search
+      click_on 'Add'
+    end
+
+    assert page.has_no_css?('#ajax-modal')
+
+    # Both users should be added even though Robert Hill was not visible
+    group.reload
+    assert group.users.include?(rhill),
+      "Expected Robert Hill to be added to group"
+    assert group.users.include?(someone),
+      "Expected Some One to be added to group"
+  end
+
+  def test_unchecked_user_should_not_be_rechecked_after_search
+    group = Group.find(10)  # A Team
+    rhill = User.find_by_login('rhill')
+
+    assert_not group.users.include?(rhill)
+
+    log_user('admin', 'admin')
+    visit "/groups/#{group.id}/edit?tab=users"
+
+    click_on 'New user'
+
+    within '#ajax-modal' do
+      # Step 2: Check Robert Hill
+      find('label', text: 'Robert Hill').click
+
+      # Step 3: Search that hides Robert Hill
+      fill_in 'user_search', with: 'Some One'
+      assert page.has_no_css?('label', text: 'Robert Hill')
+
+      # Step 4: Clear search - Robert Hill reappears and should be checked
+      fill_in 'user_search', with: ''
+      assert page.has_css?('label', text: 'Robert Hill')
+      rhill_cb = find('label', text: 'Robert Hill').find('input[type=checkbox]', visible: :all)
+      assert rhill_cb.checked?, "Robert Hill should be checked after clearing search"
+
+      # Step 5: Uncheck Robert Hill
+      find('label', text: 'Robert Hill').click
+      rhill_cb = find('label', text: 'Robert Hill').find('input[type=checkbox]', visible: :all)
+      assert_not rhill_cb.checked?, "Robert Hill should be unchecked after clicking again"
+
+      # Step 6: Search that matches Robert Hill
+      fill_in 'user_search', with: 'Robert Hill'
+      assert page.has_css?('label', text: 'Robert Hill')
+
+      # Step 7: Robert Hill should still be unchecked
+      rhill_cb = find('label', text: 'Robert Hill').find('input[type=checkbox]', visible: :all)
+      assert_not rhill_cb.checked?,
+        "Robert Hill should NOT be re-checked after search refresh"
+
+      # Also verify no hidden input exists for Robert Hill
+      rhill_id = rhill.id.to_s
+      hidden_values = page.evaluate_script(
+        "$('form input.hidden-checked-value').map(function(){return $(this).val()}).get()"
+      )
+      assert_not_includes hidden_values, rhill_id,
+        "No hidden input should exist for unchecked Robert Hill"
+    end
+  end
+end

test/system/members_test.rb

@@ -0,0 +1,76 @@
+# frozen_string_literal: true
+
+# Redmine - project management software
+# Copyright (C) 2006-  Jean-Philippe Lang
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+
+require_relative '../application_system_test_case'
+
+class MembersSystemTest < ApplicationSystemTestCase
+  fixtures :projects, :users, :email_addresses, :roles, :members, :member_roles,
+           :trackers, :projects_trackers, :enabled_modules
+
+  def test_add_member_after_search_preserves_selection
+    project = Project.find('ecookbook')
+    rhill = User.find_by_login('rhill')
+    admin_user = User.find_by_login('admin')
+
+    assert_not project.members.map(&:user).include?(rhill)
+    assert_not project.members.map(&:user).include?(admin_user)
+
+    log_user('admin', 'admin')
+    visit '/projects/ecookbook/settings/members'
+
+    click_on 'New member'
+
+    within '#ajax-modal' do
+      # Check Robert Hill
+      find('label', text: 'Robert Hill').click
+
+      # Search for 'Redmine Admin' - only Redmine Admin should remain
+      fill_in 'principal_search', with: 'Redmine Admin'
+      assert page.has_no_css?('label', text: 'Robert Hill')
+
+      # Check Redmine Admin while Robert Hill is hidden
+      find('label', text: 'Redmine Admin').click
+
+      # Clear search - both should be visible and Robert Hill should still be checked
+      fill_in 'principal_search', with: ''
+      assert page.has_css?('label', text: 'Robert Hill')
+      assert page.has_css?('label', text: 'Redmine Admin')
+
+      # Select a role
+      check 'Manager'
+
+      # Submit
+      click_on 'Add'
+    end
+
+    # Wait for modal to close and page to update
+    assert page.has_no_css?('#ajax-modal')
+
+    # Verify both users were added as members
+    project.reload
+    assert project.members.map(&:user).include?(rhill),
+      "Expected Robert Hill to be added as member"
+    assert project.members.map(&:user).include?(admin_user),
+      "Expected Redmine Admin to be added as member"
+
+    # Verify on the page
+    assert page.has_css?('#tab-content-members', text: 'Robert Hill')
+    assert page.has_css?('#tab-content-members', text: 'Redmine Admin')
+  end
+end

test/system/watchers_test.rb

@@ -0,0 +1,76 @@
+# frozen_string_literal: true
+
+# Redmine - project management software
+# Copyright (C) 2006-  Jean-Philippe Lang
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+
+require_relative '../application_system_test_case'
+
+class WatchersSystemTest < ApplicationSystemTestCase
+  fixtures :projects, :users, :email_addresses, :roles, :members, :member_roles,
+           :trackers, :projects_trackers, :enabled_modules, :issues, :issue_statuses,
+           :watchers
+
+  def test_add_watcher_after_search_preserves_selection
+    issue = Issue.find(1)
+    jsmith = User.find_by_login('jsmith')
+    dlopper = User.find_by_login('dlopper')
+
+    # Clear existing watchers
+    issue.watcher_users = []
+
+    assert_not issue.watched_by?(jsmith)
+    assert_not issue.watched_by?(dlopper)
+
+    log_user('admin', 'admin')
+    visit "/issues/#{issue.id}"
+
+    # Open watcher modal
+    within '#sidebar' do
+      click_on 'Add'
+    end
+
+    within '#ajax-modal' do
+      # Check John Smith
+      find('label', text: 'John Smith').click
+
+      # Search for 'Dave Lopper' - only Dave Lopper should remain
+      fill_in 'user_search', with: 'Dave Lopper'
+      assert page.has_no_css?('label', text: 'John Smith')
+
+      # Check Dave Lopper while John Smith is hidden
+      find('label', text: 'Dave Lopper').click
+
+      # Clear search - both should be visible and John Smith should still be checked
+      fill_in 'user_search', with: ''
+      assert page.has_css?('label', text: 'John Smith')
+      assert page.has_css?('label', text: 'Dave Lopper')
+
+      # Submit
+      click_on 'Add'
+    end
+
+    # Wait for AJAX to complete and sidebar to update
+    assert page.has_css?('#sidebar', text: 'John Smith', wait: 5)
+
+    # Verify both users were added as watchers
+    issue.reload
+    assert issue.watched_by?(jsmith),
+      "Expected John Smith to be added as watcher"
+    assert issue.watched_by?(dlopper),
+      "Expected Dave Lopper to be added as watcher"
+  end
+end