search

zinc-collective / convene

An Operating System for the Solidarity Economy
https://convene.zinc.coop
Other
56 stars 19 forks source link

🐛 `Marketplace`: ActionView::Template::Error: undefined method `strftime' for "BETWEEN 4:30 PM - 6PM":String #1265

Closed sentry-io[bot] closed 1 year ago

sentry-io[bot] commented 1 year ago 
NoMethodError: undefined method `strftime' for "BETWEEN 4:30 PM - 6PM":String

            value&.strftime("%Y-%m-%dT%T")
                 ^^^^^^^^^^
Did you mean?  strip
  app/views/application/_datetime_field.html.erb:3:
    <%= form.datetime_field attribute %>
  app/furniture/marketplace/carts/_cart.html.erb:6:
    <%= render "datetime_field", attribute: :delivery_window, form: cart_form, disabled?: cart.marketplace.delivery_window.present? %>
  app/furniture/marketplace/carts/_cart.html.erb:4:
    <%= form_with model: cart.location do |cart_form| %>
  app/furniture/marketplace/marketplaces/_marketplace.html.erb:9:
    <%= render cart %>
  app/views/furnitures/_furniture.html.erb:34:
    <%= render furniture.furniture %>
...
(239 additional frame(s) were not displayed)

ActionView::Template::Error: undefined method `strftime' for "BETWEEN 4:30 PM - 6PM":String

            value&.strftime("%Y-%m-%dT%T")
                 ^^^^^^^^^^
Did you mean?  strip
anaulin commented 1 year ago

This was introduced when we changed the expected type of Marketplace#delivery_window in https://github.com/zinc-collective/convene/pull/1251, but did not migrate the existing data in the https://piikup-sandbox.zinc.coop instance.

anaulin commented 1 year ago

we are going from strings that mostly look like they were meant to be mostly time intervals, to one DateTime value, so there is really no good way to migrate this data.

I'm going to at least try to parse the existing string-type delivery windows to Dates, which will stop the 500s and whill gives us something like:

> m = Marketplace::Marketplace.all.select { |m| m.delivery_window.present? }
> m.map { |m| [m.id, m.delivery_window, begin DateTime.parse(m.delivery_window) rescue nil end] }
=> 
[["3c2df6a3-f6b0-4e4e-b137-79860e8b28c9", "as needed", nil],                                                                      
 ["f07fe90e-5a1d-43ca-aca2-fb8dfa70d915", "Same day 3pm - 6pm in Oakland , Berkeley and Alameda", Sat, 25 Mar 2023 15:00:00 +0000],
 ["dfb19803-7131-4443-ad6d-80cbc1f4571e", "Monday from 10AM to 2PM", Mon, 20 Mar 2023 10:00:00 +0000],                            
 ["0025198b-2196-4a26-96b1-921fb325ba84", "FRIDAY BETWEEN 3PM - 6PM", Fri, 24 Mar 2023 15:00:00 +0000],                           
 ["4bee9409-3ffa-4e49-b002-bbe3d63887fe", "BETWEEN 4:30 PM - 6PM", Sat, 25 Mar 2023 16:30:00 +0000]]

It feels like delivery windows really need to be a time interval, and we probably want a list of them in each Marketplace.

anaulin commented 1 year ago

I have fixed the stored delivery windows, but we now have a different bug, where the cart returns its delivery window as a string:

undefined method `strftime' for "2023-03-20T10:00:00.000+00:00":String

            value&.strftime("%Y-%m-%dT%T")
                 ^^^^^^^^^^
Did you mean?  strip
zspencer commented 1 year ago

I left some comments in the other P.R. but one thing we want to keep in mind is that the Delivery Window is supposed to be pretty flexible; like they do want to preserve the ability to enter a plain string describing the delivery constraints until we get to the point where we've modeled it well.

So, Carts should have a Delivery object that is the time they would like to receive the items, and the Marketplace should have a DeliveryWindow?DeliverySchedule? Which is used to indicate what options a Shopper has when ordering.

zspencer commented 1 year ago

I've:

Which appears to have fixed the bug !