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

DiscussionModel BeforeGetCount event not fired #2222

Closed
bleistivt opened this issue Nov 3, 2014 · 4 comments
Closed

DiscussionModel BeforeGetCount event not fired #2222

bleistivt opened this issue Nov 3, 2014 · 4 comments
Assignees
Milestone

Comments

@bleistivt
Copy link
Contributor

When GetCount(); is called without arguments (like in DiscussionsController->Index) the counts are calculated from the CountDiscussions cache of the categories, rendering the BeforeGetCount event useless:

if (!$Wheres || (count($Wheres) == 1 && isset($Wheres['d.CategoryID']))) {
// Grab the counts from the faster category cache.
if (isset($Wheres['d.CategoryID'])) {
$CategoryIDs = (array)$Wheres['d.CategoryID'];
if ($Perms === FALSE)
$CategoryIDs = array();
elseif (is_array($Perms))
$CategoryIDs = array_intersect($CategoryIDs, $Perms);
if (count($CategoryIDs) == 0) {
return 0;
} else {
$Perms = $CategoryIDs;
}
}

@linc linc added the Bug label Feb 14, 2015
@linc linc removed the Bug label Sep 26, 2015
@linc
Copy link
Contributor

linc commented Aug 29, 2016

Are you requesting an earlier hook to use? I'm reluctant to move an existing hook, and I don't wanna make a new one for no reason.

@linc linc added the Status: Awaiting Feedback This issue or pull request is awaiting feedback from someone. DON'T MERGE! label Aug 29, 2016
@bleistivt
Copy link
Contributor Author

The problem is that there is no way to modify the discussion counts in some cases.

You can always modify the discussions fetched using beforeGet, but beforeGetCount sometimes just doesn't work, which is inconsistent.
This will also lead to a faulty pager showing a wrong number of pages.

The $Wheres eventarg is not used by the non-cache logic. Thus moving the hook above the hightlighted piece of code I linked to should be safe.

I'll be happy to submit a pull request.

@linc linc removed the Status: Awaiting Feedback This issue or pull request is awaiting feedback from someone. DON'T MERGE! label Sep 12, 2016
@linc
Copy link
Contributor

linc commented Sep 12, 2016

A PR is always best. 👍

@DaazKu
Copy link
Contributor

DaazKu commented Feb 10, 2017

PR is under review.

@DaazKu DaazKu closed this as completed Feb 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants