2.x version added a new column 'max_points' to {userpoints} table, it breaks the current retroactive.module. It's sort of a easy fix, just adding it in the corresponding INSERT sql. But before submitting a patch, I'd like to get a few questions cleared/addresses:

1. What's the use of this 'max_points' field? I looked at the code to calculate but couldn't understand it. Can somebody help to explain?

2. When doing a retroactive through new userpoints_userpointsapi(), is it ok to use op='txn approval' instead of 'points' to bypass the moderation? This also leads to question 3

3. Does anyone really need moderation for points when a node/comment is posted or deleted? This is how the current 2.x version works. But it looks to me in most if not all cases, these points are better off handled automatically, admins just needs to moderate admin-manually-designated points. Otherwise, it's really a tedious work to moderate every node and comment creation.

4. If my argument above makes sense, in txn_table we probaly just need to store admin-designated points not points can be auto caculated from user posts.

Once these questions are answered, I will submit patches accordingly.

Thanks.

CommentFileSizeAuthor
#3 userpoints_retroactive_0.patch1.98 KBjenlampton

Comments

kbahey’s picture

Status: Active » Needs work

1. What's the use of this 'max_points' field? I looked at the code to calculate but couldn't understand it. Can somebody help to explain?

It is for future use, for example, if someone redeems his current points for a gift, ...etc., we want to still know what was the maximum he got to before he redeemed them.

2. When doing a retroactive through new userpoints_userpointsapi(), is it ok to use op='txn approval' instead of 'points' to bypass the moderation? This also leads to question 3

I don't think so. But go ahead and try it and see what happens to be sure.

3. Does anyone really need moderation for points when a node/comment is posted or deleted? This is how the current 2.x version works. But it looks to me in most if not all cases, these points are better off handled automatically, admins just needs to moderate admin-manually-designated points. Otherwise, it's really a tedious work to moderate every node and comment creation.

Moderation was added to prevent abuse. There were cases where users would sit and comment/vote ..etc. to exceed a certain number.

You can turn moderation off completely, and that is the default.

If you want to make moderation more granular, i.e. for selected operations, then go ahead and submit a patch. However, remember that the event is free text, and not predefined, so you may not catch all cases.

4. If my argument above makes sense, in txn_table we probaly just need to store admin-designated points not points can be auto caculated from user posts.

No, we should store everything. The whole point is to be able to audit everything that happens. If you want to fast track some types of transactions (i.e. skip the moderation steps for them selectively), then go ahead and submit a patch. It has to be backwards compatible though.

jredding’s picture

Version: 5.x-2.1 » 5.x-2.10

I also don't understand the use of the max_points column. Couldn't the max points be calculated through the transactions? If someone redeems points its a transaction and similar to looking at a bank statement you can calculate the points up to the point of redemption.

I agree with kbahey in that ALL items should go into the transaction log. userpoints table should be a SUM of all transactions for auditing reasons. The userpoints table simply acts as a caching table.

The only time I can see killing the transaction table is for performance, but you'd have to be on a hefty site and be using points in such as a way as tension will never flair.

jenlampton’s picture

Title: Patch needed for Retroactive, and more general question on moderation » Retroactive with MaxPoints, and can be run without 'reset'
Version: 5.x-2.10 » 5.x-2.14
Component: Code » Code: userpoints_retroactive
Assigned: Unassigned » jenlampton
Status: Needs work » Needs review
StatusFileSize
new1.98 KB

I was frustrated by this too, here is the fix I used.

My patch also includes an automatic 'reset' so that you don't have to run reset before retroactive anymore.

Jen

jredding’s picture

Status: Needs review » Needs work

Patch in its current state is incorrect. It only takes into account the userpoints table and ignores the userpoints_txn table. If the is attempting to "reset" the userpoints it should ideally either
(A) use the API to insert a negating entry into userpoints_txn thus wiping out all points
or
(b) Truncate BOTH userpoints_txn table and userpoints.

This patch was not incorporated into v3 although I agree that both the retroactive module and the reset module could use some serious work.

jredding’s picture

Status: Needs work » Closed (fixed)

Back to the original bug report. Max points is used to demonstrate what the highest points gained were. Although its correct that this could be calculated by a sum SQL statement, this wouldn't work IF older transaction had been deleted as might be the case on high traffic websites.

regardless, max point has been fixed and the API completely rearranged. in v3 the retroactive module was modified to NOT do a direct INSERT statement instead relying on the userpoints_userpointsapi() to do the work. All modules have been modified to do this.

NO MODULES should do direct INSERT or UPDATE statements to the tables instead they should us userpoints_userpointsapi to do the work this will eliminate bugs, such as this one, in the future.

Also.. moderation was modified in v3 which should address your concerns.

jredding’s picture

Project: User Points » User Points Contributed modules
Version: 5.x-2.14 » 5.x-2.x-dev