Patch for MS Membership suite

2 replies [Last post]
Mon, 04/05/2010 - 20:33
Evolver
Offline
Membership Suite
Pay to Publish
Affiliates Suite
File Downloads Suite
Bonus Pack
Pay Per View
Premium Gateways Pack
Joined: 03/12/2010
Status: 
Answered

This patch (below) fixes the following bugs in ms_membership-5.x-5.8.  I am aware this is not the latest version of the module;

  1. Add plan form validation incorrectly fails under certain criteria (According to 5.9's release notes, it seems this was also fixed there)
  2. Success message when added a new plan is missing the new plan's name
  3. 'PayPal History' tab is displayed to anonymous users

The following recommendations are made with @todo annotations in the patch;

  1. Merge the form/validate/submit functions for the add_plan and edit_plan forms into one set of form/validate/submit functions for both forms.  This would mean that bug #1 above would have been found and fixed a lot sooner — and probably not released, because there would be better code-reuse.
  2. Convert the paypal API form to use the Form API (or perhaps a theme function).  This would allow a developer to make various fields customizable by the end user.  E.g. 
    http://www.realitysandwich.com/esm/join-evolver-social-movement-esm

Further, I recommend you use Drupal's code syntax standards and use the coder module to learn them and fix code style.

These changes are recommended because the modules do not, generally, adhere to Drupal coding standards, despite MoneyScript.net's claims to "follow Drupal Best Practices".

  1. Only in moneyscripts: .svn
  2. Only in moneyscripts: moneyscripts.patch
  3. Only in moneyscripts/ms_charts: .svn
  4. Only in moneyscripts/ms_charts/includes: .svn
  5. Only in moneyscripts/ms_charts/js: .svn
  6. Only in moneyscripts/ms_helper: .svn
  7. Only in moneyscripts/ms_membership: .svn
  8. Only in moneyscripts/ms_membership/includes: .svn
  9. Only in moneyscripts/ms_membership/js: .svn
  10. diff -rup orig/moneyscripts/ms_membership/ms_membership.module moneyscripts/ms_membership/ms_membership.module
  11. --- orig/moneyscripts/ms_membership/ms_membership.module        2010-02-20 20:13:49 -0800
  12. +++ moneyscripts/ms_membership/ms_membership.module     2010-04-05 20:01:58 -0700
  13. @@ -2014,6 +2014,7 @@ function ms_membership_process_ipn($type
  14.  
  15.  // ======================================
  16.  // FORMS
  17. +// @todo: Merge the form/validate/submit functions for the add_plan and edit_plan forms into one set of form/validate/submit functions for both forms.
  18.  // ======================================
  19.  
  20.  function ms_membership_add_plan_form() {
  21. @@ -2991,7 +2992,7 @@ $v = $form_values;
  22.      form_set_error('srt', t('Number of Payments must be positive.'));
  23.    }
  24.    // Validate Number of Payments
  25. -  if (($v['srt'] < 1) && $v['src']) {
  26. +  if (($v['srt'] < 0 || $v['srt'] == 1) && $v['src']) {
  27.      form_set_error('srt', t('You must enter more than 1 payment if you are using Recurring Subscription'));
  28.    }
  29.    // Validate Number of Payments
  30. @@ -3046,7 +3047,7 @@ function ms_membership_edit_plan_form_su
  31.      $v['exp_rid'], $v['mpid']
  32.      );
  33.    
  34. -  drupal_set_message(t('Membership Plan Saved: %sname.', array('%sname' => $m_plan->item_name)));
  35. +  drupal_set_message(t('Membership Plan Saved: %sname.', array('%sname' => $v['item_name'])));
  36.  }
  37.  
  38.  
  39. Only in moneyscripts/ms_paypal_api: .svn
  40. Only in moneyscripts/ms_paypal_api/includes: .svn
  41. diff -rup orig/moneyscripts/ms_paypal_api/includes/ms_paypal_api_class.php moneyscripts/ms_paypal_api/includes/ms_paypal_api_class.php
  42. --- orig/moneyscripts/ms_paypal_api/includes/ms_paypal_api_class.php    2010-01-01 14:14:26 -0800
  43. +++ moneyscripts/ms_paypal_api/includes/ms_paypal_api_class.php 2010-04-05 20:05:56 -0700
  44. @@ -18,6 +18,7 @@ class ms_paypal_api_class {
  45.     }
  46.  
  47.     function submit() {
  48. +     // @todo: To adhere with Drupal conventions, convert this to use the Form API or a theme function.
  49.        $form = "<form id='paypalPaymentForm' method='post' name='paypal_form' action='". $this->ipnLink ."'>";
  50.        foreach ($this->values as $name => $value) {
  51.          $form .= "<input type='hidden' name='$name' value='$value' />";
  52. diff -rup orig/moneyscripts/ms_paypal_api/ms_paypal_api.module moneyscripts/ms_paypal_api/ms_paypal_api.module
  53. --- orig/moneyscripts/ms_paypal_api/ms_paypal_api.module        2010-01-22 07:21:57 -0800
  54. +++ moneyscripts/ms_paypal_api/ms_paypal_api.module     2010-04-05 20:03:03 -0700
  55. @@ -39,6 +39,8 @@ function ms_paypal_api_access_pages_test
  56.   * Implementation of hook_menu
  57.   */
  58.  function ms_paypal_api_menu($may_cache) {
  59. +  global $user;
  60. +
  61.    $items = array();
  62.    if (!$may_cache) {
  63.      $uid = is_numeric(arg(1)) ? arg(1) : $user->uid;
  64. @@ -71,15 +73,19 @@ function ms_paypal_api_menu($may_cache)
  65.        'callback arguments' => array('ms_paypal_api_admin'),
  66.        'access' => user_access('administer paypal api'),
  67.        'type' => MENU_NORMAL_ITEM,
  68. -     );
  69. -    $items[] = array(
  70. -      'path' => 'user/'. $uid .'/paypal-history',
  71. -      'title' => 'PayPal History',
  72. -      'callback' => 'ms_paypal_api_history',
  73. -      'callback arguments' => array($uid),
  74. -      'access' => ms_paypal_api_access_test($uid),
  75. -      'type' => MENU_LOCAL_TASK,
  76.      );
  77. +
  78. +    // Don't show this tab to anonymous users.
  79. +    if ($uid > 0) {
  80. +      $items[] = array(
  81. +        'path' => 'user/'. $uid .'/paypal-history',
  82. +        'title' => 'PayPal History',
  83. +        'callback' => 'ms_paypal_api_history',
  84. +        'callback arguments' => array($uid),
  85. +        'access' => ms_paypal_api_access_test($uid),
  86. +        'type' => MENU_LOCAL_TASK,
  87. +      );
  88. +    }
  89.    }
  90.  
  91.    return $items;

‹ Token interaction with MoneyScripts Coupons with Membership Suite - X Months for Free? ›
Top
Mon, 04/05/2010 - 21:44
#1
Leighton Whiting
Offline
Membership Suite
Pay to Publish
Affiliates Suite
File Downloads Suite
Bonus Pack
Pay Per View
Premium Gateways Pack
Joined: 06/02/2009
Bevan, First, thanks for your

Bevan,

First, thanks for your patch and the suggestions, they are much appreciated. I have already fixed most of the things you mentioned in Membership Suite 6.0, and I will be adding the other things in before release.

One thing though, I cannot use the Forms API with the payment gateway 'submit' forms because they submit their values to other sites and domains, and the Forms API wasn't built to handle that. I suppose I could build the form and then submit the values of it manually, but I think it would be more intuitive to just expose a hook for other modules to alter the values before they are sent off. With the new system, this will be possible as well.

I run my modules through Coder to clean things up every so often, but sometimes some badly formatted code gets into a release. I actually just got done running Membership Suite 6.0 through coder last night, so I can assure you that I do regularly use that great tool :)

Again, thanks for your help and contribution!

Sincerely,
Leighton Whiting

Top
Tue, 04/06/2010 - 09:47
#2
Leighton Whiting
Offline
Membership Suite
Pay to Publish
Affiliates Suite
File Downloads Suite
Bonus Pack
Pay Per View
Premium Gateways Pack
Joined: 06/02/2009
Just wanted to follow up on

Just wanted to follow up on this to say that the Edit and Add Membership Plan forms have been consolidated into one form, preserving the same functionality. :)

Sincerely,
Leighton Whiting

Top

Twitter Feed