As discussed with Berdir in IRC, each points transaction should have a 'view' page that the user or site administrator can visit to see details about that points transaction. This page would include information about the transaction ID, parent transaction ID (with 'Reason' of parent displayed and linked to that transaction), points amount, points category, timestamp, expiration date, description, operation, reference, status, approver, and other data from the userpoints_txn table.

One question we need to think about is which fields that the points recipient can view and which fields require an adminstrator level permission (such as "edit" or "administer userpoints"). For instance, we might want to make the transaction ID actually visible on this page to the end user because it could facilitate customer service questions about the points transaction. Users could simple mention the transaction ID and site administrators could easily look it up.

--Ben

CommentFileSizeAuthor
#10 userpoints_view3.patch19.25 KBberdir
#9 privatemsg_realname14.patch20.76 KBberdir
#6 userpoints_view2.patch16.61 KBberdir
#2 userpoints_view.patch8.59 KBberdir

Comments

BenK’s picture

BenK commented

One of the last few issues remaining before a 7.x-alpha1 release...

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Active » Needs review
StatusFileSize
new userpoints_view.patch8.59 KB

First version.

Currently displays everything I could think of, suggestions are welcome ;)

BenK’s picture

BenK commented
Status: Needs review » Needs work

I've begun testing this. As a first pass, it looks really good! :-) Here's one obvious bug to report first:

When I visit the /myuserpoints page for any user, I currently get the following error message:

Notice: Undefined property: stdClass::$user in userpoints_access_view_transaction() (line 298 of /Users/benkaplan/git/drupal-dev/sites/all/modules/userpoints-DRUPAL-7--1/userpoints.module).

I tried clearing the cache and rebuilding the menu, but that didn't appear to solve the problem.

--Ben

BenK’s picture

BenK commented

Hey Berdir,

I've done further testing and got some suggestions for possible UI enhancements, but I think it might be easier to discuss directly. Can you stop by drupal-games sometime soon?

--Ben

BenK’s picture

BenK commented

A. As discussed in IRC, we should group the fields as follows (and in the following order):

!Points amount
Category
User
Date
Reason
Transaction ID

STATUS
Approval status
Last modified date
Expiration status
Expiration date

RELATED !POINT TRANSACTIONS
Parent transaction
Child transactions

ADMIN
Moderator
Description (manually entered)
Description (auto generated)
Operation
Reference
Edit

B. Rename "Approved by" as "Moderator".

C. Maybe rename "Parent transaction" as "Earlier transactions"? And "Child transaction" as "Follow-up transactions"?

D. On an expiration transaction, hide the "Expiration date" and "Expiration status" fields. Otherwise, it could be confusing for an end user to see "Never" on the expiration status for something that just expired.

E. Make the possible choices for "Expiration status" either "Expired" or "Not expired".

F. Should the expiration date and status fields just be hidden if an expiration date is not set for the transaction? I wasn't sure.

G. Make the fields in the "ADMIN" group above only visible to those with "edit" or "administer userpoints" permission. This includes the following fields: Moderator, Description (manually entered), Description (auto generated), Operation, Reference, and Edit.

H. Make a setting on the UP settings page for "Footer text". This would be handy to enter explanations about what the labels mean, links to request review of a declined transaction, etc. Would be global for the transaction view page.

I. Explore the possibility of a setting to change the start number of the transaction ID. But this may be problematic because of auto-increment values and different database types.

J. Change the page title to be like this: "Details for transaction #ID"

K. On this page, their may be a name conflict between the "Operation" field and "Operations". Any ideas on how to resolve? But this is only for admins so this is not a huge deal. But since the "Operations" column is user-facing at /myuserpoints (and "view" isn't really an operation), maybe should that column be renamed "Actions"?

--Ben

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs work » Needs review
StatusFileSize
new userpoints_view2.patch16.61 KB

A. Ok, implemented. Also made sure that only the things are shown that actually contain useful information. For example, the manually entered *and* generated description is only shown when there is a manual description. If not, only the reason is shown.

B. Done.

C. Not sure. Follow-up could work but Earlier is unclear imho. The definition of parent as we are using it here is "(computing, object oriented computer programming) The object from which a child or derived object is descended.". Any ideas for a good synonym? LEO found these: http://dict.leo.org/ende?lp=ende&p=CqhggsWkAA&search=Vorg%C3%A4nger&tres...

D. I assumed you meant "non-expiring" :) Done.

E. Changed.

F. I think so, yes. Hidden.

G. Done.

H. Not sure. hook_page_alter() allows to extend and alter everything the page. It allows to place a help text above or below or even somewhere between.

J. Changed.

K. Renamed to Actions everywhere.

Status: Needs review » Needs work

The last submitted patch, userpoints_view2.patch, failed testing.

BenK’s picture

BenK commented

This is looking great. I like this latest version a lot. Here are some things I noticed:

A. The labels on "Expiration status" and "Expiration date" are being reversed. The date is being labeled as the status and vice versa, Also, the actual "Expiration status" field should be listed above the expiration date.

C. How about "Prior transactions" and "Follow-up transactions"?

H. Okay, we can do it that way. How would I use hook_page_alter() in a custom module to do this?

New stuff:

1) I think using "General" as the group header for the first group is a little confusing because "General" is also the default category name. So how about we change the first group header to "Details". Then, so that we don't have a conflict with the page title, we could change the page title to: "View !point transaction #TID"

2) The operation on an expiration transaction is still "expiry". I thought we changed this before? It would be better if the operation was "expiration" (since that's a real word)

3) Instead of the label "Last modification date", how about just "Last modified"? I thought that might be a bit shorter and simpler.

4) Can we add CSS classes for each grouped section and each individual field? It might be nice to be able to style the "Details" section, for instance, different than the "Status" section. And maybe the "Admin" section would have a whole different look as well.

--Ben

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs work » Needs review
StatusFileSize
new privatemsg_realname14.patch20.76 KB

A. Fixed.

C. Ok, works for me, changed. "Prior transaction" actually, since there can only be one (unless we check recursively, but I don't think we should).

H. Example:

/**
 * Implements hook_page_alter().
 */
function hook_page_alter(&$content) {
  if (arg(0) == 'userpoints' && arg(1) == 'view') {
    $content['content']['system_main']['note'] = array(
      '#markup' => t('Add your stuff here'),
    );
  }
}

The important parts are the checks if we are on the userpoints/view/X page and then we add something inside $content['content']]['system_main']. By default, this will show below. You can add a #weight property with a negative value to add it above. You can also alter anything that is printed on that page, just use dpm($content) to figure out how it is structured.

1. Changed.

2. Not relevant :) This is the technical identifier. userpoints_nc_node_insert is not a word either :) Normal users will never see this information, they only see the reason. It would be possible to change it, but we would need to change all existing instances in the database to have the reason show up correctly for them. This is a lot of work that's not really necessary.

3. Sure, changed.

4. Added explicit weight (so that you can move stuff around easily and add something in between if you want to do so) and added a class to all categories and items. I also added a class to indicate not yet expired and expired transactions and another one for the current status, similiar to what we did for rows in listings.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
StatusFileSize
new userpoints_view3.patch19.25 KB

Wrong patch :)

I need to get some sleep...

BenK’s picture

BenK commented
Status: Needs review » Reviewed & tested by the community

Hey Berdir,

Latest patch works great. I'm marking this RTBC! :-) All fixes have been verified to work properly.

Before you commit this, I've got a few small string clean-up and CSS tweaks if you want to add them:

a) To be consistent with how we use it elsewhere, maybe the page title should use the plural "!points" rather than the singular "!point". So it should be "View !points transaction #TID" instead of "View !point transaction #TID". This way, it matches stuff like "Related !points transactions".

b) The label "!Points amount" should probably just be "!Points" The word "amount" just confuses things and makes it longer.

c) I love the CSS you added. It's really comprehensive and gives a lot of control. A few small tweaks:

* Instead of class "userpoints-group-general", it should probably be "userpoints-group-details" since we changed the name of that group (and don't want to get it confused with the general category).

* h3 tags should probably include the approrpriate group class name so that you can style individual group titles differently. So instead of just h3 tags around the "Details" group header, for instance, we should probably include "userpoints-group-details" as the class of the h3 tag.

* You wrote: "I also added a class to indicate not yet expired and expired transactions and another one for the current status, similiar to what we did for rows in listings." I love that you added this to the entire view page (on the parent div for the page). Can we also add a CSS class for the category here? This way, you could style the view page differently based on the points category. We also did this on the row listings.

--Ben

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Reviewed & tested by the community » Fixed

Ok, updated the title and css classes, added a global css class for the category and one for negative/positive points.

Commited!

robby.smith’s picture

robby.smith commented

Hi! Is this view for each points transaction available in 6.x version?

Regards

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

No, and it won't be. This depends on too many things that are new in 7.x-1.x

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.