Notice and Warning messages for nap_node_grants() (line 799 of \sites\all\modules\nap\nap.module)

subscribe

I made a quick code change regarding the first error but I'd have to do some testing to find the cause of the second one.

not sure but i suspect maybe there is a ":types" and ":types_something_else" and it is causing a confusion? Perhaps I will prefix the placeholder with 'nap_'

Hmm I found a couple more small problems which might affect this. You'll have to resave the config page for the module.

Is this still an issue?

Can't speak for the original poster, but I had an issue similar to #5 which was resolved by disabling and uninstalling NAP & NAP UC, then reinstalling it.

Rather new to all this, but I believe I have resolved this issue, and many others regarding the move to Drupal 7 and Ubercart 3.x, in the attached patch. Would love extra eyes on this.

My QA found an additional issue, but with the attached patch, this module has passed our QA. The issue at hand was the usage of "0000-00-00 00:00:00" as the default value for the expiry_date field, where Drupal 7 uses the STRICT_ALL_TABLES option. Have converted checks to NULL instead.

There are some seriously questionable things in those patches, I think we'll have to dissect each change bit by bit.

I'm not sure what is questionable in the patches. Can you expand on that?

I think a lot of things have been snuck in that don't relate to this issue and while they may be positive changes they really could have used some discussion or a separate issue to properly address them, there are a number of drupal coding standards issues, some changes don't seem to change anything just added white space or slightly change the format of the code, and the patches are done against a packaged version not a git checkout which adds a lot of junk to the files that shouldn't be there.

There a lot of good changes too, and I thank you for spotting those, but I'm at a loss to figure out which ones are necessary to solving the current issue.

You've shed some light on a few major problems that were missed during the upgrade to Drupal 7 but at the same time the patch is quite busy and I'm worried I may commit something that while it fixes the problem for you, may introduce issues for people using the module in a different way.

I might just have to manually do some cumulative improvements and fixes based on your patch and we can then revisit whether this issue continues to be a problem. I may need your advice on the purpose of some of the changes when I get down to it.

Hopefully I will work on it very soon.

I need some feedback about this.

This code appears to remove CA integration and replaces it with rules. How will this affect users relying on the current CA functionality?

 /**
  * Implements hook_nap_save_purchase().
  */
 function uc_nap_nap_save_purchase($nap_purchases, $nap_transaction) {
-  if (module_exists('ca')) {
+  if (module_exists('rules')) {
     $order = uc_order_load($nap_transaction->txn_id);
-    ca_pull_trigger('uc_nap_purchase', $order);
+    rules_invoke_event('uc_nap_purchase', $order);
   }
 }

I've committed the changes I think are necessary, some I think do not change anything, and a couple I disagree with so I haven't added those.
I'd like to hear about this ca/rules thing, other than that I think any other changes should be discussed in a separate issue.

And I think if at this point the module is working we should consider making a tagged release of D7 finally

To my knowledge, conditional actions have been removed for Ubercart 3.x and replaced with Rules.

http://www.ubercart.org/forum/support/22086/where_are_conditional_action...

Of course, you are right about my changes being a lump sum and probably not the way things are not normally done, so I apologize for that. I'll just address specific issues in the future.

Thanks, I've made that change too, and created a release candidate.
Can sminons or anyone else provide feedback about whether any of the original error message persist with the latest code?

I think more needs to be done with this rules thing. There is still a hook_ca_trigger() there instead of hook_rules_event_info(), and perhaps all of those should be moved into nap.module not uc_nap as rules is more generic than ca was. Nevermind that last bit, the code within there is uc specific.

something like this?

<?php
/**
 * Implements hook_rules_event_info().
 */
function uc_nap_rules_event_info() {
  $events['uc_nap_purchase'] = array(
    'label' => t('Order placed containing at least one NAP product.'), 
    'group' => t('Order'), 
    'variables' => array(
      'order' => array(
        'type' => 'uc_order', 
        'label' => t('Order'),
      ), 
    ),
  );
  return $events;
}
?>