Thanks Max :)

Summary of change adding a filtering option to include articles by their IDs to the mod_articles_category module

Testing Instructions
In mod_articles_category, replace the existing files with the files in the ZIP archive attached
mod_articles_category.zip

Actual result BEFORE applying this Pull Request
The current version of the module (3.0.0) doesnt allow to include articles by their IDs

Expected result AFTER applying this Pull Request
In the filtering options, you should have a new option to include articles by their IDs
Add some IDs there and check the output to control the module displays the articles you've indicated.

looks like this was made from an old version of the module

It's from module 3.0.0 version

If you look at the changes https://github.com/joomla/joomla-cms/pull/41745/files you can see all the code that is being removed by this pull request as well as the changes. I would expect to ONLY be seeing code added.

Sorry, I don't get you (again).

I've edited the files of the module currently installed in a J4.
I've worked on mod_articles_category 3.0.0 which is the latest version.
I've provided the new files with my changes to make it simplier to test and validate.
I didn't submitted the PR, as I do not have the rights to.
If needed, I can provide you only the lines I've changed. Is that you ask/want/need?

I understand you're very familiar with but to me, GitHub is a complete strange and scary universe. I don't ask for a favor, just some understanding if I don't reply correctly or as expected.

@web-eau-net Daniel github shows all the changes in this pull request. You can see them if you follow the link I gave before https://github.com/joomla/joomla-cms/pull/41745/files The original is on the left and the changes from this PR are on the right

Everything in "pink" is being removed and Everything in "green" is being added. You dont need to understand the code to realise that thiss pull request is removing a lot of code when it should really be adding a small bit of code.

Thanks for your explanations Brian, really appreciate your time and efforts.

I do understand the adding/removed color codes you've mentioned.
I don't know where the differences come from but as I've worked on the latest version of the module, we should have the same code in the PR (or shouldn't have any differences except my changes in green)?

What should I do now with my limited GitHub account to make this PR happen?

As I did the PR in helping Daniel, I asked him if the 3 files will replace the ones already in.
I just generated a new branch, based on the 5.0-dev one, and added the new files from Daniel.
If I did something wrong, pls tell me

See #41746 - easier to show you than explain

Thank you Brian

Basically you have taken code from 4.x edited and put it into 5.0
BUT the code in 5.0 was already chanaged so you were replacing those changes

ps - your github account is no different to mine

I've understood where was the confusion.
I've applied the same changes in the J5 version of the module files and @max123kl will submit it.

I think a change is needed:

Without the patch I can put some blank space in the Article IDs to Exclude and that does not prevent all articles in the category appearing in the frontend module. With the patch applied, any blank space only in the Article IDs to Include causes the module to disappear in the front end. I think you need to check that Article IDs to Include does not include only white space and perhaps check for valid numbers. It is a pity that there is no field descriptor saying 'One per line'.

@ceford I got your point.
Attached a ZIP archived with a new version of the XML with an explicite tooltip description for these 2 fields.

mod_articles_category_xml.zip

@web-eau-net @max123kl needs to include any changes in the PR. What about the problem with accidental white space in the Articles to include field? I can just see the questions in the Forum now - my mod_articles_category module has disappeared.

Honestly @ceford I have absolutly NO idea about this issue :/
I've simply copied/pasted the code for articles to exclude and turned into articles to include (my coding knowledges are that point but no more). For the others questions, I can't help at all.
Sure that @max123kl will do the job for including the changes.
Thanks for your help here.

@web-eau-net & @ceford
we shall discuss the best way to optimise the PR via DM on mattermost and then commit the solution

Maybe there is a misunderstanding of how the module works.
It first makes a list of all categories which match the filter.
Then it makes a list of all articles in these categories.
Now it is easy to exclude some articles from this list.

But you cannot include articles without another search in the database for these articles. And if they are not in the specified categories, the name of the module mod_articles_category would be wrong.

@web-eau-net, are you sure that this worked in a former version of the module?

@chmst I use the current version of the module (J4 3.0.0) with this modification and without any issue.
Of course, I've indicated a category because I didn't had the idea to use the module without this parameter and I would say I use mod_articles_category like I always did.

Maybe I'm wrong but to exclude the articles with the module, nobody need a documentation. Why would it be different with including?

Okay... we can use the description tooltip id needed. It would be the easiest way to inform the user to select at last one category before excluding or including any article by their IDs.

This pull request has been automatically rebased to 5.1-dev.

I have tested this item ✅ successfully on 07bc613

Nice work!!

I have tested this issue successfully. I have found that if you add the same ID in exclude and include options, include option prevails.

I have tested this item ✅ successfully on 07bc613

RTC

Please fix the cs error

Even once the CS error is fixed this should not be merged until @richard67 's comment is addressed

Even once the CS error is fixed this should not be merged until @richard67 's comment is addressed

@wilsonge I just see that the already existing exclude list works the same way, so for me it's ok, and I've marked my review comment as resolved.

to me it seems not a core feature

@web-eau-net i have adapted it as we discussed. now there is an inclusive/exclusive filter switch and only the original input field for the article ids.

Would appreciate your feedback after testing.

Module tested ✔️

Module tested ✔️

@web-eau-net Was that a successful test of this PR? If so, please go to the issue tracker and mark your test result so it's properly counted. Go to https://issues.joomla.org/tracker/joomla-cms/41745 and use the blue "Test this" button at the top left corner, select your test result and submit. Thanks in advance.

I have tested this item ✅ successfully on 233e5ec

Thanks @richard67 , done :)

this pr is a b/c break and can't be merged.

This pull request has been automatically rebased to 5.2-dev.

This PR has a longer story so I will explain the not clear points.

This is a b/c break because of a change of the parameter name. If you have an existing site with excluded articles that these articles will be shown after the update. That is something we cann fix but I wouldn't do it, instead I would close this one and concentrate on #43738 who should have included this function into the new article module.

Thanks to all who have worked on this one.