Closed (fixed)
Project:
Commerce Wishlist v3
Component:
Code
Priority:
Normal
Category:
Task
Assigned:
Reporter:
Created:
11 Jun 2015 at 18:56 UTC
Updated:
10 Dec 2015 at 15:24 UTC
Jump to comment: Most recent, Most recent file
Below the wishlist page we should also include an 'add all products to cart' button. I could not find a really neat implementation so I kept on looping through the submit function. Based on previous patch in this comment thread.
It works somewhat.
| Comment | File | Size | Author |
|---|---|---|---|
| #15 | add_all_to_cart_wsod_on_0.patch | 3.44 KB | chadsten |
| #6 | add_all_to_cart.patch | 5.93 KB | chadsten |
Comments
Comment #1
aramboyajyan commentedGood call, I agree.
Comment #2
chadsten commentedComment #3
neograph734Feel free to give this a spin. Attached is where I left off.
Comment #4
chadsten commented@neograph734 - Awesome, thanks for posting that. I'll begin work on this with your code as a baseline.
Comment #5
neograph734I believe it did work, but I disliked the double code therefor it has not yet been committed. It shouldn't be that much work to get it implemented properly.
Comment #6
chadsten commentedI have moved the duplicate code to a helper function, and done a little cleanup. I believe there is still more that needs done, like removing items from the wishlist once they are added to the cart.
Thoughts on other tasks related to this one?
Comment #7
neograph734Thanks for the help!
If I am not mistaken the items should be automatically removed from the wishlist as soon as the user checks out. (At least that was the intended behavior.) Until bought, the items are still wished for ;).
I'll give the patch a spin this weekend and will get back to you.
Comment #8
chadsten commented@neograph734 - Gotcha, I haven't dug around that much yet. I think that functionality is superior, didn't realize it was there.
Just keep me posted - I'm somewhat new to Drupal Commerce, so I'm more than open to suggestions/guidance if I have done something in a less than optimal way. Thanks!
Comment #9
chadsten commentedComment #10
neograph734Cool, seems to be working pretty nice. I ain't no hero in Drupal Commerce either, but I guess this is quite a neat way to handle this.
I am not sure what you meant with
@TODO - remove the item from the wishlist. Is that related to the WSOD I get when I set the desired amount of one item to 0, and the add to cart amount to a positive integer? So we should update amounts first and then add items to the cart?Comment #11
chadsten commentedNo, that was before I realized we clear the contents upon successful checkout. I'll remove that comment when I commit my changes to the repo.
Comment #12
neograph734Understood.
Do you think you could take a look at the WSOD descibed above?
Comment #13
chadsten commentedSure, I'll take a look today or tomorrow at this.
Comment #14
chadsten commentedSorry for the delay. I'm on this today.
Comment #15
chadsten commentedOkay, I've isolated the issue, and we have a few options for a solution.
Firstly, I'll attempt to describe the issue. Adding a single item to the cart uses the second quantity field (which makes sense), but adding ALL to cart uses the wishlist quantity, not the 'add to cart quantity'. This being the case, an item set to 0 in the wishlist quantity field, but a non-zero number in the add to cart will still attempt to add 0 to the cart.
To remedy this specifically, I've created a patch that simply checks if ($line_item['#value'] > 0), and it will skip attempting to add the product if the quantity is set to 0.
Now, is this desired behavior? Why do we look at one field for adding a single, but the other for adding multiple? Let me know your thoughts on this, and I'll adjust my solution accordingly.
Comment #16
chadsten commentedAnother option would be to have both add single and add multiple look at the 2nd (add to cart) quantity values, and only use the wishlist quantity for maintaining the wishlist, and setting the default amount in the add to cart quantity. This seems a little more intuitive in my mind, but obviously a single opinion isn't of much merit.
Comment #17
chadsten commentedComment #18
neograph734I agree that the last option (#16) sounds the most intuitive. That would be my preferred solution as well.
Comment #19
neograph734Oh, one small thing (it took me a while to realise this was an incremental patch). Please make sure that a submitted patch contains all code required to solve the issue as a whole.
This means the patch of #15 should also include the changes in #6. This way we can commit one patch instead of several incremental ones (and testbots on more advanced projects will fail to apply the patch otherwise). The inbetween changes can be uploaded as an interdiff.txt file.
Thanks a lot for your time and effort though!
Comment #20
chadsten commentedGreat, I'll move forward with #16, and I'll create a full patch of both. I'm still pretty green with the patch process, so thanks for the heads up on that!
Comment #21
aramboyajyan commentedUpdating the status.
Comment #22
neograph734@chadsten, how is the patch coming along? Could you give us an update?
Also could you remove the
// @TODO get rid of this dpm() ?line from the patch in #6? It is blocking #2571617: Remove form validation from commerce_wishlist_handler_field_add_to_cart_from_wishlist. I didn't want to commit the other issue because it would break this patch.Thanks in advance.
Comment #23
chadsten commented@neograph734 - I'm going to work on the patch today/tomorrow, and will update this issue when I'm ready for review.
Comment #24
neograph734@chadsten, did you have had a chance to look at this? Otherwise I'll start digging into it.
Comment #25
neograph734I'll continue with this.
Comment #27
neograph734Introduced a secondary submit handler so both buttons actually trigger different actions. This prevents the 'Add all to cart' button from triggering the update quantities actions.
Added a check to see if the add to cart section (quantity + button) is in the view, and do not display the button if it is not.