Problem/Motivation

With a larger vocabulary where each user has multiple allowed terms selected in their user edit form if a site manager accidentally doesn't hold down the Ctrl or Cmd key when clicking on a new term they're adding, all the terms that were previously selected get lost. This is a nightmare for sites with lots of users and lots of terms.

Steps to reproduce

1. Have a vocabulary with many terms.
2. Edit the user to add a new term.
3. Accidentally click on the new term without holding the Ctrl/Cmd key.

Proposed resolution

Create an option to output the list of terms as Checkboxes so that each term is independently an on/off check and will not be affected by other terms.

CommentFileSizeAuthor
#8 before.png33.75 KBdivya.sejekan
#2 3397350-2.patch4.29 KBsidharth_soman

Issue fork permissions_by_term-3397350

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

marty.true created an issue. See original summary.

sidharth_soman’s picture

sidharth_soman commented
StatusFileSize
new 3397350-2.patch4.29 KB

I have introduced an option for choosing the checkboxes output in the module's config and also a conditional for outputting the terms in checkboxes in the user form.

Please apply the patch and review.

marty.true’s picture

marty.true commented

The patch applied cleanly, and the new option works a treat! Thank you for addressing this issue — this option makes the module much more feasible now.

vitaliyb98 made their first commit to this issue’s fork.

vitaliyb98 opened merge request !66

vitaliyb98’s picture

vitaliyb98 commented
Status: Active » Needs review

Improved code provided in the patch.
Fixed error with unsetting the checkbox:

TypeError: Drupal\permissions_by_term\Service\AccessStorage::addTermPermissionsByUserIds(): Argument #2 ($termId) must be of type string, int given, called in /var/www/html/modules/permissions_by_term/permissions_by_term.module on line 339 in Drupal\permissions_by_term\Service\AccessStorage->addTermPermissionsByUserIds() (line 274 of /var/www/html/modules/permissions_by_term/src/Service/AccessStorage.php).

Added functional tests for this functionality.

vitaliyb98’s picture

vitaliyb98 commented

I would appreciate it if someone else could also test this solution.

divya.sejekan’s picture

divya.sejekan commented
Status: Needs review » Needs work
StatusFileSize
new before.png33.75 KB

Tested the MR!66 , The fix works
If checked the "Display the terms in checkboxes" - checkbox in /admin/permissions-by-term/settings . The Terms are displayed as check boxes in the user account settings page

The Fix is working as expected . But the MR looks like it has merge issue . Changing it to need work , for someone to fix the merge error

vitaliyb98’s picture

vitaliyb98 commented
Status: Needs work » Needs review

@divya.sejekan, Thank you for reviewing and testing. I've resolved the merge conflict, but will keep the issue in Needs review, hoping that someone else can also verify it.

dhruv.mittal’s picture

dhruv.mittal commented

Reviewing it

damienmckenna’s picture

damienmckenna
Location TN, USA
commented

Instead of naming the variable "output_checkboxes" maybe name it "selector_style" and have two values - "checkboxes" and "select", with it defaulting to "select"?

vitaliyb98’s picture

vitaliyb98 commented
Status: Needs review » Needs work

@damienmckenna Thanks for the feedback. I agree, it's a good idea.