Reviewed & tested by the community
Project:
Commerce Wishlist
Version:
7.x-3.x-dev
Component:
Code
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Reporter:
Created:
27 Feb 2016 at 15:51 UTC
Updated:
20 Feb 2019 at 03:18 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
cslevy commentedComment #3
cslevy commentedImplemented commerce wishlist for anonymous users.
Comment #4
cslevy commentedComment #5
nvahalik commentedI know this patch is only 12 days old, but -dev saw a lot of changes in just the past couple of days. It'll need a re-roll.
In general, though, I tend to like what I see in the patch, but can't really do a review until the patch gets updated.
Comment #6
jmarr commentedI'm going to say this as calmly and politely as I can......It stuns me that someone would build a commerce wishlist module that does absolutely nothing for 99.0999999% of visitors to an e-commerce site. My suggestion (and I'm, only a full time e-commerce merchant so what do I know?) is that whoever is working on this module DROP what whatever you are doing and address the glaring problem!
Or failing that put a bold face disclaimer on your web page saying that anonymous users of sites using this will click a button that does nothing...and then call the merchant in fuming anger to say they will never shop on that site again since it is obviously run by incompetent clowns!
I understand that Drupal is a volunteer effort but please...if you don't know e-commerce enough to see a major failing in your module you're probably better off not contributing to it.
Comment #7
nvahalik commentedHi jmarr, will gladly consider any patches you have to contribute to this issue!
Comment #8
andyg5000Hey @jmarr. You sound like a nice guy. Thanks for all your contributions to Drupal!
Comment #9
jmarr commentednvahalik, I'm not a developer but I really wanted to get the message through that things like this - so glaringly obvious and so harmful to real world users - are sinking Drupal. Most of my peers have moved to other platforms simply because the number of incidents of things like this is really intolerable. Don't shoot the messenger.
Comment #10
andyg5000Dear @jmarr. We don't shoot messengers only ass holes
"I understand that Drupal is a volunteer effort but please...if you don't know e-commerce enough to see a major failing in your module you're probably better off not contributing to it."
Have fun with woo commerce.
Comment #11
cslevy commented@jmarr
I don't really understand what is your problem. This is an extra functionality. If you use it or not is your decision. But maybe someone need this functionality.
Comment #12
nvahalik commented@jmarr, if you'd like to fund the development of this module, I'd be happy to help point you in the right direction. These modules grow organically from the needs of our clients and this work rarely gets done for free.
So if you're interested in contributing: code or cash is cool, but comments about how you can't believe this doesn't work the way you want it to are not helpful.
Comment #13
jmarr commentedMy apologies to anyone who saw my input as anything but what it was. I guess I should have kept my criticism to myself.
Comment #14
kfitz commentedI'm not entirely sure why the patch fails to apply cleanly, but I've attempted to clean it up as much as possible. Applied patch against commerce_kickstart-7.x-2.36. @cslevy, hopefully you can go through this and make the necessary changes.
Comment #15
kfitz commentedAdding patch referenced in #14
Comment #16
thejacer87 commentedkfitz's patch in #14 gives me a white screen when applied. cslevy's patch in #3 doesn't apply properly and has quite a few errors. I've cleaned it up so my patch applies and fixes some errors from the original patch.
I can add things to my wish list (as a user and anonymous) but if you try to view your wishlist the site crashes:
Call to a member function set_display() on null in /home/.../modules/contrib/commerce/commerce.module on line 358>.The original patch tried to change the user wishlist view and things broke there. You cannot open the view from the admin menu.
Anyway here it is. Like I said, it should apply. I think the commerce_wishlist.views_default.inc file will need to get fixed up before any more work can be done.
Comment #17
thejacer87 commentedalso here are some errors I got after I tried adding an item to wishlist
Comment #18
cslevy commentedWhat I discovered when I exported my first patch, and try to apply on the original code, it was only applying partially. The modifications on the vies handler wasn't applied. Maybe if you apply manually the changes on views handler then will work. Sorry i didn't have time to investigate further.
Comment #19
thejacer87 commented@cslevy, can you explain what/where to make the changes in the views handler? I plan on working on this issue very soon.
Comment #20
cslevy commented@thejacer87
The changes that need to be done are in the patch file, but they doesn't apply for some reason. You can check the difference in the patch file.
Comment #21
thejacer87 commentedHoly smokes that was crazy. Here is a BRAND NEW patch. Pretty much went line by line with cslevy's patch. Then made adjustments for the view and other stuff. At the end of the day, the anon user can add stuff to a wishlist that is stored in the session. And when you login, those items get appended to a wishlish that you may have already had. The functionality is all there.
I believe that there are some access argument issues for viewing/updating the wishlist. If anyone wants to look at those or help me sort it out, let me know.
Also the wishlist table got messed up somehow (see attached png). If anyone can point me in the right direction for that, or fix it up, please do.
Please test and review!!
Comment #22
thejacer87 commentedComment #23
joshmillergit apply -v commerce_wishlist-anonymous-wishlist-2677086-21.patch
produces ...
So we need to base your patch off the latest dev release before additional testing can commence.
Comment #24
thejacer87 commentedHere it is. Applies cleanly to 7.x-3.x!!
1 thing to look at:
when you click "allow anons to have a wishlist" in the module settings form, you still have to grant the "manage own wishlist" permission to anon users (the patch notes this in the form element description). Should the module not automatically grant that permission when that checkbox in the settings form is selected? Is that even possible (i've never granted permission programmatically before)
Comment #25
thejacer87 commentedHere, use this patch. There is something funky going on with the "remove from wishlist" rules action that automatically removes teh item from the wishlist on events such as completing a purchase...
Comment #26
thejacer87 commentedComment #27
liamanderson commentedAfter applying the patch, during testing I came across the following issues:
Notes:
Tested with kickstart 2.x. 'Display "Add to Wishlist" for anonymous users' and 'Show "Remove from Wishlist"' both enabled.
Let me know if you have any questions or cannot reproduce.
Comment #28
joshmillerComment #29
thejacer87 commentedok, so here is the patch. this one takes car of the issue i mentioned in #24 where checking the allow anon wishlists will also change the permission.
i dont know why or how the array_flip() error is happening stack overflow says its probably from an older version of a xxx_load() function. there are a few
user_load()calls in the module, also somecommerce_product_load()calls. maybe those are the issues. i am having trouble tracing the call to anything in the wishlist module though.Comment #30
thejacer87 commentedComment #34
zdenkov commentedI think I found a cause for error described under #27 (wishlist page).
It seems that in the file "includes/views/handlers/commerce_wishlist_handler_field_add_to_cart_form.inc", you have to fetch product ID from the commerce line item before fetching a product.
So, in function
render($values), the line$product = commerce_product_load($wish_list_line_item->commerce_line_items[LANGUAGE_NONE][0]['line_item_id']);should be changed to:
Well, at least it works for me. I don't know how to prepare a patch so I leave it to you guys.
Comment #35
liamanderson commentedSuccessfully tested patch #29 on Kickstart 2.
Good job everyone!
The following was tested:
Note:
I don't think this is an issue I just wanted to make note of the behaviour when following the steps below:
Comment #36
liamanderson commentedComment #37
liamanderson commentedComment #38
lubwn commentedApplied patch successfully, thanks a lot for this! But I do not seem to be able to get it working. I checked wishlist configuration and checked anonymous access but clicking on "add to wishlist" link as anonymous user still redirects to login page. Everything is working super good when logged in (as admin) but not as anonymous user. Anything I missed? Maybe some configuration. Also checked permissions but still can not get it to work. Thanks a lot!
Comment #39
lubwn commentedI actually got it working. I was applying patch to release version instead of dev version. Now it works nicely. Thanks.
Comment #40
AusJohn commentedI'm getting array_flip errors as well. Trying to get to the bottom of it.
I applied patch 27 to core includes/common.inc from here to try backtrace this error some more.
So far I got this. 'false' is being passed as an ID, this causes the error.
I'll try dig deeper but, I think I'm in way over my head with this one.
Comment #41
AusJohn commentedImage of xdebug
Comment #42
AusJohn commentedI could fix the array_flip() issue.
The issue is this will only accept an int or string.
The wishlist id doesn't exist for Anon, as anon has no order in the system. If they have something in Cart, then it might work.
I fixed this by checking if $wishlist is FALSE, then change it to an INT value of 0.
Hope this helps anyone suffering from the same issues.