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

Add trigger to com_content front category model #4656

Closed
wants to merge 2 commits into from
Closed

Add trigger to com_content front category model #4656

wants to merge 2 commits into from

Conversation

Arkadiy-Sedelnikov
Copy link
Contributor

Trigger a need for content filtering.

@zikkuratvk
Copy link

Good innovation. Allows you to expand the use of com_content.

@Bakual
Copy link
Contributor

Bakual commented Oct 15, 2014

Please have a look at #4308 which tries to unify plugin triggers.
Maybe by coordinate you can achieve more 😄

@Arkadiy-Sedelnikov Arkadiy-Sedelnikov changed the title Add trigger to com_content category model Add trigger to com_content front category model Oct 15, 2014
@Arkadiy-Sedelnikov
Copy link
Contributor Author

See "MiniCCK Filter" on this page http://demoj3.argens.ru/index.php/blog
Its work with this trigger. Good or bad?

@roland-d
Copy link
Contributor

Hello @Arkadiy-Sedelnikov

Thank you for your contribution.

The last comment here was on 25th November. So the question is, Is this issue/pull request still valid? Does the PR linked by Bakual solve the issue?

If no reply is received within 4 weeks we will close this issue.

Thanks for understanding!


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/4656.

@Arkadiy-Sedelnikov
Copy link
Contributor Author

I already solved the problem of the absence of the trigger in my extension substitution model. All it'd be better if the trigger was in the home model, but the urgent need for this anymore.

@@ -239,6 +239,12 @@ function getItems()
$model->setState('filter.max_category_levels', $this->getState('filter.max_category_levels'));
$model->setState('list.links', $this->getState('list.links'));

$dispatcher = JEventDispatcher::getInstance();
// Include the content plugins for the change of category state event
Copy link
Contributor

Choose a reason for hiding this comment

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

Please fix the comment as it is not a change of category state event.

@mbabker
Copy link
Contributor

mbabker commented Aug 21, 2015

I'm confused on what exactly this is supposed to be doing.

@fancyFranci
Copy link
Contributor

Testing the trigger was successful. The trigger calls the function "onGetContentItems" for a content plugin like "contact".

@Arkadiy-Sedelnikov
Copy link
Contributor Author

See http://demoj3.argens.ru/index.php/blog
Module "MiniCCK Filter" working on this trigger.

@mbabker
Copy link
Contributor

mbabker commented Aug 21, 2015

I get that the trigger is added and running. What I don't get is why it's being added and what the purpose is for this very specific use case. Generally, we do not have event triggers to manipulate an object (model or database query typically) before a data query is executed; this changes that behavior for one very exclusive use case. Right now this PR looks to me like a "I need this feature for my extension so I'm going to add it without anymore detail" situation and my inclination is to suggest it be closed because of that (looks to be something for an edge case in an extension and no clarifying explanation about why this is proposed). IMO, this PR needs to be better explained; why is this event trigger needed for only this one use case out of all the components shipped with the Joomla core.

@Arkadiy-Sedelnikov
Copy link
Contributor Author

I wrote that for my extension this PR is not mandatory, I already made a spoof of this model with a built-in trigger. The trigger I use for content filtering ин additional fields on my CCK, so it can use other CCK parasitic on com_content.

@Bakual
Copy link
Contributor

Bakual commented Aug 23, 2015

I'm closing this PR as in the current state it will not get merged. To many questions are open.

If we want to introduce this new plugin event, it should better be placed in a library model somewhere so it's triggered for all extensions, not a single one.

@Bakual Bakual closed this Aug 23, 2015
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

Successfully merging this pull request may close these issues.

None yet

7 participants