Sandbox: http://drupal.org/sandbox/thtas/1494902

Git repo: http://git.drupal.org/sandbox/thtas/1494902.git

git clone --recursive --branch 7.x-1.x http://git.drupal.org/sandbox/thtas/1494902.git commerce_justpay

A single page payment form

Provides a single page payment page where a user can enter payment information and an amount to pay.

Good for donations, settling invoices for services not managed by the current commerce system.

Provides an order with transactions against it much like a normal checkout process.
Provides a couple of alter hooks so additional form fields can be easily processed along with the order.

Notes from README.txt:

Only supports a single payment method at a time (but you could alter which method to use using provided hooks)

Requires a prodict be set up as the base product of the transaction. This is done automatically when you first access the payment form (if you have the proper permission)

requires the default setup for drupal commerce such that entity relationships exist like this:
$product->commerce_price
$order->commerce_customer_billing->commerce_customer_address

The default product type "product" must exist

**Depends on most of the commerce modules**

INSTRUCTIONS:

1. Install module
2. The form will work at /justpay
3. There is also a Commerce Justpay block which you can set up to show the payment form wherever you like
4. Use commerce rules to configure what happens after payment, by default you just get returned to the form.
5. Configure the product dummy JUSTPAYMENT sku to set the currency of the payment

Sponsored by Eighty Options

Comments

mayank-kamothi’s picture

mayank-kamothi commented

Hi,thtas

Manually Review

1) Remove unnecessary code like commented code.

2) Control statements should have one space between the control keyword and opening parenthesis,
if(!empty($form_state['values']['mail'])) {

Like
if (!empty($form_state['values']['mail'])) {

3)Put function comment in your module .

Mayank Kamothi

amitgoyal’s picture

amitgoyal commented
Status: Needs review » Needs work

1. Add git link like http://git.drupal.org/sandbox/thtas/1494902.git and version information in the issue summary.
2. Review your module from coder module and see the automated review here.
3. Remove the empty hook functions like hook_commerce_justpay_payment_method_alter(), hook_commerce_justpay_order_alter from commerce_justpay.api.php.\
4. when i installed the module the unrecoverable error occurred
Call to undefined function commerce_product_new() in /var/www/7/sites/all/modules/custom/commerce_justpay/commerce_justpay.install.
5. The menu link corresponding to justpay is not visible to user.
can use link like
$items['admin/config/people/justpay'] = array(
'title' => 'Pay',
'page callback' => 'drupal_get_form',
'page arguments' => array('commerce_justpay_form'),
'access arguments' => array('access commerce justpay form')
);

thtas’s picture

thtas commented
Status: Needs work » Needs review

Thanks for the comments. I've addressed all of the items mentioned.

amitgoyal’s picture

amitgoyal commented
Status: Needs review » Needs work

@thtas - I am still getting errors when I try to install/uninstall your module. It's happening because you haven't specified all the dependencies.

For example, you are calling commerce_product_load_by_sku() in commerce_justpay.install but that is in the commerce_product.module which in not in your commerce_justpay.info file's dependencies.

You need to test it on fresh drupal 7 install and keep adding required modules. Drush is the best way to enable/disable modules and test this.

Errors,
PHP Fatal error: Call to undefined function commerce_product_load_by_sku() in /var/www/d7/sites/all/modules/contrib/commerce_justpay/commerce_justpay.install on line 35

Notice: Undefined property: stdClass::$type in CommerceProductEntityController->save() (line 90 of /var/www/d7/sites/all/modules/contrib/commerce/modules/product/includes/commerce_product.controller.inc).

EntityMalformedException: Missing bundle property on entity of type commerce_product. in entity_extract_ids() (line 7562 of /var/www/d7/includes/common.inc).

amitgoyal’s picture

amitgoyal commented
Issue summary: View changes

adding link to git repository

thtas’s picture

thtas commented
Status: Needs work » Needs review

I think this is ready for another review.
I've done testing against a fresh Drupal install with the latest Drupal and the latest Commerce (as of this posting)

paravibe’s picture

paravibe commented
Status: Needs review » Needs work

Hello,

Please add a git clone link " git clone --recursive --branch 7.x-1.x http://git.drupal.org/sandbox/thtas/1494902.git commerce_justpay".

Manual review:
commerce_justpay.install: line 10, 17, 24, your hook function are empty, why did you leave them?
commerce_justpay.module: line 27, don’t use a MENU_NORMAL_ITEM as it used by default.
line 128: here should be a whitespace after 'commerce_justpay_submit_text' in variable_get().
line 151: else { must be in a new line, please fix all issues that automated test reports and read coding standarts as you have many errors with it.
See this report and please fix it http://ventral.org/pareview/httpgitdrupalorgsandboxthtas1494902git.

paravibe’s picture

paravibe commented
Issue summary: View changes

updated with new README.txt text detail

thtas’s picture

thtas commented
Status: Needs work » Needs review

Ok - all coding standards are fixed.

Can anybody tell me why function short description comments must be a single line of 80 characters?
What if i had more to say then could fit in to 80 characters, where would i put that text? README or something?

Edit: menu type removed, hooks in .install removed (there were there to set up configurations before i added an admin interface)

nchar’s picture

nchar commented
Status: Needs review » Needs work

Hi thtas,

I think your module is great! I am definitely going to use it. But there are a few critical issues i came up with and i think you need to fix them soon.

1. In commerce_justpay_form the payment details ajax refresh fails. It happens because the element's "payment_method" ajax callback which is "commerce_payment_pane_checkout_form_details_refresh" try to return $form['commerce_payment']['payment_details'] instead of $form['commerce_justpay_payment ']['payment_details']. I think an override of element's callback would fix this.

2. This is issue is more complex and difficult than the first one. When i enabled paypal as a payment method the checkout crashed. In function commerce_justpay_form_submit there is no functionality for payment methods that work with redirection like paypal WPS. I think that at this point your code is incomplete. You need to see how commerce handles these type of payment methods and modify your code accordingly.

Keep up your good work. I think you are close to create a very useful module.

thtas’s picture

thtas commented

Thanks for checking out the module, it's much appreciated.

I'll see If i can come up with a solution to those problems...

thtas’s picture

thtas commented

Ok I've added support for offsite payments with Paypal WPS.

Note: I've added a hook call in so that module authors can edit the return URLS for remote payments.
e.g. Paypal would return to checkout/XXX/payment, but we need to return to justpay/offsite instead.
So i've added a hook as follows

/**
 * Allows the offsite redirect form to be altered specifically for this module.
 *
 * @param array $form
 *   The just submitted single page checkout form
 * @param object $order
 *   The order object about to be paid for
 *
 * @see commerce_justpay_offsite_payment_form()
 */
function hook_commerce_justpay_CALLBACK_alter(&$form, $order) {
  // This example modifies the Paypal WPS return URLS.
  $form['return']['#value'] = str_replace('checkout/' . $order->order_id . '/payment', 'justpay/offsite', $form['return']['#value']);
  $form['cancel_return']['#value'] = str_replace('checkout/' . $order->order_id . '/payment', 'justpay/offsite', $form['cancel_return']['#value']);
}

Remote payments really aren't as elegant a solution here as the direct web service calls, I'm not sure how much extra time I want to put in to this to support that feature...

thtas’s picture

thtas commented
Status: Needs work » Needs review

changing status

pere orga’s picture

pere orga
Primary language Catalan
commented

Hi,

commerce_justpay.info:
Package should be 'Commerce (contrib)' instead of 'Commerce'.
Please end the description with a dot.


commerce_justpay.module:
I detected some style issues. This
function commerce_justpay_base_product($sku=NULL) {
should be:
function commerce_justpay_base_product($sku = NULL) {


I think you don't need to use filter_xss() if you already use placeholders inside t(), just use:
drupal_set_message(t('A product with SKU %sku has been automatically generated, if you want to use a different base product for Commerce Justpay, configure it !here', $vars));
instead of:

  $message = t('A product with SKU %sku has been automatically generated, if you want to use a different base product for Commerce Justpay, configure it !here', $vars);
  drupal_set_message(filter_xss($message));
pere orga’s picture

pere orga
Primary language Catalan
commented
Status: Needs review » Needs work
thtas’s picture

thtas commented

I've addressed the first two issues, but the filter_xss function is there because the code review module gives this critical error without it:

Line 346: Potential problem: drupal_set_message() only accepts filtered text, be sure all !placeholders for $variables in t() are fully sanitized using check_plain(), filter_xss() or similar. (Drupal Docs)
drupal_set_message(t('A product with SKU %sku has been automatically generated, if you want to use a different base product for Commerce Justpay, configure it !here', $vars));

My solution to wrap the message in filter_xss seemed to make it happy, but it does look funny.

Is there a better way to include a URL in drupal_set_message?

pere orga’s picture

pere orga
Primary language Catalan
commented

You used the bad !placeholder. Just use %here or @here and everything should be fine.

thtas’s picture

thtas commented
Status: Needs work » Needs review

I can't use % or @ because it will escape all the html in the link i'm trying to display and render it useless.

I decided to just copy how Drupal core deals with this scenario - see node.module line 75 in node_help

So mine ended up like this:

$vars = array(
      '@config_url' => url('admin/commerce/config/justpay'),
      '%sku' => $sku,
    );
    drupal_set_message(t('A product with SKU %sku has been automatically generated, if you want to use a different base product for Commerce Justpay, configure it <a href="@config_url">here</a>', $vars));
subhojit777’s picture

subhojit777
he/him
Primary language Bengali
commented

There are some Drupal coding standard mistakes http://ventral.org/pareview/httpgitdrupalorgsandboxthtas1494902git

Rest seems ok

Nice module!

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented

We are currently quite busy with all the project applications and I can only review projects with a review bonus. Please help me reviewing and I'll take a look at your project right away :-)

vladimir-m’s picture

vladimir-m commented

Hello thtas,

Thank you for great module.

Manual review:
1. Remove also variable "commerce_justpay_exlude_order_fields" what is present in file: commerce_justpay.admin.inc at line: 35 and other places.
2. In file: commerce_justpay.module at line: 182, function description in wrong "hook_submit().", I think this is form submit handler.
3. In file: commerce_justpay.module at line: 185, was initialized variable $user but it is not used in commerce_justpay_form_submit() function.

vladimir-m’s picture

vladimir-m commented
Status: Needs review » Needs work

changed status

PA robot’s picture

PA robot commented
Status: Needs work » Closed (won't fix)

Closing due to lack of activity. Feel free to reopen if you are still working on this application.

I'm a robot and this is an automated message from Project Applications Scraper.

thtas’s picture

thtas commented
Status: Closed (won't fix) » Needs review

Thanks for the manual review Vladimir!

I've committed the fixes mentioned in #19

erez111’s picture

erez111 commented
Status: Needs review » Needs work

Hi, check
http://ventral.org/pareview/httpgitdrupalorgsandboxthtas1494902git
and solve all errors/warnings.

Except that, code is functionality is working and it seems ok to me.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented

Minor coding standard errors are not application blockers, please do a manual review.

thtas’s picture

thtas commented
Status: Needs work » Needs review

Fixed minor standards issues - Thanks erez111.

cafuego’s picture

cafuego commented
Status: Needs review » Reviewed & tested by the community

Looks good to me and coder is happy as well.

I'd maybe remove the dependency on commerce_product because commerce_product_ui depends on that anyway, but I don't see that should stop this from being a full contrib module and I don't need to pick nits ;-)

kscheirer’s picture

kscheirer
Primary language English
Location Vallejo
commented
Title: Commerce Justpay » [D7] Commerce Justpay
Status: Reviewed & tested by the community » Needs work

The validation code in commerce_justpay_form_validate() seems to allow negative numbers and zero. I'm not sure how commerce would handle that but it doesn't sound like something you want to have happen. I'm not sure if that's a major issue, can you please provide more info about that?

Otherwise, this module looks good to me and ready for promotion.

thtas’s picture

thtas commented
Status: Needs work » Needs review

You're right Kscheirer, that could cause issues.
Most likely the payment method in use would throw some kind of error, but you never know - some might allow me to enter -$1000000 and credit me the amount!

So I've added an extra check for positive amounts in the validate function.

kscheirer’s picture

kscheirer
Primary language English
Location Vallejo
commented
Status: Needs review » Fixed

Thanks for resolving that, based on cafuego's RTBC this module looks good and is ready to go.

Thanks for your contribution, thtas!

I updated your account to let you promote this to a full project and also create new projects as either a sandbox or a "full" project.

Here are some recommended readings to help with excellent maintainership:

You can find lots more contributors chatting on IRC in #drupal-contribute. So, come hang out and get involved!

Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

Thanks to the dedicated reviewer(s) as well.

----
Top Shelf Modules - Enterprise modules from the community for the community.

cafuego’s picture

cafuego commented

Nice one, thanks kscheirer.

thtas’s picture

thtas commented

Thanks everybody for testing and reviewing!

Status: Fixed » Closed (fixed)

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

Anonymous’s picture

(not verified) commented
Issue summary: View changes

adding git link