?
Related to # 5327
avatar infograf768
infograf768
4 Dec 2014

Test to reproduce:
Go to Extensions Manager->Update Sites.
Disable the "Accredited Joomla! Translations" row.
Go to Extensions Manager->Update
Click on Purge, then on Find Updates;
Then switch to the Install Languages Manager
You will see this:
screen shot 2014-12-04 at 12 14 58

Once one has clicked on Find Languages, this unwanted package will not show anymore.

Votes

# of Users Experiencing Issue
0/1
Average Importance Score
3.00

avatar infograf768 infograf768 - open - 4 Dec 2014
avatar jissues-bot jissues-bot - change - 4 Dec 2014
Labels Added: ?
avatar Bakual
Bakual - comment - 4 Dec 2014

What I wonder: Does this happen on an updated installation or a fresh one?
Because we have the pkg_weblinks.xml in our repo but for fresh installations it will be removed during build. See https://github.com/joomla/joomla-cms/blob/aa211d32e5c6fd5f40927eb1df5a6cc5298151b8/build/build.php#L224.

The updateserver on the other hand is only added during an update.

So for fresh installations, I don't see how this could happen.
It may be an issue only for when you create a site from staging.

Maybe that helps to find the source of the issue.

avatar nikosdion
nikosdion - comment - 4 Dec 2014

Um, no. I can see the problem just by glancing at InstallerModelLanguages. See _getListQuery. The query naïvely believes that any update with extension_id = 0 is an available language. No. Every single update coming from an update server pointing to no extension or an extension ID which doesn't exist will end up having extension_id = 0. As a result the InstallerModelLanguages model is broken. This also explains why @infograf768 was complaining to me that Akeeba Backup is breaking the language installation. It wasn't like that at all. Akeeba Backup defined an update site, Joomla! screwed up on update (new extension ID, the old update site did not get removed) and Joomla! screwed up on displaying the available languages.

I hope that sheds some light into a whole great lot of misidentified issues.

As for the fix? First run a query against #__update_sites to find out the update site ID for the languages. Let's say it's 3. Then change the query in InstallerModelLanguages to have WHERE update_site_id = 3 AND type = package AND client_id = 0 AND details_url LIKE '%joomla.org/language' If you want me to fix this too, give me a holler. I'm on a roll these days :D

avatar richard67
richard67 - comment - 4 Dec 2014

@nikosdion we should not hard-code the 3, see our discussion in issue #5284 .

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

avatar nikosdion
nikosdion - comment - 4 Dec 2014

Read carefully :) As I said: "First run a query against #__update_sites to find out the update site ID for the languages. Let's say it's 3." This is where the 3 came from, it's not hard-coded.

avatar richard67
richard67 - comment - 4 Dec 2014

@nikosdion I've just checked _getListQuery in administrator/components/com_installer/models/languages.php. The commend above the "$query->where('extension_id = 0');" tells that this is for not listing languages which are already installed. I think we should leave this, and have the update_site_id = result from query to #__update_sites for name ='Accredited Joomla! Translations' as additional condition => "... where extention_id = 0 and update_site_id = x " in SQL. I could make a PR to fix this little thing, but maybe there is more to do?

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

avatar nikosdion
nikosdion - comment - 4 Dec 2014

Yes, you are right. The WHERE clause should be the following really long thing:
WHERE extension_id = 0 AND update_site_id = 3 AND type = package AND client_id = 0 AND details_url LIKE '%joomla.org/language'
Also, while we are at this, we MUST NOT hardcode the entire WHERE clause. We should use the very nice Joomla! query builder. More so when we have this really long WHERE clause.

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

avatar richard67
richard67 - comment - 4 Dec 2014

With the latest staging from just right now, which includes PR #5296 , clean fresh installation, I cannot reproduce the error with the steps provided by @infograf768

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

avatar nikosdion
nikosdion - comment - 4 Dec 2014

Reproducing the issue is very hit and miss. You need to end up with a situation where:

  • The language "updates" are not loaded
  • There is a "stray" update site (its extension is not installed)
  • The "stray" update site results in update records being written in the #__updates table with extension_id = 0
  • Visiting the Install Languages page does not modify any of the previous error conditions

It is very, very hard to reproduce the issue. It is very easy to understand why it happens and fix it. I can fix it, but I won't be able to provide test instructions which involve a realistic scenario because of that :(

avatar richard67
richard67 - comment - 4 Dec 2014

@nikosdion I can reproduce this with a fresh installed 3.3.6 and then updated with 3.4.0 Alpha update package, and then performing the steps as Jean-Marie described. The reason is that the weblinks update site is only added with an upgrade, as Thomas (Bakual) mentioned at the top. For a test with current staging code base it would be necessary to apply other patches (e.g. PR #5296 ) to such an upgraded 3.4.0 Alpha. But for developing and testing a correction I have all I need now, and so I will think about what's the best way to enhance the database query returned by _getListQuery in administrator/components/com_installer/models/languages.php, and then test if this is sufficient. I come back with a PR if I have it, or will tell here if I need help, ok?

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

avatar nikosdion
nikosdion - comment - 4 Dec 2014

Excellent! Let me know if you want any help.

avatar richard67
richard67 - comment - 4 Dec 2014

@nikosdion I have made here a modification on _getListQuery in administrator/components/com_installer/models/languages.php using a sub-query to get the update_site_id, and tested with success (setup/code base see my post above):

    $db       = JFactory::getDbo();
    $query    = $db->getQuery(true);
    $subQuery = $db->getQuery(true);
    $siteName = 'Accredited Joomla! Translations';

    // Set a subquery for the where clause to limit to language updates.
    $subQuery->select('update_site_id')
        ->from('#__update_sites')
        ->where('name = ' . $db->quote($siteName));

    // Select the required fields from the updates table.
    $query->select('update_id, name, version, detailsurl, type')
        ->from('#__updates');

    // This Where clause will limit to language updates only.
    $query->where('update_site_id IN (' . $subQuery->__toString() . ')');

    // This Where clause will avoid to list languages already installed.
    $query->where('extension_id = 0');

But because JDatabase not really supports subqueries it seems to be a kind of hack.

I also could do it with a join in the official JDatabase way.

Or I could execute the subquery inside _getListQuery and then just use the result in the query where clause, but this is theoretically not the clean way because the result of _getListQuery is then not executed at the same point of time as the subquery. But this is only a theroetical problem.

What is your opinion, what would you prefer? This one above, or the join, or the query for the update_site_id being executed inside _getListQuery and just the result of this being added to the wehere clause for the returned query?

Please let me know your opinion or who is maybe the expert to ask - I do not want to mess up Joomla with things not done in the right way.

At the end, the final languages.php should maybe use some constant for the string 'Accredited Joomla! Translations' so we do not have it hard-coded at 2 places (coming from this PR and PR #5296 ).

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

avatar nikosdion
nikosdion - comment - 4 Dec 2014

Don't do subqueries. The performance difference is virtually non-existent. JOINs are even worse due to the lack of indices and the use of string searches (basically, you end up with a slower query). Use two queries, one to get the update site ID, one to get the list of languages.

avatar richard67
richard67 - comment - 4 Dec 2014

@nikosdion Means I shall execute the query for the update site id and use the result in the where clause of the query for the language updates list, which is built in and returned by the _getListQuery function but executed by a calling function anywhere else? Would be of course the easiest way, but is as I described not nice (theoretical problem).

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

avatar nikosdion
nikosdion - comment - 4 Dec 2014

Yes, that's what I meant.

avatar richard67
richard67 - comment - 4 Dec 2014

Hmm, but now I am not sure how I shall handle the theoretical case that the 1st query returns more than 1 record (I know, should not happen that we have 2 update sites with the same name). I could do a MAX() in the qurey, so I always get the largest ID (i.e. the latest inserted update site with this name) and so always have only 1 result. Would this be ok?

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

avatar richard67
richard67 - comment - 4 Dec 2014

Does MAX() work with all supported databases? I am only specialist for Oracle SQL ;-)

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

avatar nikosdion
nikosdion - comment - 4 Dec 2014

You can simply use loadResult(). It will always return the first column of the first row returned by the SQL query. This is one of the most basic Joomla! database driver methods – exactly because it's so useful!

avatar richard67
richard67 - comment - 4 Dec 2014

So I can rely on getting one single integer value when using loadResult() ? Or does it need some check for this?

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

avatar nikosdion
nikosdion - comment - 4 Dec 2014

Yes, you can get a singe value using loadResult. The result will be either null or an integer. You only need to check if you get null. In this case the Joomla! core database tables are screwed up so just use the default update site ID 3. No matter what you use won't work, so who cares? :)

avatar richard67
richard67 - comment - 4 Dec 2014

@nikosdion How about the following for administrator/components/com_installer/models/languages.php:

const JLANGUAGE_UPDATE_SITE_NAME = 'Accredited Joomla! Translations';

...

protected function _getListQuery()
{
    $db        = JFactory::getDbo();
    $query     = $db->getQuery(true);
    $siteQuery = $db->getQuery(true);

    // Get the update_site_id for translations.
    $siteQuery->select('update_site_id')
        ->from('#__update_sites')
        ->where('name = ' . $db->quote(JLANGUAGE_UPDATE_SITE_NAME));
    $db->setQuery($siteQuery);
    $siteId = (int) $db->loadResult();

    // If no update site, set id so that no languages will be found.
    if (!$siteId)
    {
        $siteId = -1;
    }

    // Select the required fields from the updates table.
    $query->select('update_id, name, version, detailsurl, type')
        ->from('#__updates');

    // This where clause will limit to language updates only.
    $query->where('update_site_id = ' . $siteId);

    // This where clause will avoid to list languages already installed.
    $query->where('extension_id = 0');

...

?

The constant could be useful for future use, I hope the naming is according to the rules.

It should also be used for the query implemented with PR #5296 .

Or should I use a protected variable or a private one instead of the constant?

What do you think?

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

avatar richard67
richard67 - comment - 4 Dec 2014

@infograf768 what do you think about my proposed solution above, and especially the use of a constant?

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

avatar nikosdion
nikosdion - comment - 4 Dec 2014

Regarding siteQuery: do not search for the name. Search for the extension ID. Yes, you need a third query to get the extension ID from the #__extensions table, but it's a forwards compatible implementation. Other than that, I think you've nailed it!

avatar richard67
richard67 - comment - 4 Dec 2014

@nikosdion Hmm, I would like to wait for @infograf768 's response then, because he had introduced the search for the site name once when working on PR #5284 , which later was closed but went into PR #5296 , which already is merged into staging.

For yor last proposal, nikosdion, I could use what we had discussed with issue #5284 :

SELECT extension_id FROM #__extensions WHERE type='language' AND element='en-GB' AND client_id=0;

This assumed en-GB is allways installed, which is ok as we discussed.

When we use this, we do not need any constant.

@infograf768 Can you check this, too, and tell me your opinion? You know, I really would value this.

avatar richard67
richard67 - comment - 4 Dec 2014

@nikosdion @infograf768 This is the solution now according to dicussion above which seems to solve the problem (at least works here with my test setup as mentioned above):

    $db        = JFactory::getDbo();
    $query     = $db->getQuery(true);
    $extQuery  = $db->getQuery(true);
    $siteQuery = $db->getQuery(true);
    $extType   = 'language';
    $extElem   = 'en-GB';

    // Get the extension_id for the en-GB package
    $extQuery->select('extension_id')
        ->from('#__extensions')
        ->where('type = ' . $db->quote($extType))
        ->where('element = ' . $db->quote($extElem))
        ->where('client_id = 0');
    $db->setQuery($extQuery);
    $extId = (int) $db->loadResult();

    /*
     * Get the update_site_id for translations.
     * If nothing can be found, set update_site_id so no translations
     * will be found with the $query returned by this function here.
     */
    if ($extId)
    {
        $siteQuery->select('update_site_id')
            ->from('#__update_sites_extensions')
            ->where('extension_id = ' . $extId);
        $db->setQuery($siteQuery);
        $siteId = (int) $db->loadResult();
        if (!$siteId)
        {
            $siteId = -1;
        }
    }
    else
    {
        $siteId = -1;
    }

    // Select the required fields from the updates table.
    $query->select('update_id, name, version, detailsurl, type')
        ->from('#__updates');

    // This where clause will limit to language updates only.
    $query->where('update_site_id = ' . $siteId);

    // This where clause will avoid to list languages already installed.
    $query->where('extension_id = 0');



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

avatar richard67
richard67 - comment - 4 Dec 2014

ahh, sorry, "JFactory::getDbo()" should be "$this->getDbo()", is nicer ;-)

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

avatar richard67
richard67 - comment - 4 Dec 2014

I have meanwhile created a pull request for this issue here:
PR #5327
I decided to minimize database acces by doing the queries in the contructor.
@nikosdion Could you let me know what you think about it?
@infograf768 Of course I am also interested on your opinion, because you will have to test the PR. But You might have to do some mergin in case if the system which you used for testing did not have the other PRs for this file included.

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

avatar zero-24 zero-24 - change - 4 Dec 2014
Status New Closed
avatar jissues-bot
jissues-bot - comment - 4 Dec 2014

Set to "closed" on behalf of @zero-24 by The JTracker Application at issues.joomla.org/joomla-cms/5320

avatar jissues-bot jissues-bot - close - 4 Dec 2014
avatar zero-24
zero-24 - comment - 4 Dec 2014

Closing as we have a PR by @richard67 here: #5327

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

avatar jissues-bot jissues-bot - change - 4 Dec 2014
Closed_Date 0000-00-00 00:00:00 2014-12-04 20:55:59
avatar zero-24 zero-24 - change - 4 Dec 2014
Rel_Number 5327
Relation Type Related to
avatar infograf768
infograf768 - comment - 5 Dec 2014

@Bakual
#5320 (comment)

It is my updated test site to staging where the sql updates have been done.

@nikosdion
Thanks for finding this long standing issue

@richard67
Thanks for working on it. #5327 solves it!

Add a Comment

Login with GitHub to post a comment