bug PR-4.4-dev Pending

User tests: Successful: Unsuccessful:

avatar Fedik
Fedik
21 Aug 2024

Pull Request for Issue # .

Summary of Changes

Added a few missing but required parameters.
And replaced use of deprecated app->input to modern.

Testing Instructions

code review

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

ping @SniperSister

avatar Fedik Fedik - open - 21 Aug 2024
avatar Fedik Fedik - change - 21 Aug 2024
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 21 Aug 2024
Category Libraries
avatar Fedik Fedik - change - 21 Aug 2024
Labels Added: Release Blocker bug PR-4.4-dev
avatar Fedik Fedik - change - 21 Aug 2024
The description was changed
avatar Fedik Fedik - edited - 21 Aug 2024
avatar SniperSister SniperSister - test_item - 21 Aug 2024 - Tested successfully
avatar SniperSister
SniperSister - comment - 21 Aug 2024

I have tested this item ✅ successfully on 43605d4


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

avatar Hackwar
Hackwar - comment - 21 Aug 2024

You are filtering by CMD in most cases. Is that correct? And for id we are filtering by INT, but our id is actually a string with the alias at the end. Yes, the core does handle that, but not necessarily third party extensions. Also, should we have to add catid here as well?

avatar SniperSister
SniperSister - comment - 21 Aug 2024

Regarding the ID parameter:
If you want to change the INT into a STRING or CMD, make sure to apply the same change to the component cache key:

avatar SniperSister
SniperSister - comment - 21 Aug 2024

Same applies to the other suggested changes

avatar Hackwar
Hackwar - comment - 21 Aug 2024

Do we really need to have the cache key parameter be CMD as well? I currently don't see the necessity for that...

avatar SniperSister
SniperSister - comment - 21 Aug 2024

Do we really need to have the cache key parameter be CMD as well? I currently don't see the necessity for that...

Yes we do, otherwise we re-introduce the issue again.

avatar Hackwar
Hackwar - comment - 21 Aug 2024

But having it be INT is far more restrictive than CMD and we only need the CMD for the router...

avatar Fedik Fedik - change - 21 Aug 2024
Labels Removed: Release Blocker
avatar Fedik
Fedik - comment - 21 Aug 2024

If you want to change the INT into a STRING or CMD,

Should I? I am fine with integer.
hm, but is there still use cases like &id=1:foobar-alias?

You are filtering by CMD in most cases. Is that correct? And for id we are filtering by INT

I did not change filter for the ID.
I only change where was "WORD" filter, I checked the CMS code, and in most cases there is "CMD"
IDK if the ID realy need in pagination. But I can change to anything.

avatar Fedik Fedik - close - 22 Aug 2024
avatar Fedik Fedik - change - 22 Aug 2024
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2024-08-22 07:33:18
Closed_By Fedik

Add a Comment

Login with GitHub to post a comment