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

fix no data message may not be shown when deleteLogs is enabled #14526

Merged
merged 3 commits into from Jun 17, 2019

Conversation

tsteur
Copy link
Member

@tsteur tsteur commented Jun 14, 2019

  • Fixes DEV-1671
  • Also fixes SitesManager::hasTrackedAnyTraffic($idSite) was returning inverted result. Meaning it was returning true when no traffic was tracked, but the method suggested it should return false in this case.

@tsteur tsteur added the Needs Review PRs that need a code review label Jun 14, 2019
@tsteur tsteur added this to the 3.10.0 milestone Jun 14, 2019
return;
}

if (self::hasTrackedAnyTraffic($siteId)) {
if (!self::hasTrackedAnyTraffic($siteId)) {
Copy link
Member

Choose a reason for hiding this comment

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

delete_logs_enable can be turned on/off, correct? Should we set the above option here too?

Copy link
Member

Choose a reason for hiding this comment

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

in an 'else' I mean

Copy link
Member Author

Choose a reason for hiding this comment

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

👍 pushing it now. That should make it a lot simpler

@diosmosis
Copy link
Member

One more comment: is there a way to make this change visible in tests?

@diosmosis
Copy link
Member

Going to add some tests myself then merge

@diosmosis diosmosis merged commit ed26709 into 3.x-dev Jun 17, 2019
@diosmosis diosmosis deleted the dev-1671 branch June 17, 2019 04:42
@tsteur
Copy link
Member Author

tsteur commented Jun 17, 2019

Thank you very much @diosmosis

@mattab mattab added the Bug For errors / faults / flaws / inconsistencies etc. label Jun 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For errors / faults / flaws / inconsistencies etc. Needs Review PRs that need a code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants