Skip to content

[4.4] Fix parameter type for whereIn query in ExtensionsHelper - issue 42637#42638

Merged
MacJoom merged 2 commits intojoomla:4.4-devfrom
richard67:4.4-dev-fix-extensions-helper-where-in-data-type
Jan 10, 2024
Merged

[4.4] Fix parameter type for whereIn query in ExtensionsHelper - issue 42637#42638
MacJoom merged 2 commits intojoomla:4.4-devfrom
richard67:4.4-dev-fix-extensions-helper-where-in-data-type

Conversation

@richard67
Copy link
Member

@richard67 richard67 commented Jan 10, 2024

Pull Request for Issue #42637 .

Summary of Changes

This pull request (PR) adds the missing $dataType parameter to the call of $query->whereIn for the query which has been modified with PR #42576 , which caused a regression in Joomla 4.4.2 and 5.0.2 regarding filtering for core or non-core extensions in the Extensions Manager.

The default value of that parameter is ParameterType::INTEGER, so the missing parameter in the call caused the strings to be bound as integers. Depending on the database type and version, this may cause the regression or it may not cause it when the database does an implicit cast to string. This might be the reason why I haven't noticed my mistake when testing my PR #42576 .

Testing Instructions

Pre-conditions

This PR should be tested on a current 4.4-dev branch or on a 4.4.2. Do not use 4.4.1 or older because with that you will not be able to reproduce the issue.

If you can test only with Joomla 5, use a current 5.0-dev branch or a 5.0.2. You can apply the same change from this PR here also on a 5.0-dev branch or a 5.0.2 by manual edit of the modified file. Do not use 5.0.1 or 5.0.0 because with that you will not be able to reproduce the issue.

It might also depend on the database type (MySQL or MariaDB) and the database version if you can reproduce the issue or not.

Procedure

Go to extensions manage and filter by core/non-core extensions.

Actual result BEFORE applying this Pull Request

Non-core filter produces zero results.

Core filter produces results for the non-core extensions only.

Expected result AFTER applying this Pull Request

Filtering works as expected.

Link to documentations

Please select:

  • No documentation changes for docs.joomla.org needed

  • No documentation changes for manual.joomla.org needed

@brianteeman
Copy link
Contributor

tested with 4.4.2


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

@brianteeman
Copy link
Contributor

I have tested this item ✅ successfully on 6897cc0


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

@alikon
Copy link
Contributor

alikon commented Jan 10, 2024

i'm unable to replicate the current issues, even if i was able to replicate the original issue solved by PR #42576
btw this pr it's ok for me too

@ot2sen
Copy link
Contributor

ot2sen commented Jan 10, 2024

I have tested this item ✅ successfully on 6897cc0

Tested on J4.4.2

Works after applying patch manually.

PS: Zero results before for non-core and all results in core (non+core). After it shows non-core at non-core and core at core filter.


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

@alikon
Copy link
Contributor

alikon commented Jan 10, 2024

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jan 10, 2024
@MacJoom MacJoom self-assigned this Jan 10, 2024
@MacJoom MacJoom added this to the Joomla 4.4.3 milestone Jan 10, 2024
@MacJoom MacJoom merged commit 59eeaba into joomla:4.4-dev Jan 10, 2024
@MacJoom
Copy link
Contributor

MacJoom commented Jan 10, 2024

Thank you!

@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jan 10, 2024
@richard67
Copy link
Member Author

Thanks all testers and for merging.

@richard67 richard67 deleted the 4.4-dev-fix-extensions-helper-where-in-data-type branch January 10, 2024 18:48
@some1new
Copy link

some1new commented Jan 21, 2024

After manual installation on 4.4.2 says still no ext installed and all checkmarks green although several third developer ext. exist.

@richard67
Copy link
Member Author

richard67 commented Jan 21, 2024

After manual installation on 4.4.2 says still no ext installed and all checkmarks green although several third developer ext. exist.

@some1new You might have to trigger the pre-update check again by using the button to search for updates, and make sure that you really have applied the change. You can e.g. download the changed file from here https://raw.githubusercontent.com/joomla/joomla-cms/4.4-dev/libraries/src/Extension/ExtensionHelper.php and then upload it to your site into the "libraries/src/Extension" folder.

@some1new
Copy link

After manual installation on 4.4.2 says still no ext installed and all checkmarks green although several third developer ext. exist.

@some1new You might have to trigger the pre-update check again by using the button to search for updates, and make sure that you really have applied the change. You can e.g. download the changed file from here https://raw.githubusercontent.com/joomla/joomla-cms/4.4-dev/libraries/src/Extension/ExtensionHelper.php and then upload it to your site into the "libraries/src/Extension" folder.

My fault, updated a different website to check.
All fine.

Copy link

@nsheehan nsheehan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not see anything close to this in the 4.4.2 version of this file. Please publish a fix.

@laoneo
Copy link
Member

laoneo commented Jan 30, 2024

This pr will be published with 4.4.3 as you can see in the target milestone.

@richard67
Copy link
Member Author

As intermediate solution you can apply the changes from this pull request manually.

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.

10 participants

Comments