Drupal 7 port (userpoints_download)

You're on a roll it seems. Awesome!

Since file downloading change a lot between D6 and D7, it might also affect how this module works. I have a lot of ideas and wrote quite a lot below. I suggest you we discuss it through (BenK might have some good ideas too) before you start working on those ideas, might be quite a bit of work :)

You might also want to check out what BenK and I did with http://drupal.org/project/userpoints_nodeaccess in D7 for some ideas.

+++ b/userpoints_download/userpoints_download.infoundefined
@@ -1,6 +1,6 @@
-dependencies[] = upload

We might want to add a dependency to file.module, see below for more about that.

+++ b/userpoints_download/userpoints_download.installundefined
@@ -0,0 +1,33 @@
+/**
+ * Implementation of hook_install().
+ */
+function userpoints_download_install() {
+  if (!userpoints_download_check_method()) {
+    drupal_set_message(t('Download method is not set to private. In order for users to earn or lose points for downloads, the !method has to be changed to private.',
+      array('!method' => l('download method', 'admin/config/media/file-system'))), 'error');

We might want to just get rid of that check. In contrast to D7, the private/public setting in D7 is just a default and can be overriden for every field. Maybe just inform that this only works for private files somewhere in a description?

+++ b/userpoints_download/userpoints_download.moduleundefined
@@ -1,101 +1,157 @@
+ * Depends on upload.module, and private file system configuration.

Outdated comment :)

+++ b/userpoints_download/userpoints_download.moduleundefined
@@ -1,101 +1,157 @@
-      drupal_set_message(t('Download method is not set to private. In order for users to earn or lose points for download, the !method has to be changed to private.', array('!method' => l('download method', 'admin/settings/file-system'))), 'error');
+      drupal_set_message(t('Download method is not set to private. In order for users to earn or lose points for downloads, the !method has to be changed to private.',

I see, we already have that as a dsm() message here too. Still, I think we only want that as a description inside the vertical tab of this module (maybe something along the lines of "Default file download method is not private. !Points can only be added or subtracted for files that use private download method."

+++ b/userpoints_download/userpoints_download.moduleundefined
@@ -1,101 +1,157 @@
+      '#description' => t('!Points a user will earn, or lose when they download an attachment. Note that this module requires that the upload module be enabled, '.
+        'and that the file download method is set to private. Otherwise, you will get unexpected results.', userpoints_translation()),

Another reference of upload.module. Also, this is exactly the place where my suggested text from above should go.

+++ b/userpoints_download/userpoints_download.moduleundefined
@@ -1,101 +1,157 @@
+    /* To do: Add variable costs and categories per extension - perhaps display as a table? */

I'm not sure a per extension setting makes much sense... Maybe allow to configure it per file field instead of per file extension? So you could have a file field with stuff that costs 10 userpoints and one with stuff that costs 50 points? Even if they have the same extension?

That might not be an easy thing to do, but to get you started, you could form_alter() yourself in http://api.drupal.org/api/drupal/modules--field_ui--field_ui.admin.inc/f..., check the field type and if it's file, insert your form fields into $form['instance']. It will automatically get saved together with the other field settings and be accessible through http://api.drupal.org/api/drupal/modules--field--field.info.inc/function....

We can also wait from some ideas from BenK before getting started here.

+++ b/userpoints_download/userpoints_download.moduleundefined
@@ -1,101 +1,157 @@
+function userpoints_download_file_download($uri = NULL) {

I'm wondering if we want to use http://api.drupal.org/api/drupal/modules--file--file.api.php/function/ho... instead, which is only for file (and image) fields. That would for example allow to store the entity_type and id for of a file, which in turn would allow to link to it in /myuserpoints. Especially if we are going to allow configure this per field anyway.

+++ b/userpoints_download/userpoints_download.moduleundefined
@@ -1,101 +1,157 @@
+    if (!$disallow || userpoints_download_check_points($file_ext)) {
+      // Charge the user for the download.
+      $params = array(
+        'points' => $points,
+        'tid' => $category,
+        'uid' => $user->uid,
+        'operation' => 'userpoints_download',
+      );
+      userpoints_userpointsapi($params);
+      return NULL; // Either allowed to go negative, or user had enough points.
+    }
+    else {
+      return -1; // Not allowed.

I'm wondering if we should keep a record of files that were already accessed, similar to how userpoints_nodeaccess keeps a record for nodes that you got access to with userpoints? Would need a new table for that and should probably also be configurable.

+++ b/userpoints_download/userpoints_download.moduleundefined
@@ -1,101 +1,157 @@
+/*
+ * Helper function to check currently configured download method.
+ * Return true if private download is enabled.
+ */
 function userpoints_download_check_method() {
-  if (variable_get('file_downloads', FILE_DOWNLOADS_PUBLIC) == FILE_DOWNLOADS_PRIVATE) {
+  if (file_default_scheme() == 'private') {
     return TRUE;
   }
   else {
-    // File downloads are not set to Private, do nothing ...
     return FALSE;
   }

See above, that check isn't really helpful. The default might be public (and used for css/js aggregation for example) while all file fields are private.

+++ b/userpoints_download/userpoints_download.moduleundefined
@@ -1,101 +1,157 @@
+ */
+function userpoints_download_check_points($extension) {
+  global $user;
+  $current_points = userpoints_get_current_points($user->uid, variable_get('userpoints_download_category_'. $extension, 0));
+  $file_points = variable_get('userpoints_download_points_'. $extension, 0);
+  if ($current_points >= $file_points) {
+    return TRUE;
+  }
+  else {
+    return FALSE;
+  }
+}
\ No newline at end of file

Based on whatever we decide above, we might want to change this function to check against a specific field instead of an extension.

Powered by Dreditor.

My current goal is to try to do one of these a day, on average, until we're done. ;) I have the spare time to code at work right now so I wanted to put it to good use.

That's awesome. If you want to try something else, there is also a number of open issues (both bugs and feature requests) for userpoints.module, userpoints_nc.module and userpoints_nodeaccess.module, so if you want to try to fix some of those too, you're more than welcome ;)

re per field settings: Yes. My suggestion would be do deal with this similarly as we do with per content type settings for example. Have a default setting on the userpoints settings page, which can be overridden for each field. We can then have an api function to which you pass the name of the setting you want (points, category, ...) and the $field array. Then, it can first check if there is a field specific setting and if not, fall back to the default. See http://api.worldempire.ch/api/userpoints/userpoints_nc-7.x-1.x--userpoin... for an example.

re access counter: If we keep track of which user got access to which files then we have an access counter already built int, we just need to add a query that counts the number of rows per file.

Sounds interesting... subscribing to contribute to the brainstorming! ;-)

Hey Bastlynn,

Also, if you're interested, you might want to join the conversation at this issue:

http://drupal.org/node/985948

Berdir and I have discussed for awhile the concept of a Userpoints Flood Control module. It would impact a lot of these Userpoints Contrib sub-modules because it would allow you to set limited on the number of points that can awarded (or even spent) over a given period of time. Kind of a way to prevent gaming of the system.

So I'd love to see your suggestions there, too! ;-)

--Ben