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

Fixed error in FocusModel where clicks and submissions were for all f… #5257

Merged
merged 2 commits into from
Dec 21, 2017

Conversation

maltonite
Copy link
Contributor

…ocus, not just the one selected

Q A
Bug fix? Y
New feature?
Automated tests included?
Related user documentation PR URL
Related developer documentation PR URL
Issues addressed (#s or URLs) #5175
BC breaks?
Deprecations?

Description: Focus stats were showing global clicks/submissions, not stats for just the viewed focus item.

Steps to reproduce the bug:

  1. Create multiple focus items. Create submissions/ clicks for each.
  2. Note that the clicks/ submissions will be the same for both

Steps to test this PR:

  1. Repeat steps above for producing bug and note that the problem has been solved.

List deprecations along with the new alternative:

List backwards compatibility breaks:

@kuzmany
Copy link
Member

kuzmany commented Nov 1, 2017

Works for me 👍
Year ago I already fixed this issue. But now is back :)

@@ -411,14 +411,14 @@ public function getStats(Focus $focus, $unit, \DateTime $dateFrom = null, \DateT

if ($focus->getType() != 'notification') {
if ($focus->getType() == 'link') {
$q = $query->prepareTimeDataQuery('focus_stats', 'date_added', ['type' => Stat::TYPE_CLICK]);
$q = $query->prepareTimeDataQuery('focus_stats', 'date_added', ['type' => Stat::TYPE_CLICK,'focus_id' => $focus->getId()]);
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Fix the code standards, please.

https://travis-ci.org/mautic/mautic/jobs/295299190#L1340

There should be a space after the comma.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Have added the space.

@escopecz escopecz added bug Issues or PR's relating to bugs pending-feedback PR's and issues that are awaiting feedback from the author labels Nov 9, 2017
@escopecz escopecz added ready-to-test PR's that are ready to test and removed pending-feedback PR's and issues that are awaiting feedback from the author labels Nov 9, 2017
@alanhartless alanhartless added this to the 2.12.1 milestone Dec 13, 2017
@alanhartless alanhartless added the pending-test-confirmation PR's that require one test before they can be merged label Dec 19, 2017
@javjim javjim self-assigned this Dec 19, 2017
@javjim
Copy link

javjim commented Dec 21, 2017

Fix works properly

@javjim javjim added ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged and removed pending-test-confirmation PR's that require one test before they can be merged ready-to-test PR's that are ready to test labels Dec 21, 2017
@javjim javjim removed their assignment Dec 21, 2017
@escopecz escopecz merged commit b66f92c into mautic:staging Dec 21, 2017
@dbhurley dbhurley removed the ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged label Dec 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issues or PR's relating to bugs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants