Expiry transactions use default expiration date [#912124]

I thought the problem as part of User Points Contributed modules but seems not correct!

My test is posted here: http://drupal.org/node/912098#comment-3449542

I report it here:

Hi to all,

I'm making some tests with this modules and I found some bugs (I belive!)

The "Exipiring Every" modules seems have some problems:

My testing setting:

Expire time:

1 hour

Adding points for:

registering, comment posting, admin adjust (by "Adjust" option)

Transaction stamping:

Unchecked

My test

No problem:
Adding point (in according with point 2) is ok!
Adding point is still works good if I set an expire time different from the default setting. Example: instead of 1 hour (point 1) I set 3 minutes.

Problems:
Expire operation seems works fine (in according with time indicated before) BUT looking at database I found something strange:
All the expire operations have themselelf an "expire date" with value of the standard Expire time (point 1) instead of a "0" for a "Never expire" value.
The same problem appear adding or subtracting points, using "Adjust" function, WITHOUT an Expire date (leaving the second data filed blank, as indicated) to obtain a permanent modify.

Considerations:
If there's a "default expire time" settings, this will be added also to all "expiring actions", generating a perpetual loop of add and subtract points.

In You need I can give an extract of my "userpoints_tx" table to controll!

I hope this could be useful for a solution!
Thanks!
Bye!

Comment	File	Size	Author
#11	userpoints_expire_expired_d6.patch	4.72 KB	berdir
#7	userpoints_expire_expired3.patch	6.45 KB	berdir
#6	userpoints_expire_expired2.patch	6.31 KB	berdir
#2	userpoints_expire_expired.patch	2.84 KB	berdir

Comments

Comment #1

berdir

German

Switzerland

commented 26 September 2010 at 22:40

Title:	Problem with Expire data! A test.	» Expiry transactions use default expiration date
Version:	6.x-1.1	» 7.x-1.x-dev
Status:	Active	» Needs review

Confirmed and reproduced in 7.x-1.x-dev

The attached patch fixes the bug and also extends the existing tests to account for this. I will provide a backport for D6 once I've commited to 7.x-1.x-dev.

Comment #2

berdir

German

Switzerland

commented 26 September 2010 at 22:42

Status	File	Size
new	userpoints_expire_expired.patch	2.84 KB

Oh. Would help if I would attach the patch.

Comment #3

Dret commented 27 September 2010 at 07:48

Issue tags:

+patch

Ok, thanks a lot!

I'll wait for the 6.x patch!

Bye!

Comment #4

BenK commented 27 September 2010 at 22:22

Subscribing to help with testing...

Comment #5

BenK commented 2 October 2010 at 20:02

Status:

Needs review

» Needs work

Hey Berdir,

I tested out the patch and it works great. Expiration transactions are no longer expiring themselves. So the reported issue is solved.

I thought about marking this RTBC, but then I noticed a couple other bugs related to expiration and thought we might want to tackle them in this issue. But if you would rather have me open a separate issue, let me know.

So here are the bugs/issues I noticed:

1. When points expire, the original points award transaction is getting the timestamp of the expiration transaction. And as a result, this throws off the chronological order of point transactions. It would be better if the original points award transaction kept it's original timestamp.

2. In the "Reason" column, the expiration transaction is displaying the word "expiry". Since this is user facing, we should probably have something more understandable like "!Points have expired." Better yet, is there any way to reference the original points award (or even link to the original points award) in this field? Maybe make the original points award a "parent" transaction? Perhaps it could say something like: "500 !points were added on 5/25/10. That !points transaction has now expired."

--Ben