backdrop / backdrop-issues

Issue tracker for Backdrop core.
144 stars 40 forks source link

[BC] Change spaces to underscores in permission machine names #3787

Open ghost opened 5 years ago

ghost commented 5 years ago

It seems that permissions are the only things (that I know of) in Backdrop (and even Drupal 7) whose machine names can have spaces in them. Everything else (content types, image styles, etc.) have machine names with just "lowercase alphanumeric characters, underscores (_), and hyphens (-)".

Can/should we change permission machine names to match everything else? Is there a (valid) reason permissions are setup like this? This'd obviously be a major API change, so it'd only go in 2.x, if at all.

klonos commented 5 years ago

I agree that this is an odd thing that has bugged me as well. Also agree that it needs to be a 2.x thing.

ghost commented 5 years ago

Unless... We added something like this to the appropriate function(s):

$permission_name = str_replace (' ', '_', $permission_name);

e.g. add a BC layer.

bugfolder commented 3 years ago

Unifying machine names in that way seems like a worthy goal, and the "have simple rules" gene in me is totally in favor of it, but yeah, it would affect a lot of contrib modules that use permissions from core (or from other contrib modules).

Would this policy be strongly enforced in 2.0 on contrib or custom modules, and if so, how? Currently there's nothing preventing someone from creating a contrib module with a permission 'access MyModule'. It would seem that there might ought to be a test added to the standard test suite for all contrib modules to ensure that people are following this rule. (Also, of course, documentation added to hook_permission().)

It would also be really nice if we could help users along in this change, by, for example, providing a version of Coder Upgrade that automatically converts non-compliant permissions in 1.x modules.

In fact, might I suggest that for every old-code-breaking change that goes into 2.x, along with the commit(s) that implement the change, there also be commits that (to the extent possible) implement some sort of Coder Upgrade-like funtionality to ease the process for people making their modules 2.x-compliant. (I am a huge fan of CU, and think it needs wider advertisement, even now.)

klonos commented 3 years ago

I've been experimenting with something like this on my local (as part of #5030):

function hook_permission() {
  return array(
    'administer my module' => array(
      // @todo: Remove in 2.x.
      'alias' => 'administer_my_module',
    ),
    'administer_my_module' => array(
      'title' => t('Administer my module'),
      'description' => t('Perform administration tasks for my module.'),
    ),
  );
}

So basically, we allow permission definitions in hook_permission implementations to have an 'alias' key, "redirecting" to a new/renamed permission.

bugfolder commented 3 years ago

Is the idea to introduce permission aliases now, to encourage people to use the new form, and then in 2.x, modules would drop the old-style permission and its alias?

klonos commented 3 years ago

Yup. If this idea is accepted, we could implement this during the 1.x cycle, instead of having to wait for 2.x.

indigoxela commented 3 years ago

If this idea is accepted, we could implement this during the 1.x cycle

This is a radical change just because someone felt that spaces seem odd. And requires a huge effort, and potentially code bloat to provide backwards compatibility. Sorry, I simply don't get the point. IMO effort and gain are in no reasonable proportion.

To me this looks like a change for 2.x.