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 ]


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);

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.

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.

Title:Make enabling/disabling a role an independent option on the role configuration pageRedesign 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.

new83.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.

Status:Active» Needs review

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.

Status:Needs review» Fixed

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

Issue summary:View changes

Updated issue summary.