Feature ? PR-5.1-dev Pending

User tests: Successful: Unsuccessful:

avatar brianteeman
brianteeman
16 Oct 2023

Summary of Changes

Remove unnecessary duplicated condition

In the method to get the database query in administrator\components\com_installer\src\Model\UpdateModel.php a query is constructed that has a duplicated condition where($db->quoteName('u.extension_id') . ' != 0'

It is first added

->where($db->quoteName('u.extension_id') . ' != 0');

and then added again

$query->where($db->quoteName('u.extension_id') . ' != 0')

Testing Instructions

Install an old extension and check for updates with debug on to see the queries

Actual result BEFORE applying this Pull Request

Query 1

SELECT u.*,`e`.`manifest_cache`
FROM `euj03_updates` AS `u`
LEFT JOIN `euj03_extensions` AS `e` ON `e`.`extension_id` = `u`.`extension_id`
WHERE `u`.`extension_id` != 0 AND `u`.`extension_id` != 0 AND `u`.`extension_id` != :eid
ORDER BY `u`.`name` ASC LIMIT 25

Query 2

SELECT COUNT(*)
FROM `euj03_updates` AS `u`
LEFT JOIN `euj03_extensions` AS `e` ON `e`.`extension_id` = `u`.`extension_id`
WHERE `u`.`extension_id` != 0 AND `u`.`extension_id` != 0 AND `u`.`extension_id` != :eid

Expected result AFTER applying this Pull Request

Query 1

SELECT u.*,`e`.`manifest_cache`
FROM `euj03_updates` AS `u`
LEFT JOIN `euj03_extensions` AS `e` ON `e`.`extension_id` = `u`.`extension_id`
WHERE `u`.`extension_id` != 0 AND `u`.`extension_id` != :eid
ORDER BY `u`.`name` ASC LIMIT 25

Query 2

SELECT COUNT(*)
FROM `euj03_updates` AS `u`
LEFT JOIN `euj03_extensions` AS `e` ON `e`.`extension_id` = `u`.`extension_id`
WHERE `u`.`extension_id` != 0 AND `u`.`extension_id` != :eid

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

avatar brianteeman brianteeman - open - 16 Oct 2023
avatar brianteeman brianteeman - change - 16 Oct 2023
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 16 Oct 2023
Category Administration com_installer
avatar HLeithner HLeithner - change - 16 Oct 2023
Title
[5.0] com_installer getListQuery
[5.1] com_installer getListQuery
avatar HLeithner HLeithner - edited - 16 Oct 2023
avatar HLeithner
HLeithner - comment - 16 Oct 2023

I rebased it to 5.1 since it's not a bugfix.

avatar brianteeman
brianteeman - comment - 16 Oct 2023

What is it then if its not a bug?

avatar rdeutz
rdeutz - comment - 16 Oct 2023

your change doesn't change the query result, so it is a optimisation and not a bug fix

avatar brianteeman
brianteeman - comment - 16 Oct 2023

your logic escapes me

avatar richard67
richard67 - comment - 16 Oct 2023

The first "where" which is removed by this PR is added when there is an $extensionId = $this->getState('filter.extension_id'); given, and the 2nd one later below when there is a search with prefix eid: done in the text search field.

So to be sure that it breaks nothing it would need to test all cases, list view without any filter, with filter by extension ID in the filter tools but no search in the search box, then with search in the search box without any filter by extension ID in the filter tools, and finally with a combination of both, the filter tools and the search box. And for the search box we have 2 cases, with eid: and without eid:.

The testing instructions do not mention any of these cases.

avatar brianteeman
brianteeman - comment - 16 Oct 2023

The first condition is always applied. The second condition is conditionally applied. So you either get the condition once or twice. You can never have a situation where the condition is not applied. This PR is removing the second condition not the first

avatar richard67
richard67 - comment - 16 Oct 2023

The first condition is always applied. The second condition is conditionally applied. So you either get the condition once or twice. You can never have a situation where the condition is not applied. This PR is removing the second condition not the first

I see now. You are right. By review this PR seems right to me.

avatar Quy Quy - test_item - 19 Oct 2023 - Tested successfully
avatar Quy
Quy - comment - 19 Oct 2023

I have tested this item ✅ successfully on c5f096a

Nice catch!


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

avatar alikon alikon - test_item - 20 Oct 2023 - Tested successfully
avatar alikon
alikon - comment - 20 Oct 2023

I have tested this item ✅ successfully on c5f096a


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

avatar alikon alikon - change - 20 Oct 2023
Status Pending Ready to Commit
avatar alikon
alikon - comment - 20 Oct 2023

RTC


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

avatar brianteeman
brianteeman - comment - 24 Oct 2023

On what planet is this a feature?

avatar rdeutz rdeutz - change - 24 Oct 2023
Labels Added: Feature ? PR-5.1-dev
avatar HLeithner HLeithner - close - 24 Oct 2023
avatar HLeithner HLeithner - merge - 24 Oct 2023
avatar HLeithner HLeithner - change - 24 Oct 2023
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2023-10-24 09:12:12
Closed_By HLeithner

Add a Comment

Login with GitHub to post a comment