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

Fixes some comparison issues #15583

Merged
merged 2 commits into from Feb 17, 2020
Merged

Fixes some comparison issues #15583

merged 2 commits into from Feb 17, 2020

Conversation

sgiehl
Copy link
Member

@sgiehl sgiehl commented Feb 17, 2020

fixes #15194, #15193

@sgiehl sgiehl added not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. Needs Review PRs that need a code review labels Feb 17, 2020
@sgiehl sgiehl added this to the 4.0.0 milestone Feb 17, 2020
@@ -63,7 +63,7 @@
},

getDateRange: function () {
return [this.dateInPeriod, this.dateInPeriod];
return [new Date(this.dateInPeriod.getTime()), new Date(this.dateInPeriod.getTime())];
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need to ensure new date objects are returned here. Otherwise only references for this.dateInPeriod were returned, causing problems when they were manipulated. (That caused day periods compared two years back)

@Findus23 Findus23 linked an issue Feb 17, 2020 that may be closed by this pull request
@diosmosis
Copy link
Member

LGTM

@diosmosis diosmosis merged commit e8cb9c2 into 4.x-dev Feb 17, 2020
@diosmosis diosmosis deleted the comparison branch February 17, 2020 22:34
@tassoman
Copy link
Contributor

Hello there!
Is there any regression about this bug fixes? I've installed 3.13.5 in staging environment and I still feeling the wrong behaviour.
I thought were fixed some months ago. 🤔

@tassoman
Copy link
Contributor

The «interval» bug is confirmed also for month period

@tsteur
Copy link
Member

tsteur commented May 27, 2020

@tassoman this was only fixed in 4.x AFAIK. @tassoman feel free to create a PR for 3.X and I can merge it should you need the patch earlier

tassoman added a commit to tassoman/matomo that referenced this pull request Jun 1, 2020
@tassoman tassoman mentioned this pull request Jun 1, 2020
tassoman pushed a commit to tassoman/matomo that referenced this pull request Jun 3, 2020
* ensure new date objects are returned

* return chosen period when comparing with previous year
tsteur pushed a commit that referenced this pull request Jun 3, 2020
* ensure new date objects are returned

* return chosen period when comparing with previous year

Co-authored-by: Stefan Giehl <stefan@matomo.org>
jonasgrilleres pushed a commit to 1024pix/pix-analytics that referenced this pull request Sep 22, 2020
* ensure new date objects are returned

* return chosen period when comparing with previous year

Co-authored-by: Stefan Giehl <stefan@matomo.org>
jbuget pushed a commit to 1024pix/pix-analytics that referenced this pull request Sep 26, 2020
* ensure new date objects are returned

* return chosen period when comparing with previous year

Co-authored-by: Stefan Giehl <stefan@matomo.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review PRs that need a code review not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Compare to: previous year always returning a range Compare to: previous year (day) goes back two years
4 participants