Closed (fixed)
Project:
User Points
Version:
7.x-1.x-dev
Component:
Code: userpoints
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
21 Apr 2011 at 15:54 UTC
Updated:
30 May 2011 at 11:11 UTC
Jump to comment: Most recent file
Comments
Comment #1
BenK commentedSubscribing
Comment #2
berdirOk, here is a first patch.
- The listings now update based on the selected category with ajax.
- It is possible to default to a specific category with /myuserpoints/category-id. Note that this is only possible for /myuserpoints. I don't think it is necessary for the others.
- I noticed that userpoints_admin_transactions() and userpoints_admin_manage() were almost identical. Merged these together, this saves quite a few lines.
This patch also opens up the possibility of adding more filters to that form. But that's something for follow-up patches.
Comment #3
berdirOk, did some more refactoring, this should now also work with the User by points page.
I noticed that that page was still theme functions for the header and row definitions, replaced that with a render-array too and merged some parts together.
Comment #5
berdirUpdated the patch to fix the test fails.
Comment #7
berdirOk, fixed the tests. Really this time;)
Comment #8
BenK commentedHey Berdir,
I tested the patch and it's working fairly well. Here are the items I noticed:
1. I'm getting this warning message occasionally:
Notice: Undefined index: form_build_id in ajax_get_form() (line 312 of /Users/benkaplan/git/drupal-7.0b/includes/ajax.inc).
2. When using the filters on both the user transaction list page and the global totals/transactions pages, if I filter the list by a category and then click on the pager, I'm taken to a blank page. It looks like the pager is not updating with AJAX for the number of pages that should be displayed for the particular filter.
3. On the user transactions list page, the Total points displayed (both Approved and Pending) is not being updated on AJAX. So even though the title says that you're looking at total points for the "Participation" category, the points totals displayed are for all categories.
4. I noticed that on the user transaction list page, the page title no longer updates with the category in parenthesis. Is this possible to do with AJAX or can the page title no longer change?
5. On both the user transaction list page and the global totals/transactions pages, if you filter by a category and then choose "Display all", it appears that only the "General" category is being displayed instead of all categories.
--Ben
Comment #9
berdir1. I never got that, are you sure that you are running the most recent 7.x-dev version? Maybe a bug that got fixed already.
2. Crap, same issue as we had with Privatemsg. Most likely the same issue for sorting as well.
4. At least not possible in a simple way. Not sure how important it is.
Comment #10
BenK commented1. Yeah, I'm definitely running the latest -dev because I couldn't get the patch to apply until I updated. That notice didn't appear until I had been playing around with things for a good 5 minutes. So something I did with filtering, clicking pagers, going back to unfiltered, etc. seemed to cause that notice to appear.
4. Okay, let's not worry about it then. It's not a big deal.
--Ben
Comment #11
berdir1. Is a follow-up error because of 2. It happens after you clicked on a broken pager link. So that will fix itself once we fixed the pager links.
2. Ok, I managed to fix these links by injecting the tid as a $_GET argument and also hijacking the $_GET['q'], just like we do with Privatemsg.
3. Should be fixed now.
4. The ajax framework in Core actually allows to do stuff like this. Pretty awesome. The only problem here is that I need to be able to identify the h1 tag of the page title. Bartik uses an id called 'page-title' (And so does the default template) while Seven uses a class with the same name. I does work with both now, I haven't tested other themes. For a theme that uses a different id or class, it simply won't work.
5. Fixed. I also noticed that the Display all option on the "Users by points" page is somewhat problematic. Because it will list each user/category combination separate. In contrast to transaction pages, where it is fine to show all categories, this is somehow weird, especially because it's sorted by points by default. Another option would be to group the points by user, but depending on the meaning of the categories, that doesn't make sense either, because the points would have a totally different meaning. So what I think is the only way out of this misery is an option that allows to control "Display all" for that page, with the options "Show categories separate/ Group categories together/Hide". If hidden, it would default to the default category. But that's a problem for a different day and issue :)
This should be pretty close, I've tested this fairly extensively myself.
Comment #12
BenK commentedHey Berdir,
I've tested the latest patch and it's working pretty well. Just a few things I noticed:
1. This looks fixed.
2. Pager is working well now.
3. Points are now being updated, but there's still one bug: When you click on the "My points" link in the user menu, the totals on the default My Points page are just for the "General" category (even though it should be for all categories). You can get to "All categories" points totals by using the category dropdown, but these totals should be displayed by default.
4. Looks nice in Seven theme!
5. This looks fixed... we can do the rest in a follow up issue.
NEW
6. I'm now getting the following notice at the top of the My Points page after clicking on the "My Points" menu link:
Notice: Undefined index: tid in userpoints_list_transactions() (line 115 of /Users/benkaplan/git/drupal-7.0b/sites/all/modules/userpoints/userpoints.pages.inc).
--Ben
Comment #13
berdirOk, here is a new patch that should fix these two issues.
Comment #14
BenK commentedThe latest patch solves all prior reported issues.
However, I did notice one new issue (that may have existed but that I didn't notice before).
If you are on the global points transactions page, filter by a category, and then use the pager, you are taken to a global "totals" page for that category instead of a global "transactions" page.
--Ben
Comment #15
berdirOk, fix that last bug and commited this!
If there are any more bugs, let's deal with that in new issues.