zinc-collective / convene

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

🐞 `Marketplace`: order time is displayed in UTC #1660

Closed anaulin closed 9 months ago

anaulin commented 10 months ago

This order was placed at 2:15 AM local time, but in Convene it shows as: image

We should try to display this timestamp in the user's timezone (as inferred from the browser).

zspencer commented 10 months ago

Do you have thoughts on how we can determine the appropriate time zone? Should we just presume PT as the application default for now? Or should we let it be set on the Person and Space level?

anaulin commented 10 months ago

I was thinking we could use something like https://github.com/kbaum/browser-timezone-rails to set the timezone for each request.

anaulin commented 10 months ago

Also, we could do one or all of:

rosschapman commented 10 months ago

Converting the value automatically using the browser timezone seems sufficient to me unless Piikup mentioned they wanted configurability to handle some use case?

I tend to like an abstraction around 3rd-party lib helpers like humanized_money_with_symbol. Like, creating our own ViewComponent called DateTime or something. It cold parameterize TZ in case it does become dynamically set later.

anaulin commented 10 months ago

@rosschapman agreed on starting simple, since nobody has asked for anything special, to the best of my knowledge.

I also like the idea of encapsulating the whole thing, although a helper method might be more lightweight than a ViewComponent, just so that in views we can do something like:

Order placed at <%= in_user_tz(date: date) %>

instead of:

Order placed at <%= render DateComponent.new(date: date) %>

Although if we use something like the gem I suggested above (which is used by one of the clients I work for), then we don't need to do anything special in views, because it sets the timezone on a per-request basis at the Rails level, so when you display a date in the Rails timezone it shows up in the correct timezone.

zspencer commented 10 months ago

I believe localize(date, :format) is what you're looking for from a "how to format dates?" perspective; as this calls #to_formatted_s under the covers.

In most of the rails app I work on with time zones; we use Time.use_zone in an around_action to ensure the requests / response use a specified time zone in both ApplicationController and ApplicationMailer to call Time.with_zone.

Since emails are delivered off the thread that handles requests/responses; I don't know that the browser time zone gem will work quite right to fix this...

This leads me to think that for now, having a Neighborhood time zone set via ENV variable is probably the "cheapest thing that will work."

But I agree, we may want to support person and space level time zones once we start crossing those boundaries.

zspencer commented 9 months ago

Aight, gonna tackle this now.

zspencer commented 9 months ago

Patch is out: https://github.com/zinc-collective/convene/pull/1706

zspencer commented 9 months ago

I'm closing this since it's been in prod for a while and appears to be working well.