I just installed Better Formats. When I create a role like 'admin' or 'staff' - they do not show up under Input Filters > Defaults. Only anonymous and authenticated show.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

kevinquillen’s picture

So this only works with brand new roles and not existing ones? I created my roles before BF was turned on.

kevinquillen’s picture

I had to manually insert the records into better_format_defaults to get this to work with existing roles. Is this normal?

dragonwize’s picture

Category: bug » support
Status: Active » Postponed (maintainer needs more info)

Nope. I just tested on a blank site with roles created before hand and it works fine. By the fact that thousands of people have tested this without issue and the install hasn't changed in a long time that leads me to believe that something is up with your install. Maybe your install did not finish normally. I've seen that happen before if you have a slow server with little allotted RAM to PHP and you try to install several modules at once or have a huge module page with 100's of modules.

I would suggest for you to uninstall the module and reinstall. Adding the entries by hand may cause you issues unless you follow the exact logic in the install file all the way through.

kevinquillen’s picture

The only difference I can think of is we have a install profile that automates our Drupal install and the modules we use.

kevinquillen’s picture

Could have been what you mentioned about PHP. I think there are about 60 modules in the profile. Seems to work okay, I tested a few roles.

grendzy’s picture

Category: support » bug
Status: Postponed (maintainer needs more info) » Active

I can confirm this is a real bug. Steps to reproduce:
Install Drupal (6.16)
Enable Better Formats (1.2)
Disable Better Formats
Add a role
Re-enable Better Formats

The role will be missing from /admin/settings/filters/defaults. Completely uninstalling and re-installing the module will bring it back.

The problem seems to be that new roles only come into the system via hook_form_alter(), and hook_install(). So if a role is created via install profile, Features module, or database insert, it's not known to better_formats.

I think the solution is to LEFT JOIN instead of INNER JOIN in better_formats_get_role_default_fields().

kevinquillen’s picture

Also, even if I explicitly set user permissions for anon and auth to not have Better Formats after its been installed in an install profile, its not respected. All permissions are on once I get inside the admin.

dragonwize’s picture

Priority: Normal » Minor
Status: Active » Postponed

@grendzy
Thank you for clarifying the exact steps.

No the roles get into the system through actual user hooks not form_alters. However, that matters little. Hooks are not called on disabled modules hence BF is not able to keep up with your changes. This is a wild situation where you leave the module off for a long time that you are changing the site radically. There are many modules that would not work or at the very least be buggy or filled with old dirty data when faced with the same situation. On top of that, as you said, simple reinstall the module and every thing is fine.

The proper way to fix it would be to put code in hook_enable that verifies and updates BF with any changes to site. That too would probably be some hairy areas to ensure that security holes are not opened up in the system. On top of that, only adding 1 role to the site before re-enabling the module is a simple situation to that of having removed all the roles in your site, added new roles, removed all your formats and added new ones, removed your content types and added new ones, etc. which would the extreme case but if we are to support this feature you would have to support the whole thing.

With all that being said, is it worth it to even attempt that for such little gain? I don't know. I will think about this more and make a decision before 2.0 is released.

@stripped your speech
BF sets permissions to act exactly as core does when it is first installed. If you want to change the permissions for BF, do so after not before you install the module, which seems like it shouldn't have to be said. If you are talking about removing roles from being used or monitored by BF while BF is on, that is not supported and you are on your own on that one.

kevinquillen’s picture

I am using the install_profile_api, and it installs the modules with install_include(array of modules). After that, I tried doing a db_query UPDATE with no success to change the permissions before Drupal is done installing.

grendzy’s picture

FileSize
1022 bytes

Thanks for your feedback. I agree that making changes with the module disabled is kind of extreme. That's not really the issue for me; it's just the easiest way to demonstrate it. The real problem is Drupal core lacks an API for role creation (e.g. there's no hook_role_save) so better_formats does rely on hook_form_alter to be aware of new roles:

/**
 * Creates base format default entry for a newly created role.
 *
 * @see better_formats_form_alter()
 */
function better_formats_new_role($form, &$form_state) {

The issue that I encountered was with install profiles. A simple workaround for this might be to explicitly call better_formats_new_role() every time a new role is added. However there are lots of other ways an admin might create a role without submitting the 'user_admin_new_role' form.

This patch isn't complete, but shows one possible approach (after applying the patch you'll see the missing roles on admin/settings/filters/defaults).

dragonwize’s picture

Title: Custom roles do not appear » Does not work with install profiles
Component: User interface » Code

The patch will let you see the roles but they still are not under BF's control. The site will still not give a default to users of that role because while you can see them on the page BF has no record of them. The only way this starts to work is if you then go to the page and submit the form which was the problem to begin with.

As you said, since Drupal does not have specific API hooks for roles, this is more of a limitation with Drupal an not BF. However, I would support another method if possible. Right now I am working on a 2.x version that will require (hopefully) less use of the database but to fit all the features in we still need some kind of solution.

meecect’s picture

I can confirm this is a bug. I have a simple install profile that adds 2 new roles during the install with install_add_role(), and unless I uninstall better_formats and reinstall, there is no way (that I can find) to get bf to recognize those roles.

DamienMcKenna’s picture

This issue also happens when you insert roles directly into the database (there are no real APIs for managing roles).

DamienMcKenna’s picture

Here's some sample code I wrote to fix the problem on my site:

/**
 * Implementation of hook_update_N().
 * Fix Better_Formats.
 */
function mymodule_update_6100() {
  $ret = array();

  // Make sure the better_formats module is enabled.
  if (module_exists('better_formats')) {
    // Work out a list of roles that do not have matching BF records.
    $sql = "SELECT bf.rid AS bf_rid, role.*
            FROM {better_formats_defaults} AS bf
            RIGHT OUTER JOIN {role} AS role
            ON bf.rid = role.rid AND bf.type = '%s'
            ORDER BY bf.type_weight DESC, bf.weight, role.rid";
    $types = array('node', 'comment', 'block');
  
    // Loop over each of the types.
    foreach ($types as $type) {
      $result = db_query($sql, $type);

      // Loop over each of the records.
      while ($role = db_fetch_object($result)) {
        // If there isn't a bf record, add one.
        if (!$role->bf_rid) {
          $bf_return = _mymodule_add_bf($role, $type);
          $ret = array_merge($ret, $bf_return);
        }
      }
    }
  
    $ret[] = array('success' => TRUE, 'query' => "The Better_Formats module will now work correctly for all system roles.");
  }
  
  else {
    $ret[] = array('success' => TRUE, 'query' => "The Better_Formats module has not been installed.");
  }
  
  return $ret;
}


/**
 * Create new better_formats_defaults record(s) for a given role.
 * @param $role
 *   The role object to add record(s) for.
 * @param $types
 *   Optional array of data structures to create record(s) for.  By default
 *   will create records for 'block', 'comment' and 'node'.
 */
function _mymodule_add_bf($role, $types = NULL) {
  $ret = array();

  // Make sure we have some types to deal with.
  if (empty($types)) {
    $types = array('block', 'comment', 'node');
  }
  if (!is_array($types)) {
    $types = array($types);
  }

  // Add a record for each type.
  foreach ($types as $type) {
    $bf = new StdClass();
    $bf->rid = $role->rid;
    $bf->type = $type;
    $bf->format = 0;
    $bf->type_weight = 1;
    $bf->weight = 0;
    drupal_write_record('better_formats_defaults', $bf);
    $ret[] = array('success' => TRUE, 'query' => "Added a new 'better_formats_defaults' record for {$role->name} for the {$type} form.");
  }
  
  return $ret;
}
DamienMcKenna’s picture

More code for an update script to set some intelligent defaults. Must be ran after the above.

  // Set all comment input formats to the default.
  $ret[] = update_sql("UPDATE {better_formats_defaults} SET format=0 WHERE type='comment'");
  
  // Set all node input formats to default for anonymous & authenticated users.
  $ret[] = update_sql("UPDATE {better_formats_defaults} SET format=0 WHERE type='node' AND rid IN (1, 2)");

  // Set all node input formats to Full HTML for other users.
  $ret[] = update_sql("UPDATE {better_formats_defaults} SET format=2 WHERE type='node' AND rid > 2");
dragonwize’s picture

Category: bug » feature
Priority: Minor » Normal

As mentioned before since there is no hooks for roles, a work around solution will have to be made. There are several routes to take to do it and I am debating them as they hinge on many other decisions and the 3.x branch is going to be a rewrite the final solution will probably live there.

pcambra’s picture

suscribe

alberto56’s picture

subscribing, thanks.

dragonwize’s picture

Status: Postponed » Needs review
FileSize
2.83 KB

Please review.

Since their is no perfect solution without a complete rewrite of BF and even then I am not sure all the features would make it and it will have to wait for 2.x or 3.x, I have written a possible workaround for 1.x branch.

I split out the code that creates the BF default entries from the form alter function. This allows developers of modules that are creating roles by manually inserting them into the database to be able to call a function to keep BF in sync.

If you know the rid of the role you want to sync with BF, simply call:

// Call BF to keep role in sync.
better_formats_create_role($rid);

If you do not know the rid, for instance when you manually insert the role into the DB, or when some other code that you do not have access to is creating the role(s):

// Ensure that all roles have Better Format entries in the database.
better_formats_check_roles();

If you do not have access to the code, simply visit the format settings defaults page (/admin/settings/filters/defaults) and better_formats_check_roles() will be ran by just viewing that page.

Hopefully this will help for the short term. Please review the attached patch. When/if I get a few positive reviews I will commit this and publish a new release with it. Also if you have any other ideas on how to make this better in the short term let me know.

dragonwize’s picture

FileSize
3.42 KB

Added better_formats_check_roles() to hook_enable to help with roles being "created" when the module is disabled.

hswong3i’s picture

subscribing

DamienMcKenna’s picture

Maybe it would help to add a hook_cron() function to monitor & fix this in the background?

dragonwize’s picture

I thought of that but I don't think it is a good solution.

1. BF would still be broken anyway until cron ran, whenever it ran.
2. We would be adding yet another item that is mostly useless for most sites to the cron run.

I feel there have to be better solutions. The obvious issue is in the architecture of the module which is being rewritten but for now I was hoping the items in the patch#20 will help patch up the situation for the time being.

Danny Englander’s picture

Subscribing

Dave Kopecek’s picture

This is also an issue with the adminrole module. If admin role is installed after BF, the "administrator" role create by admin role is not visible to BF. See:

http://drupal.org/node/812278 Better Formats doesn't see the admin role created.

Can confirm that disabling and uninstalling better formats, then re-enabling fixes the issue with adminrole.

Rob_Feature’s picture

#20 worked for me when creating roles using features. (had to uninstall the unpatched module and reinstall with the patch).

shrop’s picture

For my installation profile, I still had to use db_query() to insert the roles I added during installation.

  • dragonwize committed 2430ea2 on 8.x-1.x
    Fixed #731810 by dragonwize: Help make install profiles work better with...
dragonwize’s picture

Issue summary: View changes
Status: Needs review » Closed (won't fix)

6.x is now unsupported.