Lots of errors :(

Great concept to this but unfortunately to many errors to be usable. I fixed a few of them but gave up after running into more.

First off, I'm using Drupal 6.19 and ubercart 2.x

The File Handler routine:

function uc_file_retro_form_submit($form_id, $form_state, $form) {

will always fail with a missing argument because the form handler only submits two arguments. I dropped the $form argument off this and it gets past this problem.

Next when it creates the tables on install it's not creating the "description" field on the uc_file_retro_que table, preventing it from even adding stuff to the que at all. I added the field but didn't look into why it did not create it on it's own.

After doing this and finally figuring out how to actually process the que, the new file does not inherit the correct properties. My previous files for this product are set to a 1 year expiration, unlimited download limit, and 3 ip address limit. Well the qued file shows up with no expiration, unilimited download, and unlimited ip addresses. I have not looked further into it to see why yet, might if I'm feeling spunky tomorrow but at this time I'm giving up will probably be just more errors to fix after fixing this one.

CommentFileSizeAuthor
#9 934204-api-changes.patch9.72 KBagentrickard

Comments

steven6282’s picture

steven6282 commented
Status: Active » Fixed

For those that want to use this module but are having the same problems I have here is a new function uc_file_retro_batch_process that will correct the problems with the added files not inheriting the original settings. The way I coded it is to go the same time interval from the original purchase date, not from the date you add it so you may want to change that. Feel free to message me if you need help with it.

function uc_file_retro_batch_process($qid, $nid, $model, $fid, $pfid, $count, &$context) {
  global $user;

  // TODO: Make this a config somewhere.
  $limit = variable_get('uc_file_retro_batch_limit', 100);

  // Find all orders
  $orders = db_query("SELECT uop.*, uo.*, u.name FROM {uc_order_products} uop LEFT JOIN {uc_orders} uo ON uop.order_id = uo.order_id LEFT JOIN {users} u ON u.uid = uo.uid
    WHERE uop.nid = %d AND uop.model = '%s' AND uo.order_id > %d ORDER BY uo.order_id LIMIT %d", $nid, $model, $context['sandbox']['current_order'], $limit);

  // Loop through all users who have bought this product feature and add / renew the files.
  // TODO: Check expiration rules for this file.
  while ($order = db_fetch_object($orders)) {
		$file_limits = array();
		
    if (!isset($context['sandbox']['progress'])) {
      $context['qid'] = $qid;
      $context['sandbox']['progress'] = 0;
      $context['sandbox']['current_order'] = $order->order_id;
      $context['sandbox']['max'] = $count;
    }

    // TODO: Check to make sure the user doesn't already have the file.
    $context['results'][] = array('order_id' => $order->order_id, 'user_id' => $order->uid, 'username' => $order->name, 'purchased' => $order->created);
    $context['sandbox']['progress']++;
    $context['sandbox']['current_order'] = $order->order_id;
    $context['message'] = 'Currently processing order #'. $order->order_id;
    $account = user_load($order->uid);

		//Set up file limits.
		$file_limits['download_limit'] = variable_get('uc_file_download_limit_number', NULL);
    $file_limits['address_limit'] = variable_get('uc_file_download_limit_addresses', NULL);

    $file_limits['expiration'] = _uc_file_expiration_date(array(
      'time_polarity' => '+',
      'time_quantity' => variable_get('uc_file_download_limit_duration_qty', NULL),
      'time_granularity' => variable_get('uc_file_download_limit_duration_granularity', 'never'),
    //), time());
    ), $order->created);
    
    if ($file_limits['expiration'] > time())
    {
	    // Add the file.
	    uc_file_user_renew($fid, $account, $pfid, $file_limits, TRUE);
	
	    // Add order comment
	    $filename = db_result(db_query("SELECT filename from {uc_files} where fid = %d", $fid));
	    uc_order_comment_save($order->order_id, $user->uid, t('Retroactively added file %filename ', array('%filename' => $filename)));
  	}
  }

  // Finished is when the max and the current order are the same.
  if ($context['sandbox']['current_order'] != $context['sandbox']['max']) {
    $context['finished'] = $context['sandbox']['progress'] / $context['sandbox']['max'];
  }
}

Oh and as for the "description" field not being created, thats because it's not even in the schema in the install file so of course it would not create it.

torgospizza’s picture

torgospizza
he/him
Primary language English
Location Portland, OR
commented
Status: Fixed » Needs review

Since this isn't fixed, please don't mark it as fixed. I'd love to patch this up but I just found this issue now.

Thanks for the report. Will mark this as "needs review" until I can get to it and issue a new release.

TY2U’s picture

TY2U commented

Hi there,

Thank you for this module! I was looking for something to do this for a long time before I came across this. I don't understand why file downloads is all messed up in UC. This module wasn't working properly either so I came here and tried the patches in this thread. I also recreated the database table with the description field. It now will process the queue actually and list the orders it affects. However, no orders are changed. Users still have the same files on their accounts. Am I missing something?

Thanks =)

torgospizza’s picture

torgospizza
he/him
Primary language English
Location Portland, OR
commented

I'm confused. Are you saying that the NEW files you're adding via this module are not actually being added to their accounts? That's what uc_file_user_renew() does. I'm wondering if the patch above is using an expiration date and you don't have any setup? We don't use expiration dates on our site, so I never did test that aspect of it, though I'd have to dig into the code a bit more to see if the (sensible?) default works without needing to be specified.

TY2U’s picture

TY2U commented

Thanks for the quick response =)

Yes I think you may have just nailed it on the head. I will do a test and tell you step by step what I am doing.

  1. I Edit my product features. I only have 1 product on the store at the moment.
  2. Click Add to add a file download.
  3. Select SKU 1 and the new version of the file I want, and enter a description.
  4. Check off "Queue for retroactive purchase" and click SAVE FEATURE.
  5. It says:
    * The product feature has been added.
    * Queued product feature 10 for 1
  6. I now have 1 feature for Role assignment and 1 for File download which I just created.
  7. Now I go to /admin/store/products/files/queue
  8. It shows affected rows as 11.
  9. I click Submit.
  10. It shows me:
    Processing file queue
    100%
    Processing 1 out of 1
    Currently processing order #18
  11. The takes me to another screen:
    File Process Results
    11 orders were processed:
    with all the orders listed in here
  12. And below that is a "« Go back to the Queue" link
  13. Clicking that brings me back to the queue screen with the queue listed in there still looking identical to how it did before I clicked Submit.

Now checking a users Files tab they have the old file but not the new one I just tried to add.

I do have it set to allow unlimited downloads and no expiration date so maybe this is indeed the problem.

torgospizza’s picture

torgospizza
he/him
Primary language English
Location Portland, OR
commented

Yeah that could be it. I'll have to test with that patch. The weird thing is, the module works fine for me without the patch, so I'm not quite sure without digging in. I'll see what I can do this week - pretty busy lately but I'll do my best.

TY2U’s picture

TY2U commented

Well, I think I fixed it.

I added this OR part to it. I don't know if it should be NULL or -1 but it works now. Added the file to the users =)

    if ($file_limits['expiration'] > time() || $file_limits['expiration'] == NULL)
    {
    // Add the file.
    uc_file_user_renew($fid, $account, $pfid, $file_limits, TRUE);
torgospizza’s picture

torgospizza
he/him
Primary language English
Location Portland, OR
commented

Yeah, TBH I'm not even sure the expiry stuff needs to be there - I could've sworn that uc_file_user_renew() does that on its own accord. I'll see if I can work that angle and see what that code really needs to be.

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
commented
Status: Needs review » Needs work
StatusFileSize
new 934204-api-changes.patch9.72 KB

Here's a proper patch the fixes the following errors:

1) Removed $form['#validate'][] = 'uc_file_retro_form_validate'; since the validation function does not exist.

2) Fixes arguments for function uc_file_retro_form_submit

3) Correct bad usage of "in_queue" and "processed" values for 'status' column. Replaces with 0 and 1, since the actual schema defines uc_file_retro.status as an integer column.

4) Wraps All SKUs message in t() properly.

5) Standardizes SYNTAX CASE in SQL statements.

6) Properly handles the 'All SKU' value, which means that any model should be selected.

7) Ups the batch limit from 100 to 1000. TODO: need better handling here, so that unfinished processes can be completed later.

8) Adds batch result data so that a queue can be marked complete properly. NEEDS WORK.

9) Fixes the function call to uc_file_user_renew, which was missing the final two parameters.

10) Properly calculates $file_limits being passed to uc_file_user_renew

11) Adds the 'description' column to uc_file_retro_schema

12) Provides uc_file_retro_update_6002 to add the 'description' column for existing installs.

Working more like expected now. The batch handling for "completed" vs. "partial" updates still needs a lot of work, though.

torgospizza’s picture

torgospizza
he/him
Primary language English
Location Portland, OR
commented

Wow, sweet. Thanks, Ken! Will try to commit your patch this week.

netentropy’s picture

netentropy commented

Hey thanks for the module and the patch but the patch fails when I attempt to patch

patching file uc_file_retro.module
Hunk #1 FAILED at 53.
Hunk #2 FAILED at 62.
Hunk #3 FAILED at 81.
Hunk #4 FAILED at 107.
Hunk #5 FAILED at 123.
Hunk #6 FAILED at 153.
Hunk #7 FAILED at 185.
Hunk #8 FAILED at 210.
Hunk #9 FAILED at 252.
Hunk #10 FAILED at 291.
10 out of 10 hunks FAILED

How can I fix this?

torgospizza’s picture

torgospizza
he/him
Primary language English
Location Portland, OR
commented

Let me work on getting the patch applied. I didn't get a chance to yet. Once the patch is in, I'll commit a new -dev version. Sorry for the hold up.

netentropy’s picture

netentropy commented

great thanks! we are doing a huge migration next week and this would be so helpful. if not i will have to recreate all the hash keys another way.

myregistration’s picture

myregistration commented

Hi, Any idea when you'll commit the next dev? Thanks! :)

torgospizza’s picture

torgospizza
he/him
Primary language English
Location Portland, OR
commented

Committed this to the git repo (my first git commit!)... sorry for the delay. I'll try to roll a point release soon. I noticed a couple other problems with the code itself (like using db_query("insert...") rather than drupal_write_record. I'll remedy that and roll a patch then commit those to git and hopefully an actual release! :)

torgospizza’s picture

torgospizza
he/him
Primary language English
Location Portland, OR
commented
Status: Needs work » Fixed

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.