joomla-cms icon indicating copy to clipboard operation
joomla-cms copied to clipboard

[4] Tagged Items Menu Item When Both Public and Registered Tags 404

Open alikon opened this issue 1 year ago • 8 comments

Pull Request for Issue #43920 .

Summary of Changes

removed unneeded check

Testing Instructions

  • Create a tag with access level Public
  • Create a tag with access level Registered
  • Assign public tag to an article with the access level Public
  • Assign registered tag to an article with the access level Registered
  • Create Menu Item Tags > Tagged Items
  • Access the frontend - do not login

Actual result BEFORE applying this Pull Request

404 Tag not found

Expected result AFTER applying this Pull Request

List with the public article

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

alikon avatar Aug 15 '24 07:08 alikon

This PR works BUT maybe there are other consequences of this changes as it was introduced by @Hackwar with #39114

In the tag model, the $item attribute wasn't defined. $tag at the same time wasn't used. With $item being initialised as an array, we can remove a bunch of checks. At the same time we can now properly throw a 404 if the number of tag IDs doesn't match the number of tags found in the database matching these IDs. (we might want to rewrite the TagModel::getItem() method to use a single query instead of loading each tag separately with a Table call...)

brianteeman avatar Aug 15 '24 07:08 brianteeman

I have tested this item :white_check_mark: successfully on 798cbdec30de31167816e21a86cecf7914835d4d

Tested with Joomla 4.4.7 PHP 8.1


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

MacJoom avatar Aug 24 '24 08:08 MacJoom

I have tested this item :white_check_mark: successfully on 798cbdec30de31167816e21a86cecf7914835d4d

Does exactly as it should displaying only the public tag if not logged in and both if logged in


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

softforge avatar Aug 24 '24 09:08 softforge

I have tested this item :white_check_mark: successfully on 798cbdec30de31167816e21a86cecf7914835d4d

Tested with J4.4.7 and PHP 8.3.6


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

dautrich avatar Aug 24 '24 09:08 dautrich

@softforge was faster than I was!

dautrich avatar Aug 24 '24 09:08 dautrich

RTC


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

richard67 avatar Aug 24 '24 15:08 richard67

As @brianteeman already stated: I'm not sure that this is correct. The behavior of tags was never defined and is a big mess of different and competing concepts. Should it display all items which only match one tag or all tags? If a tag is not available to the current user, should the view display all items which match the other tag or would we still expect a filtered list with less items?

Hackwar avatar Aug 25 '24 20:08 Hackwar

ok tags are a big mess, we can all agree

this pr fix a "common sense" behavior not more not less

alikon avatar Aug 26 '24 16:08 alikon

This really needs to be fixed, so I merged it. Thanks for the contribution.

laoneo avatar Sep 12 '24 12:09 laoneo