Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.I've found that tac_lite is using a non-standard naming convention for permissions. Permission names show up on the user interface for people to check and are translated from your given name to multiple languages. Permission names are usually start with a verb. Using "administer_tac_lite" as the permission name exposes a machine-name-looking string, which is not suitable for translation and is inconsistent with the rest of Drupal. I assume you might have used administer_tac_lite, so you get code completion or something like that in your IDE, but this is not code, this is a string shown to the user and translated to foreign languages.
The attached patch does the following:
- renames the permission to "administer tac_lite" (if you know a better user friendly name, please do use that :)
- adds an update function so that existing roles with this permission keep this (also introduces the Drupal standard update number versioning scheme)
- removes some debug code and a global $user which was never used.
- fixes code style in usage of the permission (tabs to spaces, curly braces around conditional blocks, etc)
- removes access callback and access arguments from hook_user()'s 'category' op - these are not supported and should not be there to give a false feeling that these do anything to protect the category to qualified users (the condition block above that does)
- adds a t() to the hook_user() categories op
- does little bit of code cleanup at the top of the file
Apart from code style, I noticed other issues, and would like to continue submitting patches, if you think they are good :)
| Comment | File | Size | Author |
|---|---|---|---|
| permissions_tac_lite.patch | 4.34 KB | gábor hojtsy |
Comments
Comment #1
gábor hojtsyWhat's your opinion Dave? Comments from anybody else?
Comment #2
Dave Cohen commentedGabor,
Sorry for the delay. I'm in favor of the patch. There are a couple things I wanted to do before getting it in. One as simple as fixing my indentation in emacs (I don't want to apply the patch then have my editor change the indentation back). I was able to do that over the weekend, BTW. The reason I named my permission as a single word (administer_tac_lite) was to take advantage of my editor's auto-completion. So probably I'll make a constant for "administer tac_lite" and use that in the code. With that one tweak, I'll check in this patch.
I like the patches, keep them coming.
Comment #3
gábor hojtsyGreat. Using a constant for the permission name is fine as long as you also use the permission name in its literal form somewhere in a t() call, so that Drupal can pick it up for translation.
Comment #4
Dave Cohen commentedGábor,
I ended up checking this in pretty much as is. It's in the DRUPAL-6--1 branch.
I've also checked in some bug fixes recently and I'd like to release a new version of the module. Do you have anything else you'd like me to include in the next release?
Thanks again, -Dave