Events and Event Repeat
| Project: | User Points |
| Version: | 5.x-3.x-dev |
| Component: | Code: userpoints API |
| Category: | support request |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | active |
Jump to:
I have an interesting (and unwanted problem) related to Event Repeat. When you create a node of type Event, and it has a repeating schedule, a new node is created for each schedule date. If the event date is scheduled to repeat five times, you end up with five nodes. Userpoints does exactly what it's supposed to do - it awards points for each of these five nodes.
I am looking for ideas how to get around this. I want the points to be awarded only on the first node creation.
One option is to use hook_userpoints on the points before operation, but I don't have enough details at that point about the transaction to be able to return true or false. I'd like the decision whether to allow points to be granted to happen in real time, so doing a cleanup on hook_cron is less than ideal.
Any ideas?
#1
"points before" is the right place for this.
On top of my head, I don't recall how event repeat works, but a quick check in the code leads me to believe that $node->eventrepeat_rid is what you need to check for.
Of course userpoints will not have access to the node object, but you can easily write a module that would check the following:
<?phpfunction yourmodule_userpoints($op, $points, $uid, $event) {
if ($op == 'points before' && arg(0) == 'node' && is_numeric(arg(1) && arg(2) == '') {
$node = node_load((int)arg(1));
if ($node->eventrepeat_rid) {
// Deny awarding the point for repeated events
return FALSE;
}
}
?>
You may need to flesh out some specifics, but you get the general idea.
#2
Thanks for the idea Khalid - unfortunately, the values of args is:
arg(0) = node
arg(1) = add
arg(2) = event
We don't seem to have access to either $nid or the $node object, and the userpoints hook doesn't accept that as a parameter. $event is also empty when the hook is executed.
Is there any other way to access the node that's about to be created there?
#3
How about this?
<?phpfunction yourmodule_userpoints($op, $points, $uid, $event) {
static $first_time = TRUE;
if ($op == 'points before' && arg(0) == 'node' && arg(1) == 'add' && arg(2) == 'event') {
if ($first_time == TRUE) {
// This is the first time, we allow to go through so points can be awarded on the event node itself.
$first_time = FALSE;
return;
}
else {
// This is the event repeat, we don't award points ...
return FALSE;
}
}
}
?>
Give it a try and see if it does the trick ...
#4
That might work - but here's my thought. Why not have a fifth argument to hook_userpoints where the node can be passed in? If the purpose of the 'points before' operation is to make a determination whether or not points should be awarded, wouldn't it make sense to have access to the node to help make that determination? If that were the case, we could do:
<?phpfunction yourmodule_userpoints($op, $points, $uid, $event, $node=null) {
if ($node->eventrepeat_rid) {
// Deny awarding the point for repeated events
return FALSE;
}
}
?>
This seems to be not only the most straightforward way of handling this condition, but also the most useful.
What do you think?
#5
That is a good idea.
But it will require an API change, and hence is not possible in the 5.x-3.x series. I am not sure if I want a 5.x-4.x series as well, so perhaps that has to be in the 6.x-1.x or 2.x series.
Open feature request for it, and let us get input from jredding on it as well.
#6
While this doesn't help you now.
In Version 4 I'd like to change the hooks so that we pass in the entire $params array (by ref of course.. but this is PHP 5 not 4 we're talking so whatever)
If we did this we wouldn't be having this discussion now as any "before" hook event could just directly access/modify the array before actions are carried out.
#7
@kbahey - Would the change be egregious if the additional argument was set to null? Yes, the API would change, but would be backwards compatible - no one would have to modify any existing code, right?
@jredding - even if the $params array was accessible, you'd still have to do a node_load() on $params['entity_id'] if the entity type was node. It makes sense to me that most people are going to determine whether to return true or false based on something either in the $user object or the $node object. We already have access to the $user object through the global variable, but not the node. It would be nice to simply work on the node without having to load it.
#8
I say lets make the change for a 6.x series,
I already posts a series of changes that I'd like to make up over at groups.drupal.org/user-points
My suggestion is that we pass off only the params array into the hook and made this more like a standard hook_nodeapi call (wherein the hook gets the actual node object for tweaking) so the new function call is
function yourmodule_userpoints($params)
$params will also be an array that will be the same array passed into the API call. Then this really allows the hook to take control over the API without touching the API itself.
I'd be down for a full 6.x complete rearrange/introduce new features and make the 4.x series 6 only and we just leave 5.x where it is except for bug fixes.
#9
the hook is called by userpoints and not by userpoints_basic. Userpoints does not care about nodes, comments, taxonomy, etc. it only cares about points. Adding in $node to the hook defeats the purpose of making it data agnostic.
Thus I don't support placing $node on the hook_userpoints call, it doesn't belong there. Especially since userpoints.module the file invoking the hook doesn't access to the node object. Userpoints_basic does.
So if there were any passing off of nodes it would be in the userpoints_basic file as that is what is actually awarding the points.
You are correct in that if this were modified it would solve your problem but you'd have to do a node_load. A node load does seem a bit heavy handed especially considering the node was just passed through the hook_nodeapi in the userpoints_basic.module file.
Personally i think the best option here is for you is...
1) Don't use userpoints_basic.module to award points for events... (use it for other things)
--Copy the code to a new custom module that only awards point for events. Its the same code with a tiny extra check.
2) dont' use userpoints_basic.module at all, just copy/paste to a new module and go from there.
I'd go for #1, its about 10 or so line in a custom module but an annoying admin note as to why userpoints_basic isn't awarding points to the event node type but to all other types.
Maybe there is a better solution? (btw: I still want to modify the hook ;) to send the $params off)
#10
Those are really good suggestions!
#11
Just following up here -- our solution was indeed to reset the points for event creation to 0 thus bypassing userpoints_basic, and then determine whether or not to award them in hook_nodeapi() in our custom module. Insert looks like this:
<?php
case 'insert':
// Award points here for events
// Set the points in admin/settings/userpoints for these:
// (expand "Points for basic events")
// Points for posting a Event: <set to 0>
// Points for posting a Event with volunteer timeslots: <set to 0>
// This is because otherwise, for repeating events, the points will be awarded
// once for each event.
if ($node->type == 'event' or $node->type == 'volunteer_timeslots') {
// It's an event or an event with volunteer timeslots
$event_points = 3;
$award_points = FALSE;
if ($node->eventrepeat_COUNT > 1 or ($node->eventrepeat_endday and
$node->eventrepeat_endmonth and
$node->eventrepeat_endyear)) {
// It's a repeating event
// Award the points only if the FREQ is set (which only happens
// on the last node which is also the first in the sequence)
if ($node->eventrepeat_FREQ) {
$award_points = TRUE;
}
} else {
// Not a repeating event
// Always award the points
$award_points = TRUE;
}
if ($award_points) {
$params = array(
'points' => $event_points,
'uid' => $node->uid,
'operation' => 'insert',
'entity_id' => $node->nid,
'entity_type' => 'node'
);
userpoints_userpointsapi($params);
}
}
break;
?>
Fundamentally the question was, is the node part of a repeating event sequence, and how do we award the points for just one of them. Unfortunately using eventrepeat_rid did not work. I found that the sequence only gets written to the database just prior to the processing of the last node insert (which is actually the parent node or first in sequence). So prior to that, the rid is unavailable. However, I could not use that fact to avoid awarding the points, because it is also unavailable when a non-repeating event is created.
Code for delete:
<?php
case 'delete':
// Award points here for events
// See 'insert' above for more info
// Only award the (negative) pts if the node being deleted is the one the
// points were awarded on in the first place
if ($node->type == 'event' or $node->type == 'volunteer_timeslots') {
$event_points = 3;
$award_points = FALSE;
// select all rows for this node, and check that only one row was returned
// if more than one, then we must have already awarded the pts on delete
if (db_num_rows(db_query('SELECT txn_id FROM {userpoints_txn} WHERE entity_id = %d', $node->nid)) == 1) {
$award_points = TRUE;
}
if ($award_points) {
$params = array(
'points' => -$event_points,
'uid' => $node->uid,
'operation' => 'operation',
'entity_id' => $node->nid,
'entity_type' => 'node'
);
userpoints_userpointsapi($params);
}
}
break;
?>
Note that we only subtract the points on delete if we're deleting the node on which the points were originally awarded, we decided that was enough for our purposes.
Hopefully this will help someone else. Thank you to everyone for your suggestions and ideas!