Fine-grained administration permissions and improved add/edit form

I think this is a good feature request. I'm going to suggest that it be pushed out to a later version and not included in the initial stable release of version 3.

I'd also like to explore the possibly of moving this code over to userpoints_basic, this way the core API stays at the collect and report points arena and userpoints_basic grants the points.

moving to head for future development...

Reclassifying as "postponed" since there are no active plans, patches or developers working on separating these permissions.

This patch does quite a few things more than the original request but I still think it makes sense to use this issue:)

There are basically two separate things in this patch but I think they are all quite related.

1. Introduce 3 new permissions. add/edit/moderate userpoints. These permissions can be combined or used separatly. They allow allow the following:
- All three permissions allow access to the list of userpoints at admin/config/people/points, as it is the default local task.
- add obviously allows to create new userpoint transactions
- edit allows to edit existing transactions and currently also gives access to /myuserpoints/otheruser. Reason for that is that it's currently the only place (except a custom made view) that lists all approved/declined transactions and we probably want to add the "operation links" to that page too, if you have the necessary permissions edit, approve/decline)
- moderate allows to approve/decline pending transactions
- administer can still do everything
- Introduces userpoints_admin_access($type) ($type can be list, add, edit, moderate or anything else (anything else currently means

2. Clean up and simplification of the add/edit form. Including (in the order the changes appear in the source, so it's not important stuff first or something like that ;)):
- Show not found when trying to access with no txn_id or an invalid txn_id
- Passes $txn as a single value element to the submit handler instead of multiple hidden values like txn_id and txn_uid and txn_approver_id. This allows to compare the existing and changed transaction, more on that later.
- Adds explicity weight on all visible elements to make it easier to move them around and also have a better default order (I think, we can still move things around).
- Improved descriptions
- If the user adding the transaction additionally has moderate permission, he is allowed to override the moderation site default when creating a transaction. Similar, users which are editing can only change the status if they have moderate permission too.
- time_stamp is only displayed/allowed to change if it is actually allowed to change that (there is an option that denies that by default)
- category is only displayed if there are actually categories to choose from (other than general/uncategorized). This is consistent with my changes to /myuserpoints which only shows the category filter/column if there are any.
- operation can be set when adding points
- Fixes some invalid properties, for example, the textarea uses #width/#lines instead of #cols/#rows and some elements have two #descriptions but no #title :)
- Approver is displayed as a read-only textfield *only* when there is one
- Approver is updated automatically according to the following conditions:
-- Adding message that is not moderated: You are the "approver"
-- was pending/declined and changes to approved: You are the approver
-- Was approved and stays so: Existing approver remains, if one exists (probably not if the points were granted through the API and not moderated by default)
-- Was approved and changes to pending/declined: approver is removed
- Add validation for username, points and time_stamp

Summary: Simplifies the form, makes them look almost the same for add/edit (except that add has moderate checkbox and edit has status radios), adds validation, make approver read only and handled automatically.

Happy testing :)

No screenshots yet, they don't tell that much and the form is longer than the screen :(

Subscribing to help with testing...

Hey Berdir,

Here are my notes on your recent patch:

1. At admin/config/people/userpoints, a user with only the "moderate" or "add" permission can see the "(details)" link. Clicking that link results in an access denied error. Instead, the link should probably be hidden unless the user has the 'edit' or 'administer userpoints' permission.

2. At admin/config/people/userpoints/moderate, a user with only "moderate" permission can see the "edit" link. Clicking that link results in an access denied error. Instead, the link should probably be hidden unless the user has the "edit" permission.

3. Change the following strings on the 'Add' form:

a) Change this: 'If checked, !points need to be moderated.' To this: 'If checked, this !points transaction must be approved through the moderation process.'

b) Change this: 'Enter an optional description for this transaction, such as the reason it is created.' To this: 'Enter an optional description for this transaction, such as the reason !points were added or subtracted.'

c) Change this: 'Enter an optional reference for this transaction. This is for internal tracking and is not shown to the end user.' To this: 'Enter an optional reference code for this transaction. This is for internal tracking and is not shown to the end user.'

4. Also change these strings:

On the permission page, the description for the 'edit' permission reads: 'Allows to change existing !point transactions.' Change this to: 'Allows to modify existing !point transactions, including the ability to view transaction history for all users.'

When points are awarded to yourself, the drupal_set_message has a typo. It currently reads: 'You just earned 9 kudos and have now 461 kudos in the Uncategorized category.' The word "now" should come before the word "have".

5. As you suggested in IRC, make "edit" links available at 'admin/config/people/userpoints' and 'myuserpoints/[uid]' for those who who have either 'edit' permission or 'administer userpoints' permission.

6. I'm not sure about making the points "List" page the default local task. Because those with the "moderate" permission now see this even though they don't have "view all points" permission. But I'm not sure what a better solution would be.

7. If you use the "category" filter on the points list page (admin/config/people/userpoints), you are incorrectly taken out of the overlay when using the category filter.

8. Is it possible to have a jquery calendar popup on expiration date field? I wasn't sure if D7's jquery implementation included this.

9. The points add form does not display a drupal_set_message confirmation (except when you are awarding points to yourself).

10. If you have the "Edit" permission (but not the "view all points" permission), then you can't see the "View" link when visiting another user's page. But this 'View' link should probably be visible because the edit permission now includes access to another user's point transactions.

11. After editing a point transaction, there is no drupal_set_message is displayed to let you know that changes were successfully saved.

12. If you don't have the "edit" permission, and you try to click the "edit" link, you are correctly given an access denied message. But it is not the normal Drupal access denied page (it is just a white page with some basic text in it). If possible, this should be the normal Drupal access denied page (which is themed as usual).

13. When editing a point transaction and changing the category, any category changes you make won't save.

14. Should "Approver ID" also record who declined a transaction? Meaning should that field be really about storing the ID of whoever placed it in it's current state? The reason I ask is because in testing the "moderate" permission, I realized that the site admin may want to audit all of the transactions that a certain moderator declined. I could see that being an actual table column for someone with the "administer userpoints" permission.

--Ben

1. Fixed
2. Fixed
3. Changed
4. Changed (now have was already changed elsewhere)
5. Edit link at admin/config/people/userpoints makes no sense because that lists point totals. operation links at myuserpoints is a separate issue.
6. There is nothing else available :) Also, moderate without view all points permission doesn't make much sense imho :)
7. Nothing new, I'm pretty sure. That thing is crappy anyway but again, separate issue :)
8. Maybe, but again, one thing at a time :)
9. It should if you have view all points permission because you can only see them then. We could add an additional check for the add/edit permission there, but I think an administer permission without view all points just doesn't make much sense :)
10. See above, I just don't see the point of that permission combination.
11. The message is shown if you have the view all permission. But the message is incorrect because it's the same as the add permission. Not sure what to do here...
12. Cannot reproduce, I get the default theme access denied page. Maybe that white empty page is seven's access denied page? There is nothing on there when you don't have permission for toolbar and so on..
13. Fixed.
14. I guess so, but Approver isn't really an appropriate description anymore then. Suggestions for label and description? What about setting back to pending, should that be recorded or set back to empty?. Currently changed to record both approved/denied, setting to pending empties the field.

The patch looks great. This is very, very close to being done. I only found a few minor things:

11. I see this message now. But perhaps it should be something similar to what is printed when you save the settings page. How about: "Changes to the !points transaction have been saved."

13. Category changes are being save, but there is still one problem: Regardless of the category currently selected, when you visit the edit page the select list is defaulting to "General." This make is it easy to inadvertently change the category to General when you are making other changes. Instead, the select list should show the currently selected category.

14. How about: "Moderator ID". Description: "The user ID of the moderator who gave the !points transaction its current status." And if a points transaction is set back to pending, I think the moderator ID should be stored for that change since it is a status change. This way, if you a pending transaction with a moderator ID, you would know someone set it back to that. And if it didn't have a moderator ID, then you'd know the "pending" was its default state (because it was being moderated by default). Sound good?

NEW A. At "admin/config/people/userpoints", the "(details)" link is pressed right up against the username. There should be a space between the username and the left parenthesis.

--Ben

11. Ok, changed.

13. Fixed.

14. Ok. Just used "Moderater" because that's what's displayed now, not the ID... Ok, changed it to record all changes, including creating.

15. Added a space.

Everything is working great. This is RTBC.

Note: I did notice one small string typo that could be fixed when you commit the patch. It involves the description for the "Moderator" field. Currently, the word "!points" is being displayed literally as "!points" instead of being replaced by the branded name. The easiest way to fix this would just be to delete "!points" altogether so that the description reads: 'The user who gave the transaction its current status.' Or you could leave it in and just get the replacement to work. Either way is fine with me.

Great work on this patch! :-)

--Ben

Awesome, removed !points and commited this.