Needs work
Project:
Ubercart
Version:
8.x-4.x-dev
Component:
Code
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
24 Oct 2015 at 23:29 UTC
Updated:
10 Sep 2020 at 18:54 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
trobey commentedComment #3
longwaveComment #4
longwaveComment #5
jian he commentedYes the #2 patch fixed the sql issue. But there has other issues caused product could not be add:
Nov 26 20:58:34 jian-ThinkPad-T420 apache2: PHP Fatal error: Call to a member function id() on null in /home/jian/src/drupal8/modules/ubercart/uc_order/uc_order.order_pane.inc on line 646
Nov 26 20:58:34 jian-ThinkPad-T420 apache2: PHP Stack trace:
Nov 26 20:58:34 jian-ThinkPad-T420 apache2: PHP 1. {main}() /home/jian/src/drupal8/index.php:0
Nov 26 20:58:34 jian-ThinkPad-T420 apache2: PHP 2. Drupal\Core\DrupalKernel->handle() /home/jian/src/drupal8/index.php:20
Nov 26 20:58:34 jian-ThinkPad-T420 apache2: PHP 3. Stack\StackedHttpKernel->handle() /home/jian/src/drupal8/core/lib/Drupal/Core/DrupalKernel.php:637
Nov 26 20:58:34 jian-ThinkPad-T420 apache2: PHP 4. Drupal\Core\StackMiddleware\NegotiationMiddleware->handle() /home/jian/src/drupal8/vendor/stack/builder/src/Stack/StackedHttpKernel.php:23
Nov 26 20:58:34 jian-ThinkPad-T420 apache2: PHP 5. Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle() /home/jian/src/drupal8/core/lib/Drupal/Core/StackMiddleware/NegotiationMiddleware.php:55
Nov 26 20:58:34 jian-ThinkPad-T420 apache2: PHP 6. Drupal\page_cache\StackMiddleware\PageCache->handle() /home/jian/src/drupal8/core/lib/Drupal/Core/StackMiddleware/ReverseProxyMiddleware.php:51
Nov 26 20:58:34 jian-ThinkPad-T420 apache2: PHP 7. Drupal\page_cache\StackMiddleware\PageCache->pass() /home/jian/src/drupal8/core/modules/page_cache/src/StackMiddleware/PageCache.php:82
Nov 26 20:58:34 jian-ThinkPad-T420 apache2: PHP 8. Drupal\Core\StackMiddleware\KernelPreHandle->handle() /home/jian/src/drupal8/core/modules/page_cache/src/StackMiddleware/PageCache.php:103
Nov 26 20:58:34 jian-ThinkPad-T420 apache2: PHP 9. Drupal\Core\StackMiddleware\Session->handle() /home/jian/src/drupal8/core/lib/Drupal/Core/StackMiddleware/KernelPreHandle.php:53
Nov 26 20:58:34 jian-ThinkPad-T420 apache2: PHP 10. Symfony\Component\HttpKernel\HttpKernel->handle() /home/jian/src/drupal8/core/lib/Drupal/Core/StackMiddleware/Session.php:62
Nov 26 20:58:34 jian-ThinkPad-T420 apache2: PHP 11. Symfony\Component\HttpKernel\HttpKernel->handleRaw() /home/jian/src/drupal8/vendor/symfony/http-kernel/HttpKernel.php:62
Nov 26 20:58:34 jian-ThinkPad-T420 apache2: PHP 12. call_user_func_array:{/home/jian/src/drupal8/vendor/symfony/http-kernel/HttpKernel.php:139}() /home/jian/src/drupal8/vendor/symfony/http-kernel/HttpKernel.php:139
Nov 26 20:58:34 jian-ThinkPad-T420 apache2: PHP 13. Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() /home/jian/src/drupal8/vendor/symfony/http-kernel/HttpKernel.php:139
Nov 26 20:58:34 jian-ThinkPad-T420 apache2: PHP 14. Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext() /home/jian/src/drupal8/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php:102
Nov 26 20:58:34 jian-ThinkPad-T420 apache2: PHP 15. Drupal\Core\Render\Renderer->executeInRenderContext() /home/jian/src/drupal8/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php:129
Nov 26 20:58:34 jian-ThinkPad-T420 apache2: PHP 16. Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() /home/jian/src/drupal8/core/lib/Drupal/Core/Render/Renderer.php:577
Nov 26 20:58:34 jian-ThinkPad-T420 apache2: PHP 17. call_user_func_array:{/home/jian/src/drupal8/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php:128}() /home/jian/src/drupal8/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php:128
Nov 26 20:58:34 jian-ThinkPad-T420 apache2: PHP 18. Drupal\Core\Controller\FormController->getContentResult() /home/jian/src/drupal8/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php:128
Nov 26 20:58:34 jian-ThinkPad-T420 apache2: PHP 19. Drupal\Core\Form\FormBuilder->buildForm() /home/jian/src/drupal8/core/lib/Drupal/Core/Controller/FormController.php:79
Nov 26 20:58:34 jian-ThinkPad-T420 apache2: PHP 20. Drupal\Core\Form\FormBuilder->processForm() /home/jian/src/drupal8/core/lib/Drupal/Core/Form/FormBuilder.php:319
Nov 26 20:58:34 jian-ThinkPad-T420 apache2: PHP 21. Drupal\Core\Form\FormSubmitter->doSubmitForm() /home/jian/src/drupal8/core/lib/Drupal/Core/Form/FormBuilder.php:588
Nov 26 20:58:34 jian-ThinkPad-T420 apache2: PHP 22. Drupal\Core\Form\FormSubmitter->executeSubmitHandlers() /home/jian/src/drupal8/core/lib/Drupal/Core/Form/FormSubmitter.php:56
Nov 26 20:58:34 jian-ThinkPad-T420 apache2: PHP 23. call_user_func_array:{/home/jian/src/drupal8/core/lib/Drupal/Core/Form/FormSubmitter.php:116}() /home/jian/src/drupal8/core/lib/Drupal/Core/Form/FormSubmitter.php:116
Nov 26 20:58:34 jian-ThinkPad-T420 apache2: PHP 24. uc_order_edit_products_add() /home/jian/src/drupal8/core/lib/Drupal/Core/Form/FormSubmitter.php:116
Need to fix all the issues to make add product working.
Comment #6
jian he commentedThis patch fix all the throwed errors. But also could not add product yet.
Comment #7
jian he commentedIt seems this issue is caused by http://cgit.drupalcode.org/ubercart/commit/?id=dba4ff6
That commit cause function uc_order_product_save wrong.
Comment #8
longwaveI think we should avoid db_select() and use entity queries instead, if possible.
Comment #9
jian he commentedAt least the Add blank line working now :)
about the db_select, I do not found any example code for entityQuery + Join in Drupal core, so do not changed it.
Comment #11
trobey commentedThis patch passes all the tests locally on my computer. Also, it is very similar to my changes. Is it possible to rerun the tests?
I looked at entity queries and they do not have joins. Additionally, they just return the node IDs and then the entire node would need to be loaded which would be very inefficient. Entity queries just do not make any sense here.
Comment #12
tr commentedYes, you can click the "Add test" link in the comment next to the test result (this is preferred if all you want to do is retest), or you can post a comment and set the status back to "Needs review".
Comment #13
longwaveI made a couple of minor style changes. When I test the patch, I can add one product and one blank line, but then if I try to add another product I get some errors:
Notice: Undefined offset: 1 in uc_order_edit_products_remove() (line 683 of modules/ubercart/uc_order/uc_order.order_pane.inc).Warning: call_user_func_array() expects parameter 1 to be a valid callback, function 'uc_order_edit_form_submit' not found or invalid function name in Drupal\Core\Form\FormSubmitter->executeSubmitHandlers() (line 116 of core/lib/Drupal/Core/Form/FormSubmitter.php).Comment #14
tr commentedAll those forms like uc_order_edit_product_form() need to be converted to classes, IMO.
Comment #15
longwaveFixed adding and removing products so at least this works without errors, but the order edit form still needs a full overhaul.
Comment #17
longwaveFail seems unrelated as it was in CreditCardTest::testCheckout
Comment #18
tr commentedLooks OK to me. We can convert the forms in a separate issue.
We do have to figure out those random test fails though - we get the same few fails ever three or four runs. It's a lot harder to debug thing on DrupalCI than it was with the old testbot because we get so little information back. It'll probably take a weekend of bisecting to pinpoint the problem.
Comment #20
longwaveCommitted. Thanks to both @trobey and @jian he for contributing to this fix!
I opened #2573299: Convert order panes to plugins a while back where we should finish the conversion of the order edit form.
Comment #22
trobey commentedThis is broken again and has been broken for a while. I guess it was broken when the forms were ported. When the Add product button is pressed the updating spinner briefly appears but nothing changes. Looking at ajaxCallback() there is nothing to update the form. So I added a call to buildForm which does have the code for the add product form. I had to stash the order to use in the call to buildForm. ajaxCallback() also contains a call to uc_order_pane_admin_comments() which does not exist. The code does not even belong here since there is another file AdminComments which handles the comments. So I deleted those lines since the code errors out there.
There are almost no comments in this file which makes it hard to discern the intent so there may be a better approach to fixing this. But this patch does at least fix the immediate issue.
Comment #23
trobey commentedComment #24
trobey commentedI found another problem. In Products.php in addProductForm() there is the line
uc_form_alter($form, $form_state, __FUNCTION__);In uc_attribute_uc_form_alter() is the line
if (strpos($form_id, 'add_to_cart_form') || $form_id == 'uc_order_add_product_form') {Since __FUNCTION__ is addProductForm this fails. So I have updated the patch to fix this.
Comment #25
trobey commentedThere are still a lot more problems so I have written a much more extensive patch that gets the basic functionality of adding a product to an order working. It looks like this is supposed to use AJAX and specifically AjaxResponse. But I had to make a lot of changes since the code only has superficial changes for AJAX. AjaxResponse does not seem to add event handling to added form elements so I loaded all the form elements and then just hide or show them to allow the event handlers to work.
Comment #27
trobey commentedThe test failures does not seem to have anything to do with this patch but I updated the patch to fix the failures.
Comment #29
tr commentedWhat's the difference between #25 and #27?
The test failures were due to a core change - I've fixed those now so both your patches pass.
Comment #30
trobey commented@TR thanks. I could not figure out how the changes I made would have caused those failures. The only difference is #27 adds a check for parameters in the URL that have 'q'. The packages all have parameters that are integers. q is usually when clean URLs is off or something like that. This has nothing to do with orders so I would go with #25. I have spent my share of time with tests getting broken by changes to Drupal core for Drupal 8 so I am not surprised and was starting to work on tracking it down.
Comment #31
trobey commentedI did some more testing and fixed a couple of bugs and updated the patch.
Comment #32
david.czinege commentedI tested the patch in the comment #31.
I can add product successfully to a new and an existing order also.
I find a little bug. In the created OrderProduct entity the nid value is not set, so when i view the order, the product name is not linking to the product entity.
Comment #33
trobey commentedComment #34
trobey commentedI verified the problem. I went back and forth on whether it is related to this issue but decided to go ahead and fix it. The change is not big but it took about a week of debugging to figure it out. The updated patch just makes the following change:
Comment #35
longwaveThanks for working on this. However, I'm not really a fan of doing all this with CSS and building strings of HTML. This should all be achievable with the core Ajax framework, much the same way we did it in D7.
The attached patch makes the button work, though the list of products does not yet correctly refresh at the end of the process. I think this is the better approach, however.
Comment #36
trobey commentedI am also reticent to use CSS to display and hide elements. However, this is not achievable using the D8 AjaxResponse. There is some processing that gets done automatically for forms that does not happen for an OrderPane as I mentioned in comment #25. These do not matter much for submitting forms but they become a problem for AJAX. For example, looking at the attached screenshot notice at the top is the Add product button. There is a "ev" icon showing that an event handler is attached. Now look at the bottom at the Close button which is loaded through AJAX and there is no event handler attached. This is true even if the form element specifies an event handler. There all sorts of problems like this that did not occur in D7. Another example is that the $form_state object is not getting updated with changes to the part of the form loaded with AJAX. And another issue is that form caching is bypassed. Loading everything and then using CSS to hide and show elements is a work around for getting event handlers attached. One of the drawbacks is a website with a lot of products will load them when the page is first displayed even though the add product form is never used.
But it is not impossible to build infrastructure to replace that provided by FormBuilder. I have attached an updated patch which provides much of the functionality although there is still some work remaining. It also extends AJAX to other parts of the form such as the products table. While it is currently updating the products table the default values are not getting displayed (another small item that Form Builder apparently handles). Refreshing the table works for now. Note that I am using callbacks that do not require the form (OrderPane) to be completely rebuilt. This is because without form caching provided by FormBuilder it requires building the form which somewhat defeats the purpose of AJAX of just updating the changes. I will not go into all the infrastructure provided in this patch because it would be too complex and long. Also some of it may change as I dig into the code further and find better approaches.
Comment #38
trobey commentedMost, if not all, of these test failures have to do with payment so probably are unrelated. There is still some functionality to be added so it is a bit early to worry about tests anyway.
Comment #39
trobey commentedThis should be ready for more testing and review. The entire products section of the edit order form should now be working using AJAX.
Comment #40
chrisckI applied the patch in #39 and can confirm the Add Product button in Order Pane works.
Comment #41
chrisckAfter applying the patch in #39, the product select list appears, but the product cannot be added. Clicking 'Select' shows a "Please wait" with moving circle but quickly disappears. No product was added.
Comment #42
trobey commentedI just tested this and it still is working for me. Can you provide more information?
Comment #43
trobey commentedI noticed that after adding a product it was not possible to add another product without refreshing the page. This updated patch fixes that and removes a debug statement.
Comment #44
trobey commentedThe test failure appears to be unrelated to this issue.
Comment #45
david.czinege commentedHi,
I reviewed the whole Products OrderPane, and make it work with AJAX.
This patch only contains the bug fix, I will write a new test case inside the OrderTest, if somebody says that this patch works well.
I found some problems with the original code, and I have provided some solutions for them:
Problems and solutions
1. Lots of products in the select list
When I clicked on the Add product button, all products' title and nid was loaded and set as options into a select form element.
The problem occurs when there are a lots of products on a website. Our client has 33.000 products (yeah i know it is too much) and it was loaded in more than 1 and half minutes. But anyway, when the website has a lots of products, this select can't be used.
My solution for this problem:
When the user clicks on the Add product button, the select form element is not added.
It is only rendered after the user search for the title or the model of the product. There is an asterisk (*) handling so if the website has few products and the owner want to see all products in the select list the owner can write only a * character into the search field.
2. Order was saved before I clicked on the Save changes button
When a user adds or remove a products on the order edit form, the modifications was updated immediately. I think it is not a good approach.
I modified the functionality, so the order is only saved, when the user clicks on the Save changes button, until then the product modifications are only available in the $form_state object.
I hope you will like this patch and my solutions. Please review it.
Comment #46
kiwad commentedApplied the patch in #45 and seems to work fine
Comment #47
kiwad commentedabout patch #45
For multiple products with the same node title, there's no way to tell them apart.
Also, how do you choose options after adding a product that has attributes ? The price added is the price of the product but not the price of the option
Also, seems that searching by alternate SKU is not possible.
Comment #48
david.czinege commentedYes, you are right, this solution doesn't contain the product attribute integration.
First I have repaired the order edit form. This order edit page comes from the uc_order module.
The product attributes come from the uc_attribute module. So I think first we need to accept this patch without the attribute integration. After that we can develop the attribute integration.
Comment #49
manisha.singh04 commentedExcellent, I was facing the same issue and using the #45 that has been resolved.
Comment #50
ckhadj commentedWe are using Drupal 8.6.1 with Ubercart 8.x-4.0-alpha5. Our PHP version is 7.0.32 and database version is 5.5.5-10.2.17-MariaDB
We are trying to fix the problem of adding and editing products to orders created from administrator of our electronic marketplace.
We tried bug fixes 45 and 43 so far but no list of products is shown for selection and adding to the order.
When products in an order are entered through the cart they appear in the order but there is no possibility of editing/updating.
Are there any ideas how to fix the problem in the above configuration?
We were using Ubercart with Drupal 7 for many years without many problems.
Comment #51
ckhadj commentedDoes anyone managed to solve the problem of selecting and adding products in creating an order as administrator. We have tried patches 43 and 45 as mentioned in my comment 50 above but does not work. If anybody has worked on a new parch we would highly appreciate if he can share it or advise what we need to change in existing patches. Thank you in advance.
Comment #52
hash6 commentedThe above #45 is correct but I am facing an error
error: patch failed: uc_order/src/Plugin/Ubercart/OrderPane/Products.php:3
error: uc_order/src/Plugin/Ubercart/OrderPane/Products.php: patch does not apply
So I modified and created similar patch
While trying to create interdiff
I got the above error but could not fix it.
So could not add the interdiff.
Comment #54
hash6 commentedupdated the patch with few fixes.
Comment #55
hash6 commentedComment #56
tr commentedSee the test output:
Those need to be fixed.
It would also be helpful to have an interdiff for your patch, so we can see what you changed since #45.