User tests: Successful: Unsuccessful:
Pull Request for Issue # .
Added a few missing but required parameters.
And replaced use of deprecated app->input to modern.
code review
Please select:
ping @SniperSister
Status | New | ⇒ | Pending |
Category | ⇒ | Libraries |
Labels |
Added:
Release Blocker
bug
PR-4.4-dev
|
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?
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:
joomla-cms/libraries/src/Cache/Cache.php
Line 690 in 8a868a3
Same applies to the other suggested changes
Do we really need to have the cache key parameter be CMD as well? I currently don't see the necessity for that...
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.
But having it be INT is far more restrictive than CMD and we only need the CMD for the router...
Labels |
Removed:
Release Blocker
|
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.
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2024-08-22 07:33:18 |
Closed_By | ⇒ | Fedik |
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.