Closed (fixed)
Project:
User Points Contributed modules
Version:
6.x-1.x-dev
Component:
Code: user2userpoints
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
17 Aug 2008 at 14:56 UTC
Updated:
7 Apr 2010 at 13:41 UTC
Jump to comment: Most recent file
Comments
Comment #1
neomenlo commentedSorry, I didn't classify the status properly.
Comment #2
kenorb commentedsubscribing
Comment #3
OneTwoTait commentedsubscribing. I could really use this. :)
Comment #4
johnbarclay commentedI worked from neomenlo start and got what I think is a working D6 version. Its attached as a zip. Since I haven't used it in drupal 5, when you test it keep an eye out for what you would expect in D5. Also note there is no update code to execute to go from d5 to d6 for user2userpoints.
There are still some to do's that I think would be useful, such as being able to use the form in a block, making it an enpoint for REST calls, and something more theming functionality.
I added some configuration inputs into the points settings page (/admin/settings/userpoints) using hook_form_alter. See attached screenshot.
I also added a category select input to pick the correct term. I'd never used the drupal 5 version of the module, so I'm not sure how a points category was assigned to the transferred points.
It handles urls of form:
/user2userpoints/[to]/[amount]/[tid]
where:
[to] is uid or username of user points are given to
[amount] (optional) is points quantity. defaults to 1 if empty.
[tid] (optional) taxonomy id, 'choose' to have user choose, 'all' to pass all param to userpoints api
eg:
/user2userpoints/jbarclay/5/51 (apply 5 points to jbarclay in in term 51)
/user2userpoints/11/5/51 (apply 5 points to uid #7 in in term 51. User may select another term)
/user2userpoints/jbarclay/5 (apply 5 points to jandoe and let them select category)
/user2userpoints/jbarclay (apply to jandoe, user selects amount and category)
/user2userpoints/ (blank form)
these urls will also take get and post parameters (to , amount, and tid). Same parameters in url override get or post values.
Comment #5
kbahey commented1. Please use the coder module to check the code style for this module. It needs a lot of attention to confirm to Drupal's code style.
2. Why do we need an explicit user2userpoints_admin_submit() function?
3. Remove the debugging code, print/print_r().
4. The module accepts parameters from the URL. I have not digged deep enough to see if this is before the form, or after the form. But if it is after the form without a confirm form, then it opens the door to Cross Site Request Forgeries (CSRF). If someone tricks a user into viewing an image that links to a URL for giving points, they will give points unwittingly. Please read this http://drupal.org/node/178896 and this http://drupal.org/writing-secure-code. You need to add a confirm form to giving forms.
Comment #6
johnbarclay commentedThanks. This is very helpful. #1,#3, and #4 I'll take care of.
With #2, I wasn't sure how to add configuration options for user2userpoints. So I did the hook_form_alter to the form at:
/admin/settings/userpoints
What's a better way to do this?
John
Comment #7
kbahey commentedPlease read the API section in the README.txt file in Userpoints, and look at how other modules (e.g. userpoints basic) do this. It is simpler than that.
Comment #8
johnbarclay commentedGot it. Thanks. I like that approach. The attached module fixes the 4 problems.
Comment #9
kbahey commented1. I don't see anything done for code formatting using the coder module. See http://drupal.org/project/coder and apply it to your code. I tried doing the formatting manually but it got tedious.
4. I don't see a confirm_form() call to prevent CSRF. But I have not run the module. So here is what to look for: If someone visits /user2userpoints/uid/points, do the points get given immediately, or does the user have to click "submit" to give the points? If there is a submit (i.e. a POST operation), then we are safe. If the points are given immediately, then we have a problem since this causes CSRF.
5. The function user2userpoints_giveform() is very hard to read because of the inline Forms API stuff. You must change it to have the logic separate from the FAPI elements. Do the logic first, then have normally formatted FAPI elements instead of appending to elements and such.
6. Also, why not junk the usernames in URLs altogether? Make things simpler: just the uid, or uid + points, or uid + points + tid.
Comment #10
kbahey commentedComment #11
johnbarclay commented1. I tested for minor problems with the coder module and everything came out fine this time.
4. Yes. You do have to confirm with the submit button.
5. I cleaned this up as you suggested. Reads much better now.
6. I left the username in for backward compatibility and because sometimes people want a readable url. Also how handy the uid or username are depends on what context you are generating the url in.
Comment #12
kbahey commentedMuch better.
I did some reformatting and simplified the code in some places.
The part I don't like is this:
And this:
You should use this:
And
And they get mapped into variables automatically by FAPI, without you having to call arg() all the time. Also don't use $_GET and $_POST directly because of security issues.
I am attaching my version, which you should use as a starting point, then fix the above issue and it will be ready for committing it.
Comment #13
johnbarclay commentedThanks. I made these changes. Code keeps getting shorter :) I appreciate your mentoring. Are there any other userpoints modules you'd like me to port to 6?
Comment #14
kbahey commentedThanks. This should be available in the -dev tarball tonight.
As for other modules, Download the latest 5.x -dev tarball and see what is not in 6.x and port them as you find them interesting and/or find time for.
Comment #15
jordanmagnuson commentedSubscribing.
Comment #17
mikou commentedHello, sorry for reopening this issue.
I wonder if there is a way to get a fivestar vote on a user by combining user points and user2user ?
The second thing would be to give a points to another user without decreasing your.