Order User CA Integration [#337363]

Hi,

I worked on uc_order.ca.inc to add some conditions about the user. Here is the list of added conditions :

Conditions on user UID
Conditions on user name
Conditions on user email
Conditions on user creation date
Conditions on user login date
Conditions on user prefered language settings
Conditions on user roles

The user I'm talking about is the user associated to an order. If $order->uid exists, it load this user, if not, it load the current user. (As the user is not known on the checkout page, this is the current user to load).

Here is the patch, applied on the latest 6.x dev found on drupal.org

Comment	File	Size	Author
#9	uc_order_user.ca_.patch	9.96 KB	Island Usurper
#6	uc_order_user.ca_.patch	9.72 KB	Island Usurper
#5	uc_order.ca_.inc_.2.patch	10.78 KB	cha0s
#4	uc_order.ca_.inc_.2.patch	10.7 KB	cha0s
#1	uc_order.ca_.inc_.2.patch	10.62 KB	cha0s
	uc_order.ca_.inc_.patch	10.95 KB	zmove

Comments

Comment #1

cha0s commented 17 December 2008 at 09:28

Status	File	Size
new	uc_order.ca_.inc_.2.patch	10.62 KB

I cleaned up your patch a little bit... mostly coding standards stuff, but I also added some of the $user security stuff.

Thanks for the patch!

Comment #2

Island Usurper commented 19 December 2008 at 15:02

Status:

Needs review

» Needs work

Doing $account->created % 86400 just gives us the number of seconds since midnight of that day. I think we need to do

  $user_created = $account->created - ($account->created % 86400);

Otherwise, we're only going to be comparing against different times during Jan. 1, 1970. I think we also need to use gmmktime() on the settings data since the timestamp above is zeroed to midnight GMT.

For the uid condition, is it actually useful to check that someone's uid is greater than a particular number? The equality checks are good, but I don't see why someone would use the inequality checks.

Looks good otherwise.

Comment #3

zmove commented 20 December 2008 at 15:18

Hi,

We don't have to be so limitative about the functionnalities. Even if you don't see what could be the interest to allow greater than option to the uid, it's an integer, we can easily add a greater than, lesser than condition on an integer, so this should be available.

For example to create a discount for 1000 first created account...

Comment #4

cha0s commented 21 December 2008 at 05:33

Assigned:	Unassigned	» cha0s
Status:	Needs work	» Needs review

Status	File	Size
new	uc_order.ca_.inc_.2.patch	10.7 KB

Incorporated the fix proposed by Lyle.

Comment #5

cha0s commented 21 December 2008 at 05:36

Status	File	Size
new	uc_order.ca_.inc_.2.patch	10.78 KB

Hmm, spoke too soon :) Reviewing the patch again, I find that there was a similar mistake in the user login time logic. New patch follows.

Comment #6

Island Usurper commented 9 January 2009 at 19:43

Status:

Needs review

» Needs work

Status	File	Size
new	uc_order_user.ca_.patch	9.72 KB

I've cleaned up the descriptions on the conditions so that they're more descriptive and better English. I also saw that the creation and login conditions were mixing up the month and day.

Even with those fixes, I think there are improvements to be made. Instead of checking a single name, I think that condition could use an autocomplete field like the user search page. This lets you have more than one name in the condition, and you don't need to figure out the uids, either. That would take care of the need for the uid condition, then.