Skip to content

[4.4] Fix default parameters for pagination#43954

Closed
Fedik wants to merge 4 commits intojoomla:4.4-devfrom
Fedik:fix-pager-white-list
Closed

[4.4] Fix default parameters for pagination#43954
Fedik wants to merge 4 commits intojoomla:4.4-devfrom
Fedik:fix-pager-white-list

Conversation

@Fedik
Copy link
Member

@Fedik Fedik commented Aug 21, 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

@SniperSister
Copy link
Contributor

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.

@Hackwar
Copy link
Member

Hackwar commented Aug 21, 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?

@SniperSister
Copy link
Contributor

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:

@SniperSister
Copy link
Contributor

Same applies to the other suggested changes

@Hackwar
Copy link
Member

Hackwar commented Aug 21, 2024

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

@SniperSister
Copy link
Contributor

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.

@Hackwar
Copy link
Member

Hackwar commented Aug 21, 2024

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

@Fedik
Copy link
Member Author

Fedik commented Aug 21, 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.

SniperSister added a commit to SniperSister/joomla-cms that referenced this pull request Aug 22, 2024
@Fedik Fedik closed this Aug 22, 2024
@Fedik Fedik deleted the fix-pager-white-list branch August 22, 2024 07:33
MacJoom pushed a commit that referenced this pull request Aug 22, 2024
…#43953)

* fix com_finder pagination

* Codestyle

* cs fix

* Update components/com_finder/src/View/Search/HtmlView.php

Co-authored-by: Quy <quy@nomonkeybiz.com>

* Append missing catid parameter in archive view

* also cover title search

* add additional parameters from PR #43954

* update ID filter

* PHPCS

---------

Co-authored-by: Hannes Papenberg <info@joomlager.de>
Co-authored-by: Quy <quy@nomonkeybiz.com>
Co-authored-by: Richard Fath <richard67@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants