This module provides integration with a 3rd party hosted e-commerce service called ShopIntegrator (http://www.shopintegrator.com).

Shop Integrator uses JavaScript ecommerce buttons which are pasted into the website to add a shopping cart and secure online store checkout.

This module provides the following:

  • Admin Configuration Screen
  • Choice of text field formatters for buy buttons.
  • Basket summary Block.
  • View/Basket Checkout Block.
  • Automatically adds required JavaScript for easy integration.

To fully review this module you will need to create a free 30 day trial account with Shop Intergrator: https://admin.shopintegrator.com/public/Register.do

Project page: http://drupal.org/sandbox/sittard/1774774
GIT repository: git clone http://git.drupal.org/sandbox/sittard/1774774.git shop_integrator

The module is for Drupal 7

Reviews of Other Projects:
http://drupal.org/node/1807430#comment-6618262
http://drupal.org/node/1539234#comment-6676312
http://drupal.org/node/1820260#comment-6675868

CommentFileSizeAuthor
#1 shop_intergration.txt8.15 KBweebpal

Comments

weebpal’s picture

weebpal commented
Status: Needs review » Needs work
StatusFileSize
new shop_intergration.txt8.15 KB

I'd like to give some small manual review: 

    '#markup' => '<script type="text/javascript">if(window.shopActive){showMiniBasketList();}</script><noscript><img src="http://ecommerce.shopintegrator.com/ecommerce.png?ci=' . $id . '&op=smbl" border="0"/></noscript>',

- we may use drupal_add_js is quite better in this situation
- you still miss some t() function for the title and description
- And please make your project follow Drupal coding standard by the attachment

Regards,
Steve Jan

sittard’s picture

sittard commented

Thanks for manual review.

In the block view I tried using drupal_add_js as suggested. But unfortunately the JavaScript is always placed in the header region rather than the block content. The results of this is that the e-commerce button appears in the wrong location on the page. I was unable to figure out how to set the scope to match the block region, perhaps someone could advise.

The issue with missing t() functions are resolved.

With regard to the automatic coding standards review. I have eliminated all the issues that I can. I still seem to be suffering with some whitespace issues. I have removed these in my editor so I can only assume that the automatic review is being a bit picky. Other than it's very informative and does actually help a lot.

sittard’s picture

sittard commented
Status: Needs work » Needs review

changing the status to needs review

aritra.ghosh’s picture

aritra.ghosh commented
Status: Needs review » Needs work

Hii
There are still some indentation issues. Please look into the result of the error report generated by automatic drupal code sniffer here: http://ventral.org/pareview/httpgitdrupalorgsandboxsittard1774774git

Here are some general points:

1. There must not be any whitespaces at the end od a line.
2. Array Indentation needs 4 spaces.

Also going through your code, regarding the inline js, you can have a look at the '#attached' property of the Drupal 7 FAPI. I think they may help you solve the problem of misplaced button with drupla_add_js().

Thanks.

sittard’s picture

sittard commented
Status: Needs work » Needs review

I've now fixed all the white space issues.

I'm still fumbling around trying to fix the inline JS issue. Hopefully someone can point me in the right direction with the following: 

  $script = 'if(window.shopActive){showViewBasketControl();}';
  $output = array(
    '#markup' => '<p>my content<p>',
    '#attached' => array(
      'js' => array($script, array('type' => 'inline')),
    ),
  );

It almost works but instead of inserting the JS inline it treats the $script as a file, which results in a page not found being logged. My other concern is that the Form API uses the loader function drupal_add_js(). When I used drupal_add_js it always placed the inline script in the header and not inside the block.

sittard’s picture

sittard commented
Issue summary: View changes

Updated GIT repository instruction.

sittard’s picture

sittard commented
Issue summary: View changes

Add first project review.

sittard’s picture

sittard commented
Issue tags: +PAreview: review bonus

Applying for review bonus.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -PAreview: review bonus

manual review:

  1. shopintegrator_basket() and shopintegrator_cart(): $id is user provided input and must be sanitized before printing into HTML. I was not able to exploit that because the variable is limited to 15 characters in the admin form. But you should sanitize it nevertheless to avoid XSS possibilities, see http://drupal.org/node/28984
  2. "variable_get('shopintegrator_id', '');": all variables used by your module must be removed in hook_uninstall().

Otherwise I think this is RTBC. Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

sittard’s picture

sittard commented

Fixed the possible XSS exploit and added the hook_uninstall() to remove variable as requested.

Many Thanks.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Reviewed & tested by the community » Fixed

no objections in a week, so ...

Thanks for your contribution, sittard!

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.

sittard’s picture

sittard commented

Thank you for all the reviews and the full project promotion!

Status: Fixed » Closed (fixed)

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

Anonymous’s picture

(not verified) commented
Issue summary: View changes

Added reviews of other projects