-
Notifications
You must be signed in to change notification settings - Fork 6
Support for Singletons #31
base: master
Are you sure you want to change the base?
Conversation
Hi @magnumrocha , thanks for your PR! That's a great addition that never had the time to implement. I'm not updating this addon for a long time and therefore I'll need some time to review and merge the PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In overall seems doing the job, the comments are all very minor, but keep in mind I'm not actively looking to this addon (it was probably some years last time looked into the code).
@@ -1,4 +1,4 @@ | |||
<?php | |||
<?php |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extra space?
if (!isset($collections[$data['_collection']])) { | ||
$collection = $this->app->module('collections')->collection($data['_collection']); | ||
$collections[$data['_collection']] = $collection; | ||
foreach ($results['active'] as $result) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably could we decouple better singletons from collections functionality, and reduce the size of the run function
@@ -34,7 +34,7 @@ | |||
$options['fields'][$field_name] = 1; | |||
} | |||
|
|||
$app->trigger("moderation.find.before", [$name, &$options]); | |||
$app->trigger("moderation.collections.find.before", [$name, &$options]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is a breaking change, would be possible to maintain the moderation.find.before
hook and then call the functionality depending on what we have there? its fine on having the new hooks, just concerned on removing the existing one as any addons that rely on it will break
Add Moderation support for Singletons registers