user_filters() shows permission machine names rather than human labels, and tries to translate them

[kiamlaluno]

This is the equivalent of user_filters() shows permission machine names rather than human labels, and tries to translate them for Drupal 7.

user_filters() contains the following code.

  $options = array();
  foreach (module_implements('permission') as $module) {
    $function = $module . '_permission';
    if ($permissions = $function()) {
      asort($permissions);
      foreach ($permissions as $permission => $description) {
        $options[t('@module module', array('@module' => $module))][$permission] = t($permission);
      }
    }
  }

$permission contains the permission machine name; it cannot be translated. The code should use the permission title, which is already translated when returned from the hook_permission() implementations.

[kiamlaluno]

I need to understand why _testImageStyleUrlAndPath() fails.

[kiamlaluno]

I do not see any relation between ImageStylesPathAndUrlUnitTest::_testImageStyleUrlAndPath() (the failing test) and database_test_theme_tablesort() (the function that indirectly calls user_filters()).

[kiamlaluno]

All checks have passed. The previously failing test has been fixed in another issue.

[klonos]

Thanks @kiamlaluno 🙏🏼 ...the PR code looks good, and it matches the code in the respective issue in Drupal. Just trying to understand where this function is being used, in order to be able to review this properly. Also, why we are adding the translated strings as keys in the $options array 🤔 ...and what does this mean?:

/**
 * List user administration filters that can be applied.
 */

...what user administration filters and for what? ...and where might they be applied?

[kiamlaluno]

@klonos To answer to where user_filters() is used in Backdrop, it is only called from database_test_theme_tablesort() (indirectly, via user_build_filter_query()). In Drupal 7, it is used in admin/people, but in Backdrop admin/people is a view.

It seems that both user_filters() and user_build_filter_query() can be removed. database_test_theme_tablesort() (which tests theme_tablesort() using data read from the database) could be rewritten to use different data (and avoid to call user_build_filter_query()).

I opened this issue thinking that user_filters() was used to populate the options available in some form, for which the bug was to ask translators to translate something they could/should not translate (a machine name). It is now clear that user_filters(), although it passes to t() a machine name, it is not asking to translators to translate something they are not supposed to translate, because that function is only used in tests.

[kiamlaluno]

As a side note, views_handler_field_user_permissions, the class that shows a permission field in views, already does the Right Thing™️: It uses the permission title instead of the permission machine name.

foreach ($all_roles[BACKDROP_AUTHENTICATED_ROLE]->permissions as $permission) {
  $authenticated_permissions[$permission]['permissions'] = $permissions[$permission]['title'];
}

foreach ($user_role->permissions as $permission) {
  if (isset($permissions[$permission])) {
    $this->items[$user->uid][$permission]['permissions'] = $permissions[$permission]['title'];
  }
}

What I reported here is only valid for user_filters(), which can be marked as deprecated as per #6534.