Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
filipefurtad0 opened this issue Jun 30, 2020 · 8 comments
Closed
Labels
bug-s3 The bug is stopping a critical or non-critical feature but there is a usable workaround. hackathon Issues for upcoming hackathons

Comments

@filipefurtad0
Copy link
Contributor

filipefurtad0 commented Jun 30, 2020

Description

Bulk printing of orders in cart state generates errors 422 (seen in the console) and Bugsnag errors, across staging servers (both v3 and v2.1). The user sees an endless-spinner and several GET requests are sent.

In staging-ES:

image

https://app.bugsnag.com/katuma/katuma/errors/5ef35f60d643330019edb2c6?filters[event.since][0]=30d&filters[error.status][0]=open

ActionView::Template::ErrorBulkInvoiceService#start_pdf_job_without_delay
undefined method `to_date' for nil:NilClass Did you mean?  to_a18 seconds agoJun 30th, 14:40:35 UTC | staging |  -- | -- | --

In staging-AU

image

https://app.bugsnag.com/yaycode/openfoodnetwork/errors/5ef367c30c9a67001715b9dc?event_id=5ef367c3005b7b5abac50000&i=sk&m=nw

ActionView::Template::ErrorBulkInvoiceService#start_pdf_job_without_delay
undefined method `full_address' for nil:NilClass

Expected Behavior

Hide the print invoice button when showing incomplete orders.

Actual Behaviour

Send a bulk print job to cart state orders triggers (many) errors above and leaves the user with an endless spinner.

Steps to Reproduce

  1. Login as superadmin
  2. Go to /orders and untick the "Only show complete orders" box
  3. Press Filter Results
  4. Select an order in the Cart state
  5. Open the browser console and press Print Invoices

Animated Gif/Screenshot

Workaround

You need to press escape or F5 to exit the endless-spinner.

Severity

This is probably an edge case, but generates a lot of errors...
bug-s3: a feature is broken but there is a workaround

Your Environment

  • Version used: v2.1 and v3.
  • Browser name and version: Firefox 77
  • Operating System and version (desktop or mobile): Desktop/Ubuntu

Possible Fix

@filipefurtad0 filipefurtad0 added the bug-s3 The bug is stopping a critical or non-critical feature but there is a usable workaround. label Jun 30, 2020
@RachL RachL added the hackathon Issues for upcoming hackathons label Sep 25, 2020
@filipefurtad0
Copy link
Contributor Author

filipefurtad0 commented Aug 20, 2021

This issue concerns the alternative invoice only - I've updated the title. Some additional notes:

  • the 422 error is displayed even for complete orders when bulk-printing

However:

and leaves the user with the endless spinner. Waiting long enough should display:

image

@filipefurtad0 filipefurtad0 changed the title Bugsnag and error 422 on bulk print invoice of orders in cart state Bugsnag when printing of alternative-invoice of orders in cart state Aug 20, 2021
@sinansonmez
Copy link
Contributor

sinansonmez commented Sep 13, 2021

Hello @filipefurtad0.

I recreated this issue, please find the video below. However, as you can see below, the system actually created the invoices without any issue.

I wanted to confirm the expected behavior: If an order in payment or cart status, the Print Invoices button should be disabled, right? If yes, I can start working on the solution.

Screen.Recording.2021-09-13.at.20.59.30.mov

EDIT: After certain period of time, I tried to reprint the invoices, as you mentioned above I get an endless spinner. The reason is that from the user side (who created the order), the cart is emptied automatically, after certain period. However, from the admin side, I can still see the order in the orders list with Cart status. I believe this is the main reason why user is getting endless spinner. We have two alternative solutions:

- QUICK WIN: Disable the Print Invoices button when orders with payment or cart status selected --> If you agree, I would implement this solution for now.
- OPTIMAL: Admin also shouldn't see the orders with payment or cart status after certain period (i.e. after these orders are cleared from user side)

@filipefurtad0
Copy link
Contributor Author

Hey @sinansonmez ,
Thanks for that feedback.

Agree with your proposal as quick win, as this is the current behavior, when one tries to print each order, under https://staging.openfoodnetwork.org.uk/admin/orders/<order_nr>/edit, the Actions menu is disabled:

image

This menu becomes enabled once an order is in state = complete. So I think the bulk order printing (done from /orders) should follow this behavior as well:

  • only orders in state complete should display the checkbox to be selected for bulk invoice printing, in the orders table (updated Expected Behavior accordingly)

This should prevent the endless spinner from being displayed to the user.

@RachL

This comment was marked as outdated.

@sigmundpetersen

This comment was marked as outdated.

@sinansonmez

This comment was marked as outdated.

@pedrocarmona
Copy link
Contributor

Hey, I believe this card is no longer valid. The view logic has changed in #10833

@RachL
Copy link
Contributor

RachL commented Sep 2, 2023

Thanks @pedrocarmona ! Closing :)

@RachL RachL closed this as not planned Won't fix, can't repro, duplicate, stale Sep 2, 2023
@github-project-automation github-project-automation bot moved this from All the things to Done in OFN Delivery board Sep 2, 2023
@github-project-automation github-project-automation bot moved this from Backend Easy to Done in Welcome New Developers! Sep 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug-s3 The bug is stopping a critical or non-critical feature but there is a usable workaround. hackathon Issues for upcoming hackathons
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants