Fix infinite loop when printing invoices for orders not in completed #8197

sinansonmez · 2021-09-16T21:32:27Z

What? Why?

ISSUE: If an order is not in completed state, the user can select the order with checkbox and click Print Invoices button. In this case, user see an endless-spinner.

SOLUTION:

Single Selection Case: If the order is not in completed state, the checkbox is hidden.
Select All Case: In toggleAll function, the order is not included in selected_orders so when user click select all checkbox, the order is not selected.

What should we test?

Release notes

Fixes the bug when user tries to print not completed orders

Changelog Category: User facing changes

Dependencies

Documentation updates

andrewpbrett

Thanks for this contribution @sinansonmez - I'm not sure that this is the correct solution in the long term. If there are other actions that could be applied to orders, or we want to add other actions, we would still need to be able to select orders that are not in the complete state to do those actions. There's also no indication to the user as to why the order can't be selected.

I saw @filipefurtad0's comment suggesting this as the solution; Filipe, what do you think?

sinansonmez · 2021-09-25T10:11:39Z

Thanks for the feedback @andrewpbrett. This also came to my mind but since currently the only action user can do with selection of orders is printing invoices, I thought my solution would be fine.

Alternative (maybe ideal) solution would be hiding the Print Invoices button if one of the orders selected not in complete state. Lets see what @filipefurtad0 thinks and I can update it accordingly

filipefurtad0 · 2021-09-27T08:40:24Z

Agree @andrewpbrett , it might not be clear for users why these cannot be printed.

Maybe the Print Invoices should only be active when the only show completed orders checkbox is selected?

Also, my previous comment should also mention other order states, as "completed" orders include these as well:

complete
resumed
cancelled
shipped
awaiting return

Would this sound like a good solution? What do you think @openfoodfoundation/train-drivers-product-owners ?

RachL · 2021-09-27T10:24:33Z

Maybe the Print Invoices should only be active when the only show completed orders checkbox is selected?

It might be hard to understand for the user, but I guess it's a good compromise if we cannot generate the pdf skipping uncompleted orders.

jaycmb · 2021-09-27T10:56:10Z

if we cannot generate the pdf skipping uncompleted orders.

I was going to ask the same.
if it´s not just about order in state complete but also the states @filipefurtad0 mentioned, going for the

Print Invoices should only be active when the only show completed orders checkbox is selected

would only partially solve the problem.

So best case would be automatically enable bulk printing via action button only for order state are

complete
resumed
cancelled
shipped
awaiting return

, i,e. automatically skipping the uncompleted ones when generating the PDF

But if this requires more effort than the "minimum required" solution

Print Invoices is only enabled when show only complete orders is checked
AND if user checks the show only complete orders, this should show not only order state=complete but also the other states for finalized orders

Ideally we d need a tooltip then on the Action button, indicating to the user "Choose 'show only complete orders' to enable bulk printing invoices"

sinansonmez · 2021-09-27T13:02:52Z

Thanks for the feedbacks. I will try to implement best case scenario mentioned by @jaycmb.
Therefore if user selected orders with any of the states below, Print Invoices button will be active. Otherwise, it will be disabled (I will also try to implement tooltip for proper user experience)

complete
resumed
cancelled
shipped
awaiting return

jaycmb · 2021-09-27T14:27:08Z

@sinansonmez sorry, I might have been not precise with the best case scenario.
By

automatically enable bulk printing via action button only for completed orders

I meant automatically skipping the uncompleted ones when generating the PDF, so there is no need for the user to take any action and also no tooltip needed for explaining why they need to select that option.

If this requires more effort though, then the option with

enable bulk printing only when the show complete orders only option is checked
tooltip to explain this change of feature to the user

would be a solution as well

…ecked

sinansonmez · 2021-09-27T23:04:37Z

I just updated the pull request according to discussion above. For tooltip info, I didn't know how to manage translation. For this reason I only inserted the text to English. Please guide me how to manage other languages.

Screen.Recording.2021-09-28.at.01.09.47.mov

mkllnk

@sinansonmez I'm sorry for all the confusion here. This issue hasn't been discussed that well to start with and the discussion is happening here now. I will give my opinion here but don't implement it yet until someone else agrees and nobody disagrees. 😄

I like your original contribution because it enables you to select completed orders and print them even when there are incomplete orders visible. The issue is that it's not clear why people can't select incomplete orders.

The second solution has a great tooltip to explain things but you have to do extra steps to print when you are showing incomplete orders.

What do you think about a mixture of the two? We could have checkboxes for all orders so that other actions can be added later on. But we enable the print button only if all selected orders are complete. The tooltip can explain that. That could be easier than fixing my mentioned issue below. What do you think @jaycmb?

mkllnk · 2021-09-30T02:27:12Z

config/locales/en.yml

@@ -2708,6 +2708,7 @@ See the %{link} to find out more about %{sitename}'s features and to start using
        Perhaps it has become unavailable or the shop is closing.
    admin:
      unit_price_tooltip: "The unit price increases transparency by allowing your customers to easily compare prices between different products and packaging sizes. Note, that the final unit price displayed in the shopfront might differ as it is includes taxes & fees."
+      print_invoice_tooltip: "Choose 'ONLY SHOW COMPLETE ORDERS' to enable bulk printing of invoices"


This is perfect for translations. ✔️

mkllnk · 2021-09-30T02:34:46Z

app/views/spree/admin/orders/index.html.haml

@@ -23,8 +23,14 @@
  = render partial: 'per_page_controls'

  - if Spree::Config[:enable_invoices?]
-    %button.invoices-modal{'ng-controller' => 'bulkInvoiceCtrl', 'ng-click' => 'createBulkInvoice()', 'ng-disabled' => 'selected_orders.length == 0'}
+    %button.invoices-modal{'ng-controller' => 'bulkInvoiceCtrl', 'ng-click' => 'createBulkInvoice()', 'ng-disabled' => 'selected_orders.length == 0 || !q.completed_at_not_null'}


This changes live on the page when you toggle the "Only show complete orders" checkbox. So it's possible to just tick that box but not reload the page which means that you can still select an incomplete order and run into the same issue of the infinite spinner.

sinansonmez · 2021-09-30T10:58:54Z

Thanks for the feedback @mkllnk. I will wait others to comment to implement sth

jaycmb · 2021-09-30T12:24:42Z

@mkllnk not sure if I understood what you mean by

We could have checkboxes for all orders so that other actions can be added later on.

because this we have already implemented, right?

Or you mean the change is about

we enable the print button only if all selected orders are complete.

Because currently the Print invoices button is also enabled when incomplete orders are selected?

More Product / Compliance question:
Is this something that should be kept if a user specifically wants to print an invoice for a particular uncompleted order or printing invoices should never be allowed for uncompleted orders?

mkllnk · 2021-10-01T07:01:57Z

@jaycmb

We could have checkboxes for all orders so that other actions can be added later on.

because this we have already implemented, right?

Yes, I meant that we keep the checkboxes as they are.

Or you mean the change is about

we enable the print button only if all selected orders are complete.

Because currently the Print invoices button is also enabled when incomplete orders are selected?

Yes, my suggestion was to only change two things:

The print button disables when you select an incomplete order.
Add a tooltip to explain this behaviour.

More Product / Compliance question:
Is this something that should be kept if a user specifically wants to print an invoice for a particular uncompleted order or printing invoices should never be allowed for uncompleted orders?

I can't answer that question but if we implement the ability to print incomplete orders then it's very easy to enable the print button for those again. The invoice should probably have a big label that the order incomplete so that people realise it when looking at the PDF or print-outs.

sigmundpetersen · 2021-10-01T07:18:25Z

if a user specifically wants to print an invoice for a particular uncompleted order

Maybe this functionality could be kept only on the Order Details page?
So one would have to enter incomplete orders page one by one and hit 'Actions->Print Invoice'.

I don't know if that is feasible tech-wise though.

sinansonmez · 2021-10-05T18:07:00Z

Thanks @sigmundpetersen and others, for the comments.

For Orders page: Should I update my PR as described by @mkllnk below?

@jaycmb
Yes, my suggestion was to only change two things:

The print button disables when you select an incomplete order.

Add a tooltip to explain this behaviour.

For Order Details page: When I try to print an order in payment state, I don't see any action in Action button (please see the screen recording below). I am not sure if this is a normal behavior. Anyhow, if we want to implement this, I believe this change should be an another issue and PR.

Screen.Recording.2021-10-05.at.18.29.22.mov

Please let me know about final decision on Orders page so I can update and close this PR accordingly. After that I can work on the agreed solution for Order Details page.

RachL · 2021-11-16T16:36:42Z

Sorry to get back to you so late @sinansonmez

After some thoughts, I wonder if a simple solution for now wouldn't be to hide the print invoice button when showing incomplete orders.

A bit rough, but easy to test / cleaner on the UI rather than a button with a tooltip.

I'm not a big fan of disabling the button when incomplete orders are shown. I'm afraid the user will try to unselect incomplete order one by one before even thinking at the tooltip or at the incomplete checkbox.

@openfoodfoundation/train-drivers-product-owners @openfoodfoundation/testers @openfoodfoundation/core-devs any opinion on this?

RachL · 2021-12-24T09:30:25Z

Hello @sinansonmez sorry this took so long :(

I will rewrite the original issue to include the decided solution. Let me know if you still want to work on it. Otherwise I think we will close the PR :(

sinansonmez · 2021-12-24T09:31:59Z

Hello @RachL unfortunately I will not be able to work on this. Thanks

printing invoices for orders not in completed state

0ea7688

andrewpbrett reviewed Sep 24, 2021

View reviewed changes

filipefurtad0 added the feedback-needed label Sep 27, 2021

filipefurtad0 removed the feedback-needed label Sep 27, 2021

Print invoices button is disabled if only show complete orders not ch…

ef6105b

…ecked

mkllnk requested changes Sep 30, 2021

View reviewed changes

sinansonmez mentioned this pull request Nov 5, 2021

Bugsnag when printing of alternative-invoice of orders in cart state #5708

Closed

RachL closed this Dec 24, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix infinite loop when printing invoices for orders not in completed #8197

Fix infinite loop when printing invoices for orders not in completed #8197

sinansonmez commented Sep 16, 2021

andrewpbrett left a comment

sinansonmez commented Sep 25, 2021 •

edited

Loading

filipefurtad0 commented Sep 27, 2021 •

edited

Loading

RachL commented Sep 27, 2021

jaycmb commented Sep 27, 2021 •

edited

Loading

sinansonmez commented Sep 27, 2021 •

edited

Loading

jaycmb commented Sep 27, 2021 •

edited

Loading

sinansonmez commented Sep 27, 2021 •

edited

Loading

mkllnk left a comment

mkllnk Sep 30, 2021

mkllnk Sep 30, 2021

sinansonmez commented Sep 30, 2021

jaycmb commented Sep 30, 2021

mkllnk commented Oct 1, 2021

sigmundpetersen commented Oct 1, 2021

sinansonmez commented Oct 5, 2021 •

edited

Loading

RachL commented Nov 16, 2021

RachL commented Dec 24, 2021

sinansonmez commented Dec 24, 2021

Fix infinite loop when printing invoices for orders not in completed #8197

Fix infinite loop when printing invoices for orders not in completed #8197

Conversation

sinansonmez commented Sep 16, 2021

What? Why?

What should we test?

Release notes

Dependencies

Documentation updates

andrewpbrett left a comment

Choose a reason for hiding this comment

sinansonmez commented Sep 25, 2021 • edited Loading

filipefurtad0 commented Sep 27, 2021 • edited Loading

RachL commented Sep 27, 2021

jaycmb commented Sep 27, 2021 • edited Loading

sinansonmez commented Sep 27, 2021 • edited Loading

jaycmb commented Sep 27, 2021 • edited Loading

sinansonmez commented Sep 27, 2021 • edited Loading

mkllnk left a comment

Choose a reason for hiding this comment

mkllnk Sep 30, 2021

Choose a reason for hiding this comment

mkllnk Sep 30, 2021

Choose a reason for hiding this comment

sinansonmez commented Sep 30, 2021

jaycmb commented Sep 30, 2021

mkllnk commented Oct 1, 2021

sigmundpetersen commented Oct 1, 2021

sinansonmez commented Oct 5, 2021 • edited Loading

RachL commented Nov 16, 2021

RachL commented Dec 24, 2021

sinansonmez commented Dec 24, 2021

sinansonmez commented Sep 25, 2021 •

edited

Loading

filipefurtad0 commented Sep 27, 2021 •

edited

Loading

jaycmb commented Sep 27, 2021 •

edited

Loading

sinansonmez commented Sep 27, 2021 •

edited

Loading

jaycmb commented Sep 27, 2021 •

edited

Loading

sinansonmez commented Sep 27, 2021 •

edited

Loading

sinansonmez commented Oct 5, 2021 •

edited

Loading