[#36750] - [4.1] Smart Search: Fix forced Itemid in module
- Fixed in Code Base
- 5 Jun 2022
- Medium
- Build: 4.1-dev
- # 36750
- Diff
- Hackwar:4.2-finder-module
User tests: Successful: Unsuccessful:
Pull Request for Issue #35497.
Summary of Changes
The Smart Search module allows to select which Smart Search filter configuration should be used for this search box by selecting the specific menu item with that configuration. Unfortunately the code contains an issue where the relevant Itemid is duplicated in the output and it even could have conflicting values.
Testing Instructions
I'm copying the instructions by @brianteeman here, since they are perfect for this.
Steps to reproduce the issue PART 1
Create a smart search module. Do not create a search menu item or set an ItemID in the module
image
Then visit the front end and perform a search with the module
Actual result BEFORE applying this Pull Request
The url is example.com/component/finder/search?q=&Itemid=101&Itemid=101
Expected result AFTER applying this Pull Request
The url is example.com/component/finder/search?q=&Itemid=xxx
Steps to reproduce the issue PART 2
Repeat the test but this time add a search menu item or set an ItemID in the module
Then visit the front end and perform a search with the module
Actual result BEFORE applying this Pull Request
The url is example.com/component/finder/search?q=&Itemid=101&Itemid=xxx
where xxx is the id of the search menu item or the id of the menu item selected in the module
where 101 is the id of the default home page
Expected result AFTER applying this Pull Request
The url is example.com/component/finder/search?q=&Itemid=xxx
where xxx is the id of the search menu item or the id of the menu item selected in the module
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Front End com_finder Modules |
| Title |
|
||||||
| Labels |
Added:
?
|
||
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/36750.
| Status | Pending | ⇒ | Ready to Commit |
RTC
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/36750.
| Labels |
Added:
?
|
||
No, just merge it.
If we change the signature of a public method we have to put into 5.0.
Any way to overcome this?
The new argument has a default value so we are covered.
If we change the signature of a public method we have to put into 5.0.
Any way to overcome this?
The new argument has a default value so we are covered.
@bembelimen Are you ok with @laoneo 's reply?
No, it will make problems with PHP 8.1
| Labels |
Added:
?
|
||
| Category | Front End com_finder Modules | ⇒ | Modules Front End |
I'm sorry, but I have an issue with creating a new method that is a duplicate of the other one and will confuse people. I doubt that we can annotate and document this enough so that people aren't going to be totally confused which one to use, etc. It seems to be the wrong way to go.
I now went a different approach, which should be PHP 8.1 compliant and fixes the issue for now. Unfortunately, I fear this means we need new tests for this. Thank you all for your input and time so far.
| Status | Ready to Commit | ⇒ | Pending |
Back to pending.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/36750.
| Labels |
Removed:
?
?
|
||
@brianteeman since the initial report came from you, could you maybe give this a test as well, so that we can move this forward?
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/36750.
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/36750.
| Status | Pending | ⇒ | Ready to Commit |
RTC
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/36750.
| Labels |
Added:
?
|
||
| Status | Ready to Commit | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2022-06-05 19:10:11 |
| Closed_By | ⇒ | bembelimen |
Thx
I have tested this item✅ successfully on 5c59558
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/36750.