provide reason for failure

I think this is a reasonable request except that the document already exists that currently userpoints_userpointsapi() returns FALSE on error and not a string. It could be watchdogged though.

Maybe there would be a "debug" option in the settings? But that could confuse the non-developers (which outnumber the developers by a vast majority).
Maybe a flag in the file itself (then it requires digging into the code).

hhhmm... good idea but I'm not sure how to implement it.

Do you have any examples of other Drupal modules that are doing this?
Do you know of any "standard" that exists? If we implemented this it'd be nice if it fell in line with what other modules are doing. If others are watchdogging we'll watchdog, if others are returning an array we'll return a string otherwise developers are just running all over the place trying to figure out what's going on.

The only Drupal-ish (which really isn't Drupal at all) module that does this is CiviCRM and from my experience even though the functions returned array, that contained error codes, they still weren't helpful.

I'm all for it just want to make sure its done cleanly. Also we'll have to push this out to a later version. Version 3 has been "frozen" for quite sometime and its pretty stable. Because this would require code changes in other modules I don't think its fair to just sneak it into the module, release it and then be like "what, what... you didn't read that thread ;) ".

Don't stop the conversation though.... Let figure out a good model.

If we're going to return an array what would it look like?
return array (
'success'; => 0, //0 for fail, 1 for success
'uid' => $uid, //uid of who received the points
'points_granted' => $points, //points granted after other modules possibly tweaked them
'pointtotal' => $pointtotal, //already in memory from the $mesg display no additional SQL necessary
);

etc. etc.

To be honest I would have loved it if the api call would have returned the transaction ID of the points recently granted.

Darn, if this would have been before the first stable release ...

But if we are going to do it, then it should be something like this:

return array (
  'status';  => TRUE,  // TRUE for success, FALSE for failure
  'uid' => $uid,  //uid of who received the points
  'points_granted' => $points,  //points granted after other modules possibly tweaked them, 1 for success
  'point_total' => $pointtotal, //already in memory from the $mesg display no additional SQL necessary
);

But we don't need to go overboard with too many elements in the returned array since it becomes another API in reverse ...

I think we need to create a new release (3.1?) with this feature in, and only 2 components on the returned array:

return array(
'status' => TRUE/FALSE,
'reason' = 'some arbitrary string that we add',
);

We can hold on the rest of the stuff for now.

The reason I feel this should be rushed out is that 3.0 is out and the longer we wait, the more people will use it with a true/false return and the more work for them later.

Once we have the return as an array in 3.1, then extending it later to include points, total, ...etc. is transparent to modules that use 3.1, but not 3.0.

greggles/jredding, what sayeth ye?

aaah crap. Man I really thought I was "done" with userpoints...but nnnoooo greggles had to go and make another change. (kidding)

ya, it would be nice to have in version 3.

I don't have time to code this in within the next few days but it would be very difficult to add in. We definitely have to rework the docs though and the contrib modules are definitely going to be screwed around.

Can we just pull version 3 off of stable? I know we can't do anything about the tag but pulling the release would probably help out.

Sorry I mean to write that it wouldn't be very difficult to add in. Off the top of my head I can think of only a few places where "return false" is done in userpoints_userpointsapi..

Actually, if you check the project, luckily 3.0 does not show under "Official Releases" because I forgot to make 3.0 the default release in the first place so it only showed on the View all releases page.

I just unpublished the release node for 3.0 now.

http://drupal.org/project/userpoints

Changing the "return false" is not a lot of work, but adding the reason for failure may require a bit more effort.

So instead of:

return FALSE;

to

return array('status' => FALSE, 'reason' => 'the real reason');

And this needs to be added to the documentation as well.

Nah, let is do it right.

This way, adding returned array elements in the future will be less of a pain for modules.

Jacob, if you do not get to it in say a week, let me know, and I will give it a shot.

commited with a basic return

return array(
'status' => true/false,
'reason' => 'string with reason',
);

It can be fleshed out over time.

good call..

I updated the README.txt file as well as the comments on the function.

I also put it into the CVS commit message so it'll go into the changelog.

kbahey can you make sure it gets on the release notes page when you do a new release?