Product kit in admin order creates duplicates

The kit should be exploded into its component parts, but this is currently buggy. I expect the fix for this to be the same as #1246670: Adding product kit to admin created orders several issues, it just needs someone to spend some time on it.

Marked #1920702: Product Kits are Multiplied in Cart with Admin Order as duplicate

Hi,

I was facing the same issue and made few changes which fixed my issue. I am attaching a patch for these changes.

Hth,
Sadashiv.

4: product_kit_in_admin_order_creates_duplicates_1697898_4.patch queued for re-testing.

4: product_kit_in_admin_order_creates_duplicates_1697898_4.patch queued for re-testing.

Hi,

Would be great if someone can test, verify that patch works and move it ahead.

Thanks,
Sadashiv.

With patch applied:

Notice: Undefined property: stdClass::$skip_save in uc_order_edit_products_add() (line 565 of /var/www/vhosts/site.com/htdocs/sites/all/modules/ubercart/uc_order/uc_order.order_pane.inc).

The patch is fixing the duplication issue though

Works for me.

Note that I'm NOT seeing the PHP Notice reported by 1959mvp in #11.

This patch is not accepted yet?

Changing priority. I think it's tested and works and should go in the code so that this bug is fixed.

Thanks,
Sadashiv.

@Morbus Iff: Did you look in /admin/reports/dblog for the Notice ?

@rakun: The patch was only tested 2 days before your post. Without community testing there's no chance the patch will be put in. You could help out by doing your own tests and posting the results here. When a patch is put in, there will be an issue comment showing that action and the issue status will be changed to fixed.

@sadashiv: Please read the guidelines for priority - there's a link right next to where you set the priority. Also, did you read @longwave's comments in #1? I don't see where you've read, tested, and reviewed that other issue to see whether it solves this problem too. I don't see where you've tested this issue to see if it solves or conflicts with that other one.

There are three different ways that product kits can be handled in the cart, controlled by the settings at admin/store/settings/products under the product kit tab. I don't see where *anyone* has looked at how this patch affects kits in all three cases.

Basically, I look at this issue and the linked issue in #1 and I see at least a half day of work to set up a clean site, try to reproduce the issue, apply the patches, test the patches with different combinations of settings, test them with regular products and product classes, review the patch code to try to understand it (especially if you've not added comments explaining why you've done something) and clean up if it's necessary, then if everything works correctly commit the patches and push them. When I see minimal yet conflicting reviews like in #11 and #12, and when I see a number of things unaddressed like I mention above, then it's almost like I'm starting from scratch on this issue. I don't personally need this patch, and I have many other things I need/want to work on. The more you do on your end to address all the concerns the more likely it is that this patch will get committed. If you leave it to the maintainers to do the bulk of the work ourselves then don't be surprised when we work on the things that are important to us first. After all, we are volunteers doing this for free. Alternatively, you could start helping us out by working on other issues in the queue and then maybe we'd have more time to devote towards other less important issues.

Switching to needs review to force the new testbot to retest.

Since the patch was uploaded before the new testbot was activated, I think the patch needs to be uploaded again to get the new bot to look at it. This patch is the same as #4.

As the patch is passed, I think it should get committed.

Thanks,
Sadashiv.

Am I just talking to myself here?

Reporting back on #15.

I've re-rolled the patch #4 (#16) to address a couple of small things.

1. The #4 patch tests a property: stdClass::$skip_save without verifying its existence. As stated in #11, a PHP warning was being generated, which I could duplicate by adding non-kit products to a manual order.
2. The #4 patch changes the signature of uc_product_kit_uc_order_product_alter() to make $order a reference, which is unnecessary.

I tested the re-rolled patch with the three product kit settings at admin/store/settings/products, and it seems to work correctly. With any of the three settings, Ubercart put the correct number of items in the cart, though the product kit was exploded into separate line items even for UC_PRODUCT_KIT_UNMUTABLE_NO_LIST, which perhaps is to be expected, I'm not sure.

Regarding #1246670: Adding product kit to admin created orders several issues, linked in #1, during an admittedly quick review, I had a hard time understanding the exact problem being reported. I read the patches and there is a hard-coded reference to product kits in uc_order.order_pane.inc, which doesn't smell very good to me.