Why does the module use JS to check the stock level?

I can't seem to find any benefit of creating a couple of dozens of hits to the server when browsing the catalog. Or, if you visit http://example.com/products page, you get a hundred hits (in my case). The page will not be fully loaded until all these POST-queries are finished. So one page that takes 30-60 seconds to fully load can ruin an average page loading time for your site, which is not the best thing for the SEO.

Also this is not very user friendly in case a button, the user was just about to press, suddenly gets replaced with a message. This happened to me a couple of times - very annoying. Finally, this many hits really ruin site statistics if you are using Awstats or something similar. Oh, and what about user agents that have Javascript disabled?..

What I am trying to say is that instead of modifying the page with JS, you should better replace "Add to cart" button with a hook_form_alter.

There are two forms that display this button, and, in fact, the module does already alter them, when adding the javascript file.

If there is no other big reason to use JS, I really think you should get rid of it, and I'll be more than happy to help you with a patch.

CommentFileSizeAuthor
#16 uc_out_of_stock-option_to_disable_js-657924-16.patch1.74 KBpieterdc
#3 out_of_stock-657924-comment-3.patch5.63 KB20th

Comments

hanoii’s picture

hanoii
Primary language Spanish
Location 🇦🇷UTC-3
commented
Title: Use hook_form_alter insted of JS » Give more options to control the use of JS of the module
Category: bug » task

The idea of using JS is more of a real time check. This module works with attributes in a way that when you select an attribute on a product the query is sent to the server to get if the product with that attribute is on stock or not. W/o attributes I agree it might not be that useful the JS side. It also performs server side validation, so even browser w/o JS will still work.

As for page loading for SEO, I wouldn't worry, as SE doesn't really process JS, the the page load for them will be just the HTML.

But the page loading for the user is a big thing and I agree with you and it was reported already the hit of the server. While getting rid of the JS is not the approach I would take, as I explained above and it's part of the idea of this module, I think there should be a bit more options for the user to control the use of JS, whether disable completely, or just display it in the full page of the node, etc.

Altering the form might be an option as well for products that are already known as out of stock.

An idea I had in my mind for a while is to divide the module in two different modules, one for the plain server side validations and one for the JS, although a single one with proper configuration options might just work as well.

hanoii’s picture

hanoii
Primary language Spanish
Location 🇦🇷UTC-3
commented

Related to #514170: Make the module less process consuming

20th’s picture

20th commented
Status: Active » Needs review
StatusFileSize
new out_of_stock-657924-comment-3.patch5.63 KB

Forgot about attributes...

I don't really like the idea of splitting the module. It is not big and splitting it may result in some code duplication. Though, it is completely up to you.

Here I made some changes and would like to hear what you think. Now JS-queries will be performed only on a product's full page, if it has any attributes. Javascript is not added to the catalog pages at all, and it is not added to products that have no attributes.

And I also wrapped the "Out of stock" message into a theme-function. This will help site admins to display more complex messages in their themes.

This patch is for Drupal 6 branch. Currently testing on a production server.

babbage’s picture

babbage commented

This sounds like a good patch. Is this module under active development?

hanoii’s picture

hanoii
Primary language Spanish
Location 🇦🇷UTC-3
commented

@dbabbage: It somehow is. I am aware of this patch and will certainly take a peek at it, however, I am also planning a major rework of this module in which this feature is not gonna be needed, or if it is, with major changes, so that's why I haven't committed it yet. I was also waiting to hear other opinions.

Bruno-2M’s picture

Bruno-2M commented

Seems also to be a good patch as the main problem is performance on catalog pages.

mitrpaka’s picture

mitrpaka commented

Looks a good patch. Did notify one issue with it though... While adding product to the cart or having the checkout, exceed of stock level is not notified.

@hanoii: I also agree with others that this type of feature (as an alternative) would be good to have as performance does matter. Any plans when rework of this module is going to happen as indicated in #5?

mo6’s picture

mo6
he/him
Primary language Dutch
commented

I'm was looking for the exact functionality of the patch in #3 as the use of the JS code totally freaks me out for a number of reasons: UX and performance wise. So please include a non-JS solution.

mo6’s picture

mo6
he/him
Primary language Dutch
commented
Status: Needs review » Reviewed & tested by the community

Patch applies cleanly against 6.x-1.4. Tested it with D6.17 / UC2.2, without attributes. Works fine.

@mitrpaka: I couldn't replicate your issue, I'm notified when adding to the cart exceeding the stock level. Did you enable stock management for the specific product?

arski’s picture

arski commented

+1 on this

arski’s picture

arski commented

although I would prefer a setting somewhere that lets me choose if I want JS validation at all - that would be the correct way to go imo.

hedac’s picture

hedac commented

I don't know why... but the JS check doesn't work for me now... but I'm ok with that since I don't like it at all.
So I am working in a solution using views... and not displaying the button if Stock is 0 and active. without using attributes stock.

gone404’s picture

gone404 commented

I just patched uc_out_of_stock.module and now every single product kit says "Out of Stock".

It also still makes multiple out of stock ajax requests unless I disable "Use Javascript" in Out of Stock settings.

How anyone could think that making an ajax call for every product on every page for every request is a remotely reasonable idea is beyond me. I'm glad you are at least trying to do something about it v_20q.

arski’s picture

arski commented
Status: Reviewed & tested by the community » Needs work

this obviously suggests that we're not close to a commit yet.

brunorios1’s picture

brunorios1 commented

subscribing

pieterdc’s picture

pieterdc
Primary language Dutch
Location Ghent
commented
Title: Give more options to control the use of JS of the module » Option to disable the use of Javascript
Component: Code » User interface
Category: task » feature
Status: Needs work » Needs review
StatusFileSize
new uc_out_of_stock-option_to_disable_js-657924-16.patch1.74 KB

My patch concentrates on the core question that has been asked here.
It doesn't add the other smart stuff, such as a theming function, provided by the patch from #3.

Applying this patch, and disabling Javascript, I can combine this module with the Ubercart Stock Notify module.

Happy testing!

hanoii’s picture

hanoii
Primary language Spanish
Location 🇦🇷UTC-3
commented
Status: Needs review » Fixed

With #514170: Make the module less process consuming fixed, one query to the server (regardless of catalog view or product view) shouldn't stress any server. I added, however, the option to disable JS but in a slightly different way.

Rather than a checkbox to enable I added a checkbox to disable as well as put it in an advanced setting section. This should hardly ever be needed but it won't harm to have it there.

Status: Fixed » Closed (fixed)

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