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

Jump to bottom

include server time (as timestamp) in visitor actions (visitor live API) #6814

Merged
merged 1 commit into from Jan 8, 2015
Merged

include server time (as timestamp) in visitor actions (visitor live API) #6814

merged 1 commit into from Jan 8, 2015

Conversation

FelixSchwarz
Copy link
Contributor

Events in the returned JSON/XML only have a field "serverTimePretty" but no locale-independent time field (e.g. Unix epoch, iso9660, ...). I don't see a reason why the API does not contain the date/time in such a format.

Please note I'm not a PHP developer and this was only tested lightly.

@FelixSchwarz FelixSchwarz mentioned this pull request Dec 7, 2014
Open
@mattab
Copy link
Member

mattab commented Dec 10, 2014

Thanks for the PR @FelixSchwarz it is a nice suggestion.

I think we can simply call it timestamp in this case as it is already within the action element in hierarchy.

@mattab mattab added the Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc. label Dec 10, 2014
@mattab mattab added this to the Piwik 2.10.0 milestone Dec 10, 2014
@mattab mattab added the Pull Request WIP Indicates the current pull request is still work in progress and not ready yet for a review. label Dec 10, 2014
mattab added a commit that referenced this pull request Dec 10, 2014
@mattab
Ignore column timestamp from system tests comparison refs #6814
4fbc773
@mattab
Copy link
Member

mattab commented Dec 15, 2014

Hi @FelixSchwarz would you like to make the change to calling it timestamp ?

@mattab mattab modified the milestones: Piwik 2.11.0, Piwik 2.10.0 Dec 15, 2014
@FelixSchwarz
Copy link
Contributor Author

Ah ok, can do.

Just one thing: When all the other fields are called 'serverTimestamp' why calling this one "timestamp"? Anyway, that's a judgment call for your to make so I'm perfectly fine submitting a patch for "timestamp".

@mattab
Copy link
Member

mattab commented Dec 18, 2014

Just one thing: When all the other fields are called 'serverTimestamp' why calling this one "timestamp"?

in serverTimestamp the word server is bit useless as it's implied it's the UNIX timestamp. so we might as well make the new fields more to the point 👍

@FelixSchwarz
include server time (as timestamp) in visitor actions (visitor live API)
9eb6ed6 
v2: rename attribute to 'timestamp' as suggested by mattab in pull request matomo-org#6814
@FelixSchwarz
Copy link
Contributor Author

I updated my commit however the tests are still failing because 'OneVisitorSeveralDaysImportedInRandomOrderTest' sets keepLiveDates so "timestamp" is not ignored in Response.shouldDeleteLiveDates(). After a quick look into VisitOverSeveralDaysImportedLogs.php I decided to leave this adaptation up to you :-)

mattab pushed a commit that referenced this pull request Jan 8, 2015
Merge pull request #6814 from FelixSchwarz/visitorliveapi-servertimes…
fa762c1 
…tamp

include server time (as timestamp) in visitor actions (visitor live API)
@mattab mattab merged commit fa762c1 into matomo-org:master Jan 8, 2015
mattab added a commit that referenced this pull request Jan 8, 2015
@mattab
fix build refs #6814
0aba204
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc. Pull Request WIP Indicates the current pull request is still work in progress and not ready yet for a review.
Projects
None yet
Milestone
Piwik 2.11.0
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@FelixSchwarz @mattab