Drupal 7 port (userpoints_register)

Nice work, this looks good apart from a few minor things, see below.

+++ b/userpoints_register/userpoints_register.jsundefined
@@ -0,0 +1,21 @@
+// $Id$

Still one tiny $Id$ line in here :)

+++ b/userpoints_register/userpoints_register.jsundefined
@@ -0,0 +1,21 @@
+        if ($('#edit-userpoints-register-enabled').is(':checked')) {
+          return Drupal.t('Enabled, @points (%category) for content.', params);
+        }
+        else {
+          return Drupal.t('Disabled, @points (%category) for content.', params);

Also, these texts are certainly wrong :)

+++ b/userpoints_register/userpoints_register.moduleundefined
@@ -1,73 +1,95 @@
+  if (variable_get('userpoints_register_enabled', TRUE) != TRUE) {

You can just negate the return value of the variable_get() call here, like this:

if (!variable_get('userpoints_register_enabled', TRUE)) {
  return;
}

Or even wrap the part below into the if instead of negating it and returning early.

Powered by Dreditor.

Hey Bastlynn,

I started testing this, but then noticed a fairly major issue. After installing this sub-module, I'm getting a White Screen of Death when visiting the site configuration page (admin/config). Here's the error in my php logs:

PHP Fatal error: Cannot redeclare userpoints_nodelimit_uninstall() (previously declared in /Users/benkaplan/git/drupal-7.0b/sites/all/modules/userpoints_contrib/userpoints_cap/userpoints_cap.install:12) in /Users/benkaplan/git/drupal-7.0b/sites/all/modules/userpoints_contrib/userpoints_register/userpoints_register.install on line 16

It looks like you may have some naming conflicts between userpoints_contrib sub-modules you've been working on? Can you take a look at this for each of the sub-module?

Thanks,
Ben

Don't be sorry, I made worse mistakes :)

Thanks for your work!

Bastlynn and Berdir,

Great work on this! :-) I've tested the latest patch and can confirm that the uninstall function is fixed. Here are the remaining to-do items I noticed:

a) When points are awarded upon registration, all of the point awards are in the "General" category regardless of the particular category that was assigned points on the settings page.

b) Even when the "display message" option is selected, new (anonymous) users are not seeing the points award message upon registration. The message is only displayed, it appears, to userpoints administrators who create a new user.

c) Currently, the automatic description for the operation reads "!Points for registering." To fit our preferred syntax and tense (used by other UP related modules), I'd change this to: "Created new user account."

d) On the Userpoints settings page, I'd change the vertical tab to "User registration". This better fits the syntax other UP modules use on that page.

e) On the Userpoints settings page, I'd add a description to each !points amount field. I'd suggest: "The amount of !points to be awarded upon registration."

f) I'd re-write the description on each "display !points message" setting. The current description should be much shorter. I'd suggest:

"If checked, a !points award notification message will be displayed to the user after registration is complete."

g) If an administrator creates a new user and the "display points message" setting is enabled, then the points message is being displayed above the "Created a new user account for..." message. I'd switch the order so that the "Created new user" message is above the "Points award" message.

Thanks so much for your contribution. I look forward to testing the next patch! :-)

--Ben

Trying to get back to things... :)

a) Fixed.

b) Hm. The problem is that displaying that message is based on a permission check that we can not work around here. It works if you give anon users the view all points permission. (view own doesn't work because userpoints.module doesn't know that it is their own user account.. their current uid is 0 and different than the newly created user. I am not sure what to do. Do we really need that option? Add a description? Reverse the option (Make it a [ ] Hide message option) + description that it is only shown when anon users have the view userpoints permission?

c) Changed.

d) Changed.

e) Added. Do we really need to allow to add points to multiple categories and have that option per category? Not sure and not something we are doing anywhere else.

f) Changed. Looking at that, might it be better to only grant those points when the user account is *actived*. At least when requiring mail configuration, this *might* (not sure at this point) the display message problem from b)

g) Agreed, same problem when registering. But it's not possible to change that. Changing it to display it when the account is actived would work around it when mail confirmation is required...

Small patch attached to integrate the config gui better into the vertical tabs of admin/config/people/userpoints/settings

Can you provide a combined patch?

I checked #10, infact you've got the key bits from #11 already.

Tried to apply your patch to the git version but:
patch < 1132702-drupal7port-4.patch
patching file userpoints_register.info
The next patch would create the file userpoints_register.install,
which already exists! Assume -R? [n]
Apply anyway? [n]
Skipping patch.
1 out of 1 hunk ignored
The next patch would create the file userpoints_register.js,
which already exists! Assume -R? [n]
Apply anyway? [n]
Skipping patch.
1 out of 1 hunk ignored
patching file userpoints_register.module

Why does it want to create two new files?

After appling your patch as above, I checked the functionality.
In the settings there is now "Uncategorized points for registering " and "default points for registering". This breaks the urgarde path for D6 users, so there would have to be something in the README to indicate that this needs to be set manually after upgrading?

Anyway, lets get your patch into git so all these patches are no longer needed!

Uh, maybe you've already applied the patch before?

The patch applies just fine for me against the latest 7.x-1.x branch.

mkdir /tmp/hack
cd !$
git clone --branch 7.x-1.x http://git.drupal.org/project/userpoints_contrib.git
cd userpoints_contrib
wget http://drupal.org/files/issues/1132702-drupal7port-4.patch
patch < 1132702-drupal7port-4.patch
patching file userpoints_register.info
Hunk #1 FAILED at 1.
1 out of 1 hunk FAILED -- saving rejects to file userpoints_register.info.rej
patching file userpoints_register.install
patching file userpoints_register.js
patching file userpoints_register.module
Hunk #1 FAILED at 1.
1 out of 1 hunk FAILED -- saving rejects to file userpoints_register.module.rej

And why not just commit the above patch? Its a dev release.

This is a -p1 patch, try git apply instead of patch, that is the preferred way to apply patches now.

As you can see you now have a different error than you had above. I was able to apply the patch just fine using both git apply file.patch and patch -p1 < file.patch in a new git checkout.

Yes this is a -dev release but I am keeping my -dev releases as stable as possible. You can see in the 6.x-1.x branch of this project what happens when you commit everything to -dev, many of the modules there just don't work at all and most have serious bugs. The result is that this project will *never* see a stable 6.x-1.x version.

There is also an open question about keeping the info message setting, see #9 and #10.

Feel free to test this and if you think this is working without any issues, set it to RTBC and I'll commit it.

Ah, thanks - still getting used to git/issue patch exchanges.
So, pulled from git applied the patch cleanly and tested.
TEST: Works just fine, after user registration one sees a points entry with the reason "Created new user account", for the right amount of points, in the right category.

Please also ignore the patch in #11 which is irrelevant.

As regards info message, I think you are referring to "Even when the "display message" option is selected, new (anonymous) users are not seeing the points award message upon registration".
==> Well to me, I disabled that anyway immediately, it will only confuse the new user who doesn't even know the site yet. I'd suggest removing that option, or setting it as disabled by default.

Just posting some additional feedback

I applied port-4 from #10 and it appears to be working for me.

I also have 'Drupal for Facebook' project/fb installed. The userpoints_register module also worked for users created via the FBconnect interface

Any plans to bring the patch into the -dev release?

What about now? :) Commited.

Opened a follow-up issue for the broken setting: #1512556: Remove or fix userpoints_registration show message option..

Thanks. Now I don't to worry about it falling off if I re-download the userpoints contrib module :-)

At last, a commit :-)
FYI I'd created a sandbox,http://drupal.org/sandbox/boran/1512136, with several fixes, to be able to use userpoints_contrib on D7.