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

Date error when logging caches #14746

Closed
ebmonon36 opened this issue Oct 20, 2023 · 12 comments
Closed

Date error when logging caches #14746

ebmonon36 opened this issue Oct 20, 2023 · 12 comments
Labels
Bug Issues classified as a bug

Comments

@ebmonon36
Copy link

ebmonon36 commented Oct 20, 2023

Describe your problem!

I logged a cache this evening, around 10pm. I notice that even though I have the current date selected in c:geo, the Geocaching website shows the logged date as tomorrow. My time zone is GMT-4. I went back to c:geo and it said that I had found it on the current date, but upon refreshing the cache in c:geo, it updates to match the incorrect date on the Geocaching website. I first noticed this issue after updating to 2023.10.15-RC2.

Edit: it did it again today when I logged the cache just after 9pm EDT.

How to reproduce?

Log a cache after a certain time of day. I'm not sure what time that is yet.

Actual result after these steps?

The found date in c:geo shows the current date, but the Geocaching website shows tomorrow as the date logged.

Expected result after these steps?

The found date listed on the Geocaching website should match the date selected in c:geo.

Reproducible

Yes

c:geo Version

2023.10.15-RC2

System information

Device type: SM-S906U
Android version: 13
Android build: TP1A.220624.014.S906USQS3CWI1

Additional Information

Also reported on support, eg ticket 496584

@ebmonon36 ebmonon36 added Bug Issues classified as a bug Unverified Issue not yet confirmed/reproduced or feature requests not yet checked for plausibility labels Oct 20, 2023
@Lineflyer
Copy link
Member

@eddiemuc
Can you check if there is a different handling of the timestamp we send for the log between old flow and new flow?

@eddiemuc
Copy link
Contributor

@eddiemuc Can you check if there is a different handling of the timestamp we send for the log between old flow and new flow?

Yes, there is a difference. While with the old API only the day is submitted (Year, month, day), in the new API a timestamp is submitted (year, month, day, hour, minute, second, ms).

I thought that this issue is yet another reincarnation of the well-known problem of c:geo with timezones. Is this not the case?

@Lineflyer
Copy link
Member

Lineflyer commented Oct 25, 2023

I thought that this issue is yet another reincarnation of the well-known problem of c:geo with timezones. Is this not the case?

Before we do have problems "the other way around"...e.g. showing a logbook date in c:geo without respecting time zone might lead to wrong date in c:geo.
Now we have a problem when sending to the server...thats new.
No idea what GC expects, but we probably should send the date with 00:00Z time (I guess you use the local time of where the log is composed in c:geo currently but GC expects GMT or a similar issue)

@eddiemuc
Copy link
Contributor

I won't be able to provide a "fix" for this during next days. Should be fairly easy to do for anyone though. The key method is in class GCWebAPI and called sendLogNew. It has the log date/timestamp as a parameter. This parameter is currently put into the GC.com interface as it comes (usually from the log activities) as a json timestamp via Jackson mapping.

If it is enough to throw away the time info then instead of the incoming log date a new date should be created using only the year-month-day-info from this date and the time set to 00:00:00.

@eddiemuc
Copy link
Contributor

I was able to reproduce and confirm this bug.

  • When logging over the website, gc.com sends the LOCAL time in the logDate timestamp
  • When logging via c:geo, we currently send the GMT time

Example: I test-logged a cache today around 9:45 german time (GMT+2) first on website then on c:geo

  • Logging it via website sets: logDate to "2023-10-28T09:42:09,208Z"
  • Logging it via c:geo sets: logDate to "2023-10-28T07:45:53"

I am not very firm with timezones, how should this be handled. Should we send local time too? If yes, which local time? That of the timezone where the cache is located (how would I get that?) or the timezone where the current user (resp his mobile) is currently located (where would I get that?).

Or is there a way to tell JSON that the given time is GMT? To be tested whether gc.com would handle this correctly though...

@eddiemuc eddiemuc removed the Unverified Issue not yet confirmed/reproduced or feature requests not yet checked for plausibility label Oct 28, 2023
@eddiemuc
Copy link
Contributor

Some more questions:

  • log time used in my tests is the time of log creation
  • haven't tested how c:geo changes the timestamp when user changes the log date to e.g. yesterday or last week. Question is however: how should this be handled? Should the timestamp part (hours, minutes. Seconds, Ms) stay the same when log date ia changed? Should it be set to 00:09:00.000? Local time or GMT? Something else?
  • how should offline logs be handled? If local time of mobile device is used, should it be that if the time zone when log was created or that of the time zone where log is sent?

@moving-bits
Copy link
Member

Another user on support reporting this. This user is in the US, MDT timezone.

"I have notice an issue when I am logging caches through the app I am in the US and as I log the caches it puts the date one day ahead of when I found the cache.
For example I found a cache today 10/28/23 and logged it through the app. I signed into to the geocaching website and the log for the same cache shows as found on 10/29/23."

Ticket 903980

@murggel
Copy link
Contributor

murggel commented Oct 29, 2023

Some more questions:

  • log time used in my tests is the time of log creation
  • haven't tested how c:geo changes the timestamp when user changes the log date to e.g. yesterday or last week. Question is however: how should this be handled? Should the timestamp part (hours, minutes. Seconds, Ms) stay the same when log date ia changed? Should it be set to 00:09:00.000? Local time or GMT? Something else?

Timestamp part should stay, so the order of logs of the day remain the same, and I don't have to consider sending order.

  • how should offline logs be handled? If local time of mobile device is used, should it be that if the time zone when log was created or that of the time zone where log is sent?

For me it would be the creation-time stamp, because normally I create the log on log-time and send it later. So the log-time-zone would be the one of creation-Moment.

@eddiemuc
Copy link
Contributor

eddiemuc commented Oct 29, 2023

PR #14774 changes json serialization to use local timezone instead of UTC (which apparently is the default for Jackson Json). Behaviour should now be the same as before in the "old" way, except that time-part is added.

  • time part will be the time of log creation
  • timezone used will be the one the device is set to at the time the log is actually send to gc.com

The overall timezone topic is not solved by this (nor was it before), this remains to be discussed at another point in time.

@eddiemuc
Copy link
Contributor

eddiemuc commented Oct 29, 2023

NB: gc.com itself handles timezones wrong IMHO, because in its json requests it uses the Z postfix (which would mark the timestamp being in timezone UTC/GMT), but sends a local timestamp instead. The above approach mimics this (IMHO wrong) behaviour as best as possible. I couldn't bring myself to add the Z postfix to the JSONs sent by c:geo because that would make the timestamp format REALLY wrong, but apparently it works the same on gc.com side w/o adding any timezone info either.

@fm-sys
Copy link
Member

fm-sys commented Oct 29, 2023

Is the jackson lib only used for GC? I'm wondering whether it's a good approach to change timestamp parsing behavior globally in cgeo...

@eddiemuc
Copy link
Contributor

Is the jackson lib only used for GC? I'm wondering whether it's a good approach to change timestamp parsing behavior globally in cgeo...

I scanned code, apparently the new log API is the only one using the global mapper of JsonUtils as well as date/timestamp fields with Jackson.

Also, c:geo widely uses SimpleDateFormat which by default (and unlike Jackson) uses the local timezone. So at least by changing the setting globally we create some sort of consistency inside c:geo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Issues classified as a bug
Projects
None yet
Development

No branches or pull requests

6 participants