Currently we seem to query for all objects (roles and policies). That's a bug, because policies defined in the default hook will never be taken into consideration then.

We should use proper CTools API functions to load all objects.
I might get some time to look deeper into this.

Comments

erikwebb’s picture

erikwebb commented
Status: Active » Postponed (maintainer needs more info)

Should we be using CTools API functions here or just a simple array_merge()? I'm trying to keep CTools out of the 7.x-1.x branch for now.

dixon_’s picture

dixon_
he/him
Primary language English
Location France
commented
Status: Postponed (maintainer needs more info) » Active

There's not much to leave out, it's more or less one function that we need to start utilizing instead of custom db queries and array_merge.

Basically it's ctools_export_load_object() that we need to utilize. That function will query the table for objects that are not exported, and merge in default ones. Doing it our selfs with our own array_merge is just to complicate it I think.

This will add a hard dependency on CTools but practically I don't think that's a problem since essentially everyone has CTools installed with Views these days.

dixon_’s picture

dixon_
he/him
Primary language English
Location France
commented

Here's roughly what it would look like:


/**
 * Loader callback for all password policies.
 */
function password_policy_load_all() {
  ctools_include('export');
  return ctools_export_load_object('password_policy', 'all');
}

It's quite a simple patch to add this in. So I'll try to find some time for this.

erikwebb’s picture

erikwebb commented

I'm worried about adding in a CTools dependency at this point. I have a pattern that already does this without CTools (http://erikwebb.net/blog/drupal-design-patterns/putting-it-together) that maybe could be used instead.

I'm planning on rewriting 7.x-2.x to be fully CTools-dependent with exportables, plugins, etc.

dixon_’s picture

dixon_
he/him
Primary language English
Location France
commented

Seems like a doable approach. But you've already gone down the path choosing CTools to export the object. Why not complete that path? Practically I don't think it's a problem to depend on CTools. I'd say that 99,9% of all Drupal sites is using Views, which already depends on CTools. In fact, there's more sites installed with CTools than with Views (http://drupal.org/project/usage/views and http://drupal.org/project/usage/ctools).

Just fixing this issue would make 7.x-1.x work with exportables properly. Because right now nothing shows up after installing a distribution with exported policies. It's a very broken behavior.

erikwebb’s picture

erikwebb commented

The loading functionality was a soft dependency, so I wasn't too worried about it. You're right though, I'm being too stubborn about having the dependency.

Let's make sure we fail gracefully if it isn't installed (for some weird reason) -

// Load all Password Policy objects via CTools
if (module_exists('ctools')) {
  ...
} elseif {
  drupal_set_message('CTools module is not installed. This is a new dependency of Password Policy and is required for proper loading of policies.', 'error');
}
chanac_hares’s picture

chanac_hares commented

dixon_

thx for the solution, but can you tell me where i write this function ?

thank you

aohrvetpv’s picture

aohrvetpv commented
Category: Bug report » Feature request
Issue summary: View changes
Status: Active » Postponed (maintainer needs more info)

Is this not a feature request? Also, is it still active for 7.x-1.x? 7.x-2.x adds CTools integration.

aohrvetpv’s picture

aohrvetpv commented
Status: Postponed (maintainer needs more info) » Closed (won't fix)

I don't think this is applicable to 7.x-1.x since 7.x-2.x is intended to add CTools support. Please re-open if you think I might be mistaken.