User tests: Successful: Unsuccessful:
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
and then added again
Install an old extension and check for updates with debug on to see the queries
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
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
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
Status | New | ⇒ | Pending |
Category | ⇒ | Administration com_installer |
Title |
|
What is it then if its not a bug?
your change doesn't change the query result, so it is a optimisation and not a bug fix
your logic escapes me
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.
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
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.
I have tested this item ✅ successfully on c5f096a
Nice catch!
I have tested this item ✅ successfully on c5f096a
Status | Pending | ⇒ | Ready to Commit |
RTC
On what planet is this a feature?
Labels |
Added:
Feature
?
PR-5.1-dev
|
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 |
I rebased it to 5.1 since it's not a bugfix.