Currently, access control for a role is automatically enabled when the user visits admin/config/people/taxonomy_access/role/%/edit. Instead, we should make this an independent operation, and provide the link to enable or disable at the top of admin/config/people/taxonomy_access/role/%/edit. If the role is enabled, the disable link should be at the top; if the role is disabled, they should see only the enable link and possibly some help text.

As part of this change, the enable/disable links on admin/config/people/taxonomy_access should be removed, and the "edit" link there should be changed to "configure."

#5 role_config.patch83.26 KBxjm
PASSED: [[SimpleTest]]: [MySQL] 2,250 pass(es). View
Members fund testing for the Drupal project. Drupal Association Learn more


pwolanin’s picture

Here's a quick pass at what a simple API function for enabling a role could look like:

function taxonomy_access_enable_role($rid) {
  $rid = intval($rid);
  // Fetch all default grants.
  $enabled = db_query('SELECT 1 FROM {taxonomy_access_default} WHERE rid = :rid AND vid = :vid', array(':rid' => $rid, ':vid' => '0'))->fetchField();

  // If we are adding a role, no global default is set yet, so insert it now.
  // All valid role IDs are > 0.
  if ($rid && !$enabled) {
    // Assemble a $row object for Schema API.
    $row = new stdClass();
    $row->vid = 0;
    $row->rid = $rid;

    // Insert the row with defaults for all grants.
    drupal_write_record('taxonomy_access_default', $row);
xjm’s picture

Assigned: Unassigned » xjm

I realized that I actually have to rework the entire admin form in order to implement this (and part of it may predate FAPI...) So it will take a little more time, and not a novice issue.

xjm’s picture

The API function has been added in, but I ended up rolling back the user-facing part of the change because it was just too messy. The form theming is likely due to be modernized anyway.

xjm’s picture

Title: Make enabling/disabling a role an independent option on the role configuration page » Redesign role configuration page

Broadening the scope here because I have to change half the form anyway to do this. Dratted legacy code.

  • Refactor role configuration form generation and theming to not be a pain in the ass.
  • Add a fieldset to enable or disable a role at the top of the role configuration form.
  • Change table on main configuration page to just have a "configure" link under "operations."
  • Place global default in a separate fieldset.
  • Make presence/absence of a vocabulary default control whether the vocabulary is enabled.
  • Add a hook_update_N() to insert a vocab. default matching the global default for vocabs with term records in {taxonomy_access_term}.
  • Add a fieldset to enable a vocabulary. (In the mockups each vocab. has its own fieldset regardless, but I tested this with the taxonomy from one of my production sites, and, holy scrolling Batman.)
  • Display form for each enabled vocabulary in its own fieldset.
  • Provide API for rendering the grant tables.
xjm’s picture

83.26 KB
PASSED: [[SimpleTest]]: [MySQL] 2,250 pass(es). View

Alright, lest perfect persist in being the enemy of the good, here's the patch from the role_config branch as it's been for the past month or so. There's a few outstanding improvements I'd like to make and test coverage I'd like to add, but I'll likely spin those things off as followup issues. The patch is fully functional.

xjm’s picture

Status: Active » Needs review
xjm’s picture

Couple quick code style notes:

+++ b/taxonomy_access.admin.incundefined
@@ -121,32 +104,57 @@ function taxonomy_access_admin_delete_role_submit($form, &$form_state) {
+ * Path: TAXONOMY_ACCESS_CONFIG . /role/%/enable

@@ -159,308 +167,623 @@ function taxonomy_access_admin_build_row(array $grants = NULL) {
+ * Path: TAXONOMY_ACCESS_CONFIG . '/role/%/disable/%taxonomy_vocabulary

This line has been removed from the menu callback standard, and should be stripped.

+++ b/taxonomy_access.admin.incundefined
@@ -159,308 +167,623 @@ function taxonomy_access_admin_build_row(array $grants = NULL) {
+ * Theme our grant table.

Could use an actual summary.

xjm’s picture

Status: Needs review » Fixed

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

Issue summary: View changes

Updated issue summary.