[6.1] [webservices] use logic and when filter multiple tag values#46325
[6.1] [webservices] use logic and when filter multiple tag values#46325alikon wants to merge 15 commits intojoomla:6.1-devfrom
Conversation
|
I tried to test this PR but got a 500 error response. I am an API novice so it could be something to do with my set up. This is my stack trace: {"errors":[{"code":500,"title":"Internal server error","detail":"RuntimeException: Unable to load renderer class module in /Users/ceford/Sites/joomla-cms6/libraries/src/Document/Factory.php:101\nStack trace:\n This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/46325. |
|
do you have some |
|
I have tested this item ✅ successfully on 3642879 Request: or
2 expected items Response: Request: 0 expected items Response: This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/46325. |
|
I have tested this item ✅ successfully on 3642879 With the PR applied, with: I got: { as expected based on my tags distribution on articles. SHOULD I get articles, 16, 6, 5, 4, 3, and 1 to show up EVEN THOUGH I have UNPUBLISHED the Tags in the backend? IF I am NOT supposed to have them listed, then the PR fails, if I am supposed to have them listed, then it passes. This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/46325. |
|
I have tested this item ✅ successfully on 3642879 This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/46325. |
|
RTC This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/46325. |
|
I have tested this item ✅ successfully on 3642879 This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/46325. |
|
I have tested this item ✅ successfully on 3642879 This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/46325. |
|
@alikon Your PR changes filtering in the articles list in the backend model, but your testing instructions only test the new functionality of the API, they do not covert to test that filtering in the articles list in backend works as well as before, including the filtering for articles which do not have any tags assigned (tags filer "None"). So in my opinion your testing instructions are incomplete. @Razzo1987 @exlemor @TerryHarker @jikubiswal @KishoriBKarale Has anyone of you tested that filtering in the articles list in backend is not broken so it works as well as before, also when filtering for articles which do not have any tags assigned (tags filer "None")? |
administrator/components/com_content/src/Model/ArticlesModel.php
Outdated
Show resolved
Hide resolved
administrator/components/com_content/src/Model/ArticlesModel.php
Outdated
Show resolved
Hide resolved
administrator/components/com_content/src/Model/ArticlesModel.php
Outdated
Show resolved
Hide resolved
administrator/components/com_content/src/Model/ArticlesModel.php
Outdated
Show resolved
Hide resolved
administrator/components/com_content/src/Model/ArticlesModel.php
Outdated
Show resolved
Hide resolved
richard67
left a comment
There was a problem hiding this comment.
Please apply my change suggestions to revert unrelated and unnecessary and partly wrong code style changes.
Co-authored-by: Richard Fath <richard67@users.noreply.github.com>
Co-authored-by: Richard Fath <richard67@users.noreply.github.com>
Co-authored-by: Richard Fath <richard67@users.noreply.github.com>
Co-authored-by: Richard Fath <richard67@users.noreply.github.com>
Co-authored-by: Richard Fath <richard67@users.noreply.github.com>
|
I have no idea why integration and unit tests are failing for Windows at the PHP installation, but it is not related to this PR. |
|
@alikon Please check the one remaining code style suggestion I made above: #46325 (comment) . Thanks in advance. P.S.: Human tests and so RTC would still be valid as all changes after that were only code style changes. |
| ->select('DISTINCT ' . $db->quoteName('content_item_id')) | ||
| ->from($db->quoteName('#__contentitem_tag_map')) | ||
| ->where([ | ||
| $db->quoteName('tag_id') . ' = ' . $db->quote($tagId), |
There was a problem hiding this comment.
why don't you use prepared parameters here? You only need to bind the parameter to the $query object with bindArray() and use the result or by normal bind()
Pull Request for Issue #46241 .
Summary of Changes
This change adds support for a tag_mode filter to the Articles list in com_content so callers can choose whether tag filtering should use OR (any) or AND (all) semantics. It also exposes the new filter via the API controller.
What this does
Testing Instructions
GET
api/index.php/v1/content/articles?filter[tag][]=2&filter[tag][]=3&filter[tag_mode]=allManual tests performed for:
Actual result BEFORE applying this Pull Request
only logic OR
Expected result AFTER applying this Pull Request
Adds logic AND
Link to documentations
Please select:
Documentation link for docs.joomla.org:
No documentation changes for docs.joomla.org needed
Pull Request link for manual.joomla.org:
No documentation changes for manual.joomla.org needed