-
Notifications
You must be signed in to change notification settings - Fork 165
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
Adds cdt
parameter to tracking query items
#301
Conversation
Generated by 🚫 Danger |
Hey @notjosh, this looks great! I am currently trying to get one of the backend developer to check if it's fine to send a "normal" ISO-8601 date along which I would like the most. If we don't hear back from them soon I am happy to merge it though. I'll get back to you within a day. |
Great! No rush though, I can use my branch for as long as necessary :) |
something to keep in mind... there is this setting:
So if the cdt is older than one day and no token is specified, Matomo will throw an exception and not record the request. Don't know if there's a feature that eg sends tracking requests while the user was offline eg a day later... these would not be recorded then. |
I haven't tested ISO8601 string, timestamp should work well though. |
I was looking through the Android code for other reasons earlier, but it applies here:
and
Android is already attaching Either it's ok to do that here, or it should probably be dropped from Android too, so we're not seeing inconsistent results across platforms.
Is it really much different? It's still (presumably) to be hidden behind a Ultimately, I don't mind either way - I call the SDK, and you decide how to send those dates :) But at the moment it's fairly inconsistent across platforms, so it might be helpful to align those internally across teams. |
Hey @notjosh, I had a chat with the backend engineers. ISO8601 is fine. Let's change over there (if you are fine as well). I'll try to get the documentation updated in that regard. |
Hiya, I've bumped to use ISO 8601 date formatter (in 2feaae1). It's trying to use |
Gentle bump. Any progress on this? Happy to maintain a fork (we need the functionality!) if it's a Bad Idea on your side though. |
Hey @notjosh, thanks for your patience. And thanks for the bump. I did forget about your PR here. |
Oh, doh, you're right - Prettier bumped all the bullets. I rebased, fixed the changelog, and cleaned out the merge commit. Should be ok from my side, assuming tests are green |
Perfect. Thanks a lot! |
This just got released in version 7.0.0 |
Because of ✨reasons✨, the
h
/m
/s
values don't seem to be used in calculating "time on page" in the backend, butcdt
does.This PR attaches
cdt
to the tracking events in addition to the existingh
/m
/s
values.According to this issue,
cdt
will be used to calculate the "time on page" correctly for the clients, regardless of when they're sent to the serverAccording to these docs, the format is➡️ Using ISO 8601 format since 2feaae1yyyy-MM-dd HH:mm:ss
(i.e.2011-04-05 00:11:42
) - not quite ISO-8601, but close (although we could use a unix timestamp? They're always a little harder to grok for human eyes though). However (and this is an implementation detail, so probably best to ignore or update docs), the server uses unix timestamp internally, and processes it viastrtotime()
, so we could use a formal ISO 8601 date string here if we'd prefer.Resolves #299