codefog / contao-news_categories

Extend the Contao news module with categories
MIT License
31 stars 25 forks source link

Add check get category #258

Open zonky2 opened 8 months ago

zonky2 commented 8 months ago

With the customisation, other hooks can also customise the list.

fritzmg commented 8 months ago

You can also give your Hook a higher priority.

zonky2 commented 8 months ago

You can also give your Hook a higher priority.

I set the other hook (https://github.com/numero2/contao-tags) a priority = 100 - without the PR I see Uncaught PHP Exception Symfony\Component\HttpKernel\Exception\NotFoundHttpException: "Unused arguments: category" at /html/contao4/vendor/contao/core-bundle/src/EventListener/ExceptionConverterListener.php line 97

see https://github.com/numero2/contao-tags/pull/6

fritzmg commented 8 months ago

Yes, your own Hook will then need to process the category. Or what exactly is your goal?

zonky2 commented 8 months ago

There are two different news listings - one list can be filtered by category, the other by tags.

This does not work with the previous implementations.

fritzmg commented 8 months ago

This needs to be fixed differently then. If category filtering is not enabled, then the hook needs to return false.

But you will also have to make sure, that category URLs do not point to the wrong news list.

aschempp commented 8 months ago

does the hook in news-categories need to return false, or some other place?

zonky2 commented 8 months ago

https://github.com/numero2/contao-tags/blob/1f9c9b7c55dc7eaa20ede807239dc50d2f87bfff/src/EventListener/NewsListener.php#L154 and https://github.com/numero2/contao-tags/blob/1f9c9b7c55dc7eaa20ede807239dc50d2f87bfff/src/EventListener/NewsListener.php#L223

fritzmg commented 8 months ago

does the hook in news-categories need to return false, or some other place?

Yes - basically if $criteria = $this->getCriteria() only adds the basic criteria and nothing else then the hooks need to return false. Not sure how this could be done while keeping the same structure. May be add something like NewsCriteria::isOnlyBasicCriteria() and then keep track of that in the other functions?

fritzmg commented 8 months ago

@aschempp @qzminski if you agree, I can try a PR.

aschempp commented 8 months ago

I have no opinion, but feel free to give it a try 😅

fritzmg commented 8 months ago

See #263