Ajax Add to Cart is a Drupal Commerce module.

About the module:
AJAX Add to Cart module ajaxifies the add to cart operation. The updated cart will be displayed without page refresh and a popup message will be shown after you add an item to cart. This module provides two blocks, first the shopping cart block that provides details of the product currently present in the cart, and the other is a shopping cart teaser that provides the total number of items
and the total amount including tax. You can further customize the look of the blocks using its template files.

The sandbox project of Ajax Add to Cart module can be found here:
http://drupal.org/sandbox/subhojit777/1844396

Git repository link:

git clone --branch 7.x-1.x http://git.drupal.org/sandbox/subhojit777/1844396.git commerce_ajax_add_to_cart
cd commerce_ajax_add_to_cart

pareview.sh: http://pareview.sh/pareview/httpgitdrupalorgsandboxsubhojit7771844396git

This a Drupal 7 module for Drupal Commerce.

Other similar modules:
There is a module similar to this called Commerce Ajax Cart

My module differs from this module:
- AJAX Add to Cart module provides a separate block, like a minified version of cart that you can place in any region.
- Although, AJAX removal of product from cart is feature to be implemented in Commerce AJAX Cart, AJAX Add to Cart is able to remove products from cart through AJAX.

More features to be implemented:
- Select quantity of product that will be removed from cart

I have ported Ajax Pic Preview module from Drupal 6 to Drupal 7
http://drupal.org/sandbox/subhojit777/1529444
It is still in sandbox mode, soon it will be released as a full project.

Reviews of other projects:
- https://drupal.org/node/2204535#comment-8829485
- https://drupal.org/node/2194369#comment-8834025
- https://drupal.org/node/2199965#comment-8834097
- https://drupal.org/node/1970152#comment-8865287

Comments

klausi’s picture

Status: Needs review » Needs work

Welcome,

please get a review bonus first. Then try to fix issues raised by automated review tools: http://ventral.org/pareview/httpgitdrupalorgsandboxsubhojit7771844396git

smiletrl’s picture

Hi, great module! Here're some tips:

.info file
1. Maybe you need to add a dependencies[] of commerce module.

.module file
1. For hook_init(), I dont' think it's necessary to load system file. Drupal will load them for us. Also, js and css should not be added inside this hook, unless there're specifc conditions. Otherwise, they will be loaded for all pages(and they should be put in .info file). I think this is not what you want.

2.'page callback' => 'remove_commerce_line_item','delivery callback' => 'ajax_deliver', and there're many other functions, they should have a prefix of module name to avoid name collision.

3.

  $items['remove-product/ajax/%'] = array(
    'delivery callback' => 'ajax_deliver',
  ) + $items['remove-product/nojs/%'];

This part looks wierd, or not the conventional way of hook menu.

4. For hook_theme(), 'template' => 'templates/custom-shopping-cart', you should only put a file name, instead of including the path. You may want to use 'path' to override default path. But I think it's safe to use 'template' => custom-shopping-cart', since these templates are located in this module's directory.

.tpl file.
1. Function variable_get is used in tpl file. Maybe we could add these variables in prepcrocess function and make tpl more clean.

.install file
1. Is it really necessary to set so many variables? Like this variable, variable_set('remove_cart', 'link'); it's just a regular string 'link', and you set it as a variable in database. They're not changed anywhere, unless this module is uninstalled. I think it's safe to use these strings directly. Or, you could use php 'define' keyword in .module file to keep these configuration settings.
2.Also, these variables should have a module name prefix, otherwise it's quite easy to get name collision.

.admin.inc file
1. If a page callback only returns a drupal_get_form, I think we could use drupal_get_form as the page callback directly.

Anyway, a nice module!

idflood’s picture

Here is a little addition to the comment of @smiletrl in #2. In the .info you should probably add these dependencies:

dependencies[] = commerce
dependencies[] = commerce_cart

Also in the .info file you should remove the version, it's automatically added.
Here is an automated review with some more things: http://ventral.org/pareview/httpgitdrupalorgsandboxsubhojit7771844396git

By the way, nice project!

gazoakley’s picture

Issue tags: +PAReview: Commerce

Adding PAReview: Commerce tag

2pha’s picture

There seems to already be a commerce cart ajax module, though yours seems to add a little more functionality.
http://drupal.org/project/dc_cart_ajax

subhojit777’s picture

Thank you smiletrl for the code review and the suggestions. I will make sure that the module follows your suggestions. But there are something which cannot be followed, like the variable_set in .install file. This module has admin settings form and it requires some default configuration settings, due to which I have considered so many variables in .install file, although I will change the name of variable as you suggested. Thanks again :)

Thank you idflood, I will soon add those dependencies in .info file. Thanks :)

@2pha dc_cart_ajax module ajaxifies the removal of items from cart, but this module will ajaxify the addition of items to cart. If you see in this perspective this module this module is quite different from dc_cart_ajax.

Code review of this module as per ventral.org and DrupalCS has been done.

2pha’s picture

Ahh, yes, dc_cart_ajax is only for removal. I made my own module to ajaxify adding to the cart by adding a couple of functions to dc_cart_ajax (I probably should have submitted a patch).

What I really don't want to see is many different modules providing ajax functionality to commerce and the functionality of each overlapping. As I know I am going to be using them in the future. What also worries me is if they add quite specific functionality that can not be easily removed.

Can the popup thing after adding a product be disabled in your module?
Also, I see your module creates an alternative block to display cart items. I think doing it the way that dc_cart_ajax did it might be better. It uses a custom js command to update the default commerce cart block.

Just my 2 cents :)

subhojit777’s picture

I might have missed the patch while I was searching for a module that ajaxifies the addition of items to cart.

Yes sure, the popup thing can be disabled. Read the module's README and there are instructions to customize the appearance of the blocks.

By the way the dc_cart_ajax module changes the view that is available in /cart page of a commerce website. Removing any item form there will obviously affect the default commerce cart block, but the thing is there are many commerce websites where you will see a commerce cart block visible on all pages, and also they are ajaxified. See this website http://www.foodpanda.com, here you will see that addition of food items are ajaxified and the cart block appears on almost all pages(although there are few pages in that website), and like that there are many other websites implementing that functionality. Also you will see that items can be removed from the cart itself, I am planning to implement this functionality in this module's next release.

Anyways, here is my opinion that why I want to contribute this module. Any code review error or feature that can improve the functionality of this module are welcome :)

subhojit777’s picture

Status: Needs work » Needs review

Code review done as per ventral.org and DrupalCS.

Some suggestions of smiletrl have also been implemented.

facine’s picture

Status: Needs review » Needs work

Hi subhojit777!

The PAReview tool identified a few minor issues:

http://ventral.org/pareview/httpgitdrupalorgsandboxsubhojit7771844396git

Manual review:

  • All Drupal Commerce modules are prefixed by Commerce*, please rename your módule.
  • Have a lot of variables, and I think many of them could be grouped into a single array, saving variable requests.
    You could also use default constants, while administrators do not set your settings.
  • I did check for duplicate modules. About the nearest I can find is Commerce Cart Ajax. But it seems that both do exactly the same. The other only focuses on the process of updating the cart, not the process of adding.

Aldo add in info file:
package = Commerce (contrib)

The rest of the code seems fine.

Regards!

subhojit777’s picture

Status: Needs work » Needs review

Code review done as per DrupalCS and ventral.org

Module name changed and code fixes done

darrenwh’s picture

Hi,

in your function dc_ajax_add_cart_get_shopping_cart_teaser in your .module file I would look to initialise the variables at the start and eliminate the else loop:

  $commerce_cart = dc_ajax_add_cart_get_commerce_cart_details();
  $line_items = NULL;
  $quantity = 0;
  $total = NULL; 

if(...

also since you don't use the var $content again in the function just return the theme

return theme('ajax_shopping_cart...

with the function dc_ajax_add_cart_get_commerce_cart_details

initialise you array at the beginning like so :

$output = array("order"=>NULL,'wrapper'=>NULL);

eliminate the else

look to populate and check the var $order in the if clause:

if ($order = commerce_cart_order_load($user->uid)) {...

The var $wrapper is only used once, so just assign the array to the function:

$output['wrapper'] = entity_metadata_wrapper('commerce_order', $order);

Sorry if I'm being too critical , just working on my review bonus :)

bekirdag’s picture

This may not be the most important issue at the moment but i want to report a problem.

I installed these modules:

http://drupal.org/project/commerce_product_attributes
http://drupal.org/project/commerce_file
http://drupal.org/project/commerce_product_bundle

and this one in the sandbox.

http://drupal.org/sandbox/tbenice/1690422

Then i installed your module (great module, congratulations!) . It worked fine when i click on add to cart button.

Then i uninstalled all the modules (including yours) mentioned above. Now i am getting this error when i click to Add to cart buton:

---------------
Error

Error messageEntityMetadataWrapperException: Unknown data property commerce_webform_sid. in EntityStructureWrapper->getPropertyInfo() (line 339 of /path/to/my/site/sites/all/modules/entity/includes/entity.wrapper.inc).
The website encountered an unexpected error. Please try again later.
---------------

I reenabled your module but now i am getting this error when i click Add to cart button (as a js alert):

--------------------------
An AJAX HTTP error occurred.
HTTP Result Code: 500
Debugging information follows.
Path: /mysite/cano/en/system/ajax
StatusText: Service unavailable (with message)
ResponseText: EntityMetadataWrapperException: Unknown data property commerce_webform_sid. in EntityStructureWrapper->getPropertyInfo() (line 339 of /path/to/my/site/sites/all/modules/entity/includes/entity.wrapper.inc).
------------------------------

I am not sure if it has anything to do with the above modules though, but i assume you already did enabled and disabled a few times and didn't notice such an error.

2pha’s picture

Judging by the error, your problem seems to be with the commerce_webform module.

muhleder’s picture

Hi Subhojit,

This looks like a great module, I love it. Installed and tested and it works straight away, even on our pretty complex commerce setup.

I've spotted a few minor issues that you should fix, I'd be happy to mark this as RTBC once 1 and 2 are fixed (then for klausi to have the final say). 3 and 4 would be nice to see fixed but not critical imo.

1. gitignore
You need to remove the .gitignore file, you can set ignores for yourself in ~/.gitignore

2. Define namespaces
Defines need to be prefixed with the module name to avoid namespace collisions with other modules. eg
define('DISPLAY_POPUP', 'dc_ajax_add_cart_display_popup');
should be
define('DC_AJAX_ADD_CART_DISPLAY_POPUP', 'dc_ajax_add_cart_display_popup');

3. Variable overwritten
In dc_ajax_add_cart_remove_commerce_line_item()

  // Initially quantity of products in cart will be zero.
  $quantity = 0;

The $quantity variable is overwritten before it is used. This line can be removed.

4. Code clarity
$is_ajax = $ajax === 'ajax';
would be a little clearer as
$is_ajax = ($ajax === 'ajax');
I was reading this as
$is_ajax = $ajax = 'ajax';
at first.

muhleder’s picture

Status: Needs review » Needs work

Setting to needs work.

subhojit777’s picture

@muhleder thanks for your feedback. I will try to fix the problems ASAP. Apart from the issues that you all have mentioned, I have noticed another issue, that is when you add a product to cart, the popup does not shows the right product details currently added to cart.

I am trying to fix this problem. I was hoping that there would be some kind of function that returns the product currently added to cart.

Any thoughts? Please share :)

PA robot’s picture

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.

reubenlara’s picture

My install seems to be working inconsistently. The only two differences about my product are:

1) I have multiple products I need to show on one page, so there are multiple Add To Cart buttons showing at once
2) I'm using a pricing rule on one of the fields to enter a custom price (which breaks when your module is enabled, works when not enabled)
3) The quantity button is added through a custom View and not through the standard install view (the quantity button breaks as well and doesn't add multiple quantities when your module is enabled)

This 4 minute video shows what's not working (too confusing to explain in text)

visionart-cart-error.mov

Any ideas? Thx!

subhojit777’s picture

@reubenlara thank you very much for using this module and finding bugs. There are bugs present in the module but I was not expecting bugs as you described.

Could you please explain the configuration of pricing rule and custom views, I want to replicate the bug.

subhojit777’s picture

Assigned: Unassigned » subhojit777
Issue summary: View changes
Status: Closed (won't fix) » Active
subhojit777’s picture

Issue summary: View changes
subhojit777’s picture

Issue summary: View changes
subhojit777’s picture

There is a module similar to this called Commerce Ajax Cart

My module differs from this module:
- AJAX Add to Cart module provides a separate block, like a minified version of cart that you can place in any region.
- Although, AJAX removal of product from cart is feature to be implemented in Commerce AJAX Cart, AJAX Add to Cart is able to remove products from cart through AJAX.

subhojit777’s picture

Issue summary: View changes
subhojit777’s picture

Issue summary: View changes
subhojit777’s picture

Issue summary: View changes
subhojit777’s picture

Issue summary: View changes
subhojit777’s picture

Issue summary: View changes
subhojit777’s picture

Issue summary: View changes
subhojit777’s picture

Status: Active » Needs review
subhojit777’s picture

Issue summary: View changes
Issue tags: +PAreview: review bonus
mpdonadio’s picture

Issue summary: View changes

Made git link the public one.

mpdonadio’s picture

Issue summary: View changes

Added pareview link.

mpdonadio’s picture

Status: Needs review » Needs work
Issue tags: +PAreview: security

pareview came up with nothing, but I think something is wrong with it. There are some unused variables
and bad spacing/indentation that it would normally catch.

Your module name and the namespace don't match right now. Just make sure they match when this
gets approved and you make it public.

You JS isn't using a proper behavior. See https://drupal.org/node/756722

print is preferred over echo.

(*) Your PHP in the .tpl.php files should be minimal. t() is fine for really simple things (like the Close string),
but the check_plain()s and l() shouldn't be there. The preprocess functions are the best place in this case.

Remove the .gitignore from the repo.

You don't really need the JS and CSS in the .info file as these aren't needed on every page on the site. Using #attached
in your blocks is probably a better idea.

hook_init ins't a good place to load the library. Your block view is probably better.

(*) 'access callback' => TRUE, isn't a good idea. Yes, a lot of AJAX modules do this, but I
think 'access arguments' => array('access content'),
would be better, and this looks like it aligns better with the commerce_cart module for the cart access paths.

Reusing the $items in hook_menu is a little odd. Don't think I have ever seen this before.

In dc_ajax_add_cart_form_alter(), using strpos() === 0 is probably better.

Indentation on multiline arrays isn't the best.

Do you need DRUPAL_NO_CACHE in dc_ajax_add_cart_block_info() for the blocks? It defaults to per-role, which doesn't
seem what you want.

Render arrays for the content, rather than themed output, is better for the block view functions.

(*) As far as I can tell, empty_cart_teaser_message and success_message go from form to HTML w/o sanitization.
Double check all of the admin text settings.

Your template names should really be namespaced w/in your module. This helps if someone copies them
into their theme.

I only did one visual pass on this; if there weren't security blockers, I would do another pass.

The starred items (*) are blockers. Please don't remove the security tag, we keep that for statistics and to
show examples of security problems.

mpdonadio’s picture

Also, did you contact the maintainers of https://drupal.org/project/commerce_ajax_cart to see if you could provide a patch for the additional features?

subhojit777’s picture

Status: Needs work » Needs review

Thanks for module review mpdonadio. Resolved almost all security blockers, except this one:

Reusing the $items in hook_menu is a little odd. Don't think I have ever seen this before.

I got the idea from http://www.computerminds.co.uk/drupal-code/make-link-use-ajax-drupal-7-i... this blog. I guess the implentation follows Drupal standards. Are there any other implementation idea from your side? Basically, I am making an AJAX-link using hook_menu().

Also, I have contacted the maintainers of https://drupal.org/project/commerce_ajax_cart, but unfortunately there is no reply (https://drupal.org/node/2279825)

If there are no security blockers, would you please do the next pass you were talking about :)

subhojit777’s picture

Issue summary: View changes
klausi’s picture

Status: Needs review » Postponed (maintainer needs more info)

As already mentioned this looks like it should go directly into commerce_ajax_cart. The next step would be to supply a patch against that module. If you cannot reach the maintainer(s) please follow the abandoned project process.

If that fails for whatever reason please get back to us and set this back to "needs review".

Anthony Goode’s picture

Hi subhojit77,

Anyway I like the flexibility of the module. In my opinion it is a pitty if the module is abandoned and all efforts in the module to get those functionalities are wasted.

PA robot’s picture

Status: Postponed (maintainer needs more info) » Closed (duplicate)
Multiple Applications
It appears that there have been multiple project applications opened under your username:

Project 1: https://www.drupal.org/node/2295233

Project 2: https://www.drupal.org/node/1844398

As successful completion of the project application process results in the applicant being granted the 'Create Full Projects' permission, there is no need to take multiple applications through the process. Once the first application has been successfully approved, then the applicant can promote other projects without review. Because of this, posting multiple applications is not necessary, and results in additional workload for reviewers ... which in turn results in longer wait times for everyone in the queue. With this in mind, your secondary applications have been marked as 'closed(duplicate)', with only one application left open (chosen at random).

If you prefer that we proceed through this review process with a different application than the one which was left open, then feel free to close the 'open' application as a duplicate, and re-open one of the project applications which had been closed.

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

apaderno’s picture

Assigned: subhojit777 » Unassigned
Issue tags: -
Related issues: +#2295233: [D7] Views base url