Skip to content
This repository has been archived by the owner on Jun 7, 2022. It is now read-only.

Orders panel shows no orders message when orders exist #2309

Closed
joshuatf opened this issue May 27, 2019 · 7 comments
Closed

Orders panel shows no orders message when orders exist #2309

joshuatf opened this issue May 27, 2019 · 7 comments
Assignees
Labels
focus: activity panel Issues about activity/task panels on the home screen. type: bug The issue is a confirmed bug.

Comments

@joshuatf
Copy link
Contributor

Describe the bug
A message is shown in the Orders activity panel that the store has not received any orders when it has.

I believe this is due to the before/after params defaulting to a 1 week period so totalNonActionableOrders results in 0 if no recent orders have been placed.

const totalNonActionableOrders = getReportItems( 'orders', allOrdersQuery ).totalResults;

Also related - #1103

To Reproduce
Steps to reproduce the behavior:

  1. Make sure you have orders, but none within the past week.
  2. Open the orders activity panel.
  3. Note the message.

Expected behavior
Expected to see the message relevant to having previous orders but no actionable statuses:
Good job, you've fulfilled all of your new orders!

Or alternatively, a message that mentions there have been no orders within the past week.

Screenshots
Screen Shot 2019-05-27 at 11 29 45 AM

@joshuatf joshuatf added type: bug The issue is a confirmed bug. focus: activity panel Issues about activity/task panels on the home screen. labels May 27, 2019
@joshuatf joshuatf added this to wc-admin Backlog 🔙 in Isotope via automation May 27, 2019
@timmyc timmyc moved this from wc-admin Backlog 🔙 to 📊 wc-admin Sprint Backlog in Isotope Jun 12, 2019
@Aljullu
Copy link
Contributor

Aljullu commented Jun 13, 2019

Good catch! A quick solution to this problem would be adding after: '1970-01-01T00:00:00' or something similar to the query. But I'm not sure to understand what's the rationale of having default values for before & after. Couldn't we just remove them and let the client specify them if required?

@joshuatf
Copy link
Contributor Author

Good question, @Aljullu.

A quick solution to this problem would be adding after: '1970-01-01T00:00:00' or something similar to the query.

I think this is an acceptable solution, but...

But I'm not sure to understand what's the rationale of having default values for before & after. Couldn't we just remove them and let the client specify them if required?

This feels like the right solution and what I would expect hitting this endpoint. I think the only major concerns would be whether or not there are performance impacts. @peterfabian mentioned in #1103 that these may be minimal with pagination.

It might be worth doing some tests without those time restraints to see if there are any big performance concerns.

@jeffstieler jeffstieler added the needs: author feedback The issue/PR needs a response from any of the parties involved in the issue. label Jun 18, 2019
@jeffstieler
Copy link
Contributor

Or alternatively, a message that mentions there have been no orders within the past week.

This message would need to change based on the default date range setting, no?

It might be worth doing some tests without those time restraints to see if there are any big performance concerns.

What do you think about removing the date range and introducing a LIMIT to the query instead?

@Aljullu
Copy link
Contributor

Aljullu commented Jun 19, 2019

This message would need to change based on the default date range setting, no?

AFAIK the date range setting should not affect what we display in the Activity Panel. If there is an on-hold order from (let's say) one year ago, it should still appear in the Activity Panel.

What do you think about removing the date range and introducing a LIMIT to the query instead?

I might be wrong, but given that it has pagination by default, that means there is already a LIMIT in the query, isn't it?

@rrennick
Copy link
Collaborator

Couldn't we do wc_get_orders( array( 'status' => 'processing', 'limit' => 1 ) ) if the current query returns no rows to determine which message to display?

@dreiris dreiris moved this from 📊 wc-admin Sprint Backlog to Sprint 21 (July 9 - July 23) in Isotope Jul 8, 2019
@rrennick
Copy link
Collaborator

rrennick commented Jul 9, 2019

What do you think about removing the date range

On my REST client the time went from 0.187 to 0.213 when I added &after=1990-01-01%2000:00:00 to the query string. The default limit of 10 rows should keep this at a low performance hit.

@rrennick rrennick self-assigned this Jul 9, 2019
@rrennick rrennick removed the needs: author feedback The issue/PR needs a response from any of the parties involved in the issue. label Jul 9, 2019
@jeffstieler
Copy link
Contributor

Fixed in #2659.

Isotope automation moved this from Sprint 21 (July 9 - July 23) to Done Sprint 21 (July 9 - July 23) Jul 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
focus: activity panel Issues about activity/task panels on the home screen. type: bug The issue is a confirmed bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants