Everytime the TQL finishes parsing the query, it displays (using drupal_set_message()) the query in a readable format at the top of the page (depending on your theme). This is cool but is not necessary for the module to function properly.
It should be optional.
| Comment | File | Size | Author |
|---|---|---|---|
| #6 | 244106.patch | 2.22 KB | roychri |
| #2 | 244106.patch | 2.33 KB | roychri |
| #1 | 244106-snapshot1.png | 28.81 KB | roychri |
Comments
Comment #1
roychri commentedHere is a snapshot of what I am talking about.
Comment #2
roychri commentedAttached is a patch to add this functionality.
Comment #3
rötzi commentedGood addition.
Just two little comments:
1. The implementation of hook_perm is not necessary as you (correctly) use the 'administer site configuration' permission.
2. The description of the form element has a typo: ... query i nthe ...
Comment #4
roychri commentedThanks for the review.
About the hook_perm, I would argue that most modules have their own administer permission which allow for greater granularity when assigning roles. A sub administrator role could be created and give permission to administer some module and not others. Adding this permission allow that and therefore is more flexible.
I used 'administer site configuration' permission for the "access" value in the hook_menu by mistake as I had planed to use "administer tql".
Your thoughts?
Comment #5
rötzi commentedThe "permissions" page is already difficult to use as there are so many permissions where sometimes you don't know what they do exactly (This will be better in Drupal 7 when permissions get a comment). Therefore you should not use new permission unless really useful.
Additionally, the basic settings of a module are normally set once when the module is installed. This is done by someone with the 'administer site configuration' anyway, so there is no need to introduce a special permission.
There are various modules in core and in contrib which use the 'administer site configuration' for this purpose already: "blogapi", "contact", "throttle", "upload", "system", "wikitools", "talk", ...
If there were different permissions for each of these modules, setting up user roles would get increasingly difficult.
So in general I try not to introcude permissions unless there is a good use case which needs it.
Comment #6
roychri commentedMake sense, thanks for the explanation.
Here's the revised patch. I will apply and commit to HEAD and DRUPAL-5 branch.
Comment #7
roychri commentedPatch applied and commited to HEAD and DRUPAL-5 branch successfully.
Comment #8
Anonymous (not verified) commentedAutomatically closed -- issue fixed for two weeks with no activity.