bulk_order_management_spec.rb test fails due to timezone bug

[jibees]

What we should change and why

If server timezone is different from the client's browser one, orders may be returned when they should not be (and vice versa)

How to reproduce the bug?

You can change your timezone config test by editing config/environments/test.rb and change line 34 by config.time_zone = "UTC" if you're not running you're browser in UTC. Therefore, tests in bulk_order_management_spec line 466 and line 478 should fail.

Output of the failing tests:

  1) 
  As an Administrator
  I want to be able to manage orders in bulk
 using page controls using date restriction controls only loads line items whose orders meet the date restriction criteria
     Failure/Error: expect(page).to have_no_selector "tr#li_#{li1.id}"
       expected not to find visible css "tr#li_52", found 1 match: "John Doe March 07, 2021 Enterprise 128 Product #90 - 8050: 1g, S 1"
     # ./spec/features/admin/bulk_order_management_spec.rb:466:in `block (4 levels) in <top (required)>'

  2) 
  As an Administrator
  I want to be able to manage orders in bulk
 using page controls using date restriction controls displays only line items whose orders meet the date restriction criteria, when changed
     Failure/Error: expect(page).to have_selector "tr#li_#{li3.id}"
       expected to find css "tr#li_58" but there were no matches
     # ./spec/features/admin/bulk_order_management_spec.rb:478:in `block (4 levels) in <top (required)>'

Order returned although should not be (expected not to find visible css "tr#li_52", found 1 match)

1. Date picker displays 2021-03-08 and query is sent to controller with param completed_at_gteq with value : 2021-03-08T00:00:00+01:00 which is, converted in UTC: 2021-03-07T23:00:00.000Z.
2. That's why order completed at 2021-03-07 23:59:59 UTC is therefore returned by the controller.
3. So in the user interface, the order is displayed with completed at value as : March 07, 2021 because the json contains the formatted string of the date, without taken into account the timezone of the user (see def completed_at in order_serializer.rb)

Order not returned although should be (expected to find css "tr#li_58" but there were no matches)

1. Date picker displays 2021-03-15 and query is sent to controller with paramParam completed_at_lt with value: 2021-03-16T00:00:00+01:00 that is to say, in UTC: 2021-03-15T23:00:00.000Z
2. That's why order completed at 2021-03-15 23:59:59 UTC is not returned by controller.
3. And consequently not display in the user interface.

Old description (fixed by #7119, as we know use the same timezone in both server and client)

Tests in bulk_order_management_spec.rb fails since commit e341b044878792a64a8e39b1613434c4cee4b38a

File: config/environments/test.rb

- config.time_zone = ENV.fetch("TIMEZONE", "Melbourne")
+ config.time_zone = "UTC"

Context

PR: #6902 https://github.com/openfoodfoundation/openfoodnetwork/pull/6902/commits/5e19e5e83d88d741b1f49204892071ef3bfca28c

[Matt-Yorkley]

- config.time_zone = ENV.fetch("TIMEZONE", "Melbourne")
+ config.time_zone = "UTC"

I think the Semaphore configs explicitly pass an ENV var for the timezone, and I guess the Github CI setup does not, but it wants UTC. I wonder if going half-way between those two options is the best? We could do:

config.time_zone = ENV.fetch("TIMEZONE", "UTC")

I think that will satisfy both...

[jibees]

Thanks @Matt-Yorkley In term of strictly green build, I think you are right and we should quickly follow your recommendations. But, to go further and regarding my investigations (I've updated the issue description), I think the issue is deeper than "just" timezone configuration into test env. And I've no idea on how to fix this.

[Matt-Yorkley]

Cool, I made a one-line PR for that change. Happy to leave this issue open though if you think it needs more investigation :+1:

[jibees]

Thanks @Matt-Yorkley ! Now I think this is issue is no more tech debt, and its priority should slightly decrease (and it seems that no one was bothered by this behaviour)

[sauloperez]

Where do you see we're converting the dates to UTC @jibees ? AFAIK we should send UTC dates to the backend and then localize them on the FE them depending on the user's timezone. Am I right?

[jibees]

I don't see anything that convert in UTC in the backend (actually I haven't looked for). In my explanations I've just converted to UTC to understand and compare date between each others. We send date with timezone to the server which can be a valid solution as it seems to be correctly understood (as we return the right orders)

then localize them on the FE them depending on the user's timezone. Am I right?

The date is formatted in the json, without taking into account the user's timezone.