Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[com_installer] [updatesites] Add options to delete and rebuild update sites #9744

Merged
merged 17 commits into from May 2, 2016

Conversation

andrepereiradasilva
Copy link
Contributor

Pull Request for Issue #8512.

Summary of Changes

After updating Joomla and extensions a lot of times (since 1.7) we get a lot of obsolete update sites from domain changes, url changes, etc that extension developers forget to delete on uninstall.

This makes searching for updates more slow. Right now we can disable the update sites, but we cannot delete or even rebuild the update sites table.

This PR adds two new options to com_installer updates sites view: delete and rebuild

  • delete: deletes an update site (joomla core updates sites cannot be deleted)
  • rebuild: the update sites are rebuild from the manifest files.

Testing Instructions

  1. Use latest staging
  2. Apply this patch
  3. Go to "Extensions -> Manage -> Update Sites"
  4. Notice the two new buttons
    image
  5. Try to Rebuild the update sites and check if everything is fine.
  6. Install some extensions with update sites of all the types you can imagine (libraries, templates, plugin, modules, components, etc).
  7. Repeat step 5 and check if everything is fine.
  8. Delete some update sites (the 4 core joomla update sites will not be allowed to delete) the others you can delete.
  9. Repeat step 5 and check if everything is fine.

Observations

Code revisions, improvements and suggestions are welcomed.

@joomla-cms-bot joomla-cms-bot added Language Change This is for Translators PR-staging labels Apr 5, 2016
@@ -131,6 +131,12 @@ COM_INSTALLER_MSG_UPDATE_NOUPDATES="There are no updates available at the moment
COM_INSTALLER_MSG_UPDATE_SITES_COUNT_CHECK="Some update sites are disabled. You may want to check the <a href="_QQ_"%s"_QQ_">Update Sites Manager</a>."
COM_INSTALLER_MSG_UPDATE_SUCCESS="Updating %s was successful."
COM_INSTALLER_MSG_UPDATE_UPDATE="Update"
COM_INSTALLER_MSG_UPDATESITES_DELETE_ERROR="And error as occured while trying to delete "_QQ_"%s"_QQ_" update site: %s."
Copy link
Contributor

Choose a reason for hiding this comment

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

An error has...

@andrepereiradasilva
Copy link
Contributor Author

ok still need some changes here, plus resolving the plural forms.

discovered some bugs. will tell when ready.

@infograf768
Copy link
Member

@andrepereiradasilva

I suggest to slightly change the constants for the plural strings, i.e adding _N_
from

+COM_INSTALLER_MSG_UPDATESITES_DELETE_UPDATESITES_DELETED="%s update sites have been deleted."
+COM_INSTALLER_MSG_UPDATESITES_DELETE_UPDATESITES_DELETED_1="1 update site has been deleted."

to

+COM_INSTALLER_MSG_UPDATESITES_DELETE_N_UPDATESITES_DELETED="%s update sites have been deleted."
+COM_INSTALLER_MSG_UPDATESITES_DELETE_N_UPDATESITES_DELETED_1="1 update site has been deleted."

@brianteeman
Copy link
Contributor

Thanks @infograf768 that was what I had suggested before the N

@brianteeman
Copy link
Contributor

I have tested this item ✅ successfully on 17db21e

Tested delete
Tested rebuild
Added fake updates sites in the db and rebuild removed them

All good (once the language string key is updated)


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

@andrepereiradasilva
Copy link
Contributor Author

please don't test yet i has some bugs still.
i'm trying to solve them.

@brianteeman
Copy link
Contributor

Really? Every test I could think of worked

@andrepereiradasilva
Copy link
Contributor Author

problems still to solve (that i know of):

  • the en-GB package, if somehow deleted, doesn't get the update (because it does not have an extension in the database ...).
  • also had to do an additional check if the extension is only discovered or it's really installed.
  • plural variables name

@brianteeman
Copy link
Contributor

the en-GB package, if somehow deleted, doesn't get the update (because it does not have an extension in the database ...).

Accredited Joomla! Translations update site cannot be deleted.

In addition its a protected extension so cant be deleted

@brianteeman brianteeman closed this Apr 7, 2016
@brianteeman brianteeman reopened this Apr 7, 2016
@andrepereiradasilva
Copy link
Contributor Author

@brianteeman
After some investigation and with the help form @infograf768 the en-GB language problem is something not related to this PR.

Explaining, for what i know, and correct me if i'm wrong:

A language package install 3 extensions in the database:

  • type = language | Admin
    • Manifest: /administrator/language/xx-XX/xx-XX.xml
  • type = language | Site
    • Manifest: /language/xx-XX/xx-XX.xml
  • type = package | Site
    • Manifest: /administrator/manifests/packages/pkg_xx-XX.xml

The en-GB language pack (comes by default) somehow only has two extensions in the database...

  • type = language | Admin
    • Database Id: 601
    • Manifest: /administrator/language/en-GB/en-GB.xml
  • type = language | Site
    • Database Id: 600
    • Manifest: /language/en-GB/en-GB.xml
  • type = package | Site
    • Database Id: doesn't exist
    • Manifest: /administrator/manifests/packages/pkg_en-GB.xml

See https://github.com/joomla/joomla-cms/blob/staging/installation/sql/mysql/joomla.sql#L618-L619

Since the update servers are declared in the pkg manifest, if somehow deleted, will not be reconstructed in this PR.
Of course this is a wild scenario, since deleting it is not possible through the update sites view.

So this is not related to this PR, this PR does not readd the en-GB update site after "accidental" delete because, as said, the pkg_en-GB extension does not exist, by default, in joomla database.

But from a coherence point of view, if a manifest exists, the extension should exist in the database.

In other words, IMHO it should exist a blocked en-GB package extension in the database on joomla install and the udpate site should point to that extension (as all language packs do).

Remember you have blocked extension in the database for all other manifest files, including joomla core, joomla update, etc.

@brianteeman
Copy link
Contributor

Not 100% sure I understand your last sentence but as for the rest it really is beyond the scope of this PR

@infograf768
Copy link
Member

@brianteeman
What @andrepereiradasilva says is that this PR will work perfectly if we add in our sqls the Database Id: for the en-GB fake pack (in _extensions).
If not done, this PR may let people delete the en-GB entry in updatesite by mistake. This would be an issue if someone has never installed a lang pack and does it for the first time after deleting it.
Just a matter of adding to joomla.sql and updates sql the fake pkg.
I guess :)

@andrepereiradasilva
Copy link
Contributor Author

yes, it's out of the scope of this PR.

Remember you have blocked extension for all other manifest files, including joomla core, joomla update, etc.

Some examples:

Joomla Core: All logic

Joomla Update: All logic

en-GB language package: ?

Something doesn't seem right ...

Put ok, let's concentrate in the PR and check that later.

@andrepereiradasilva
Copy link
Contributor Author

What @andrepereiradasilva says is that this PR will work perfectly if we add in our sqls the Database Id: for the en-GB fake pack (in _extensions).

Yes, but in another PR.

If not done, this PR may let people delete the en-GB entry in updatesite by mistake. This would be an issue if someone has never installed a lang pack and does it for the first time after deleting it.

This PR doesn't allow to delete any update site that has update.joomla.org in the URI.
See https://github.com/andrepereiradasilva/joomla-cms/blob/update-sites-manage/administrator/components/com_installer/models/updatesites.php#L366

Just a matter of adding to joomla.sql and updates sql the fake pkg.

Not only, the update site extension id should be linked en-Gb package too (since it's in the en-GB package manifest that exists the update server URI).

@brianteeman
Copy link
Contributor

As this PR does not allow you to delete that update site I really think its a non-issue

@andrepereiradasilva
Copy link
Contributor Author

Yes, it's not an issue because of the special treatment that exists in en-GB, it's just an inconsistency i discovered in this PR.

I will not fix this is this PR. So let's concentrate in this PR.

Will do the rest of the changes and tell when is finished.

@joomla-cms-bot
Copy link

This PR has received new commits.

CC: @brianteeman


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

@joomla-cms-bot
Copy link

This PR has received new commits.

CC: @brianteeman


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

@joomla-cms-bot
Copy link

This PR has received new commits.

CC: @brianteeman


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

@andrepereiradasilva
Copy link
Contributor Author

ok, now should be ok.

@brianteeman
Copy link
Contributor

I dont understand the comment about a discovered extension not being installed. If I click on discover and install an extension then it is installed and if it has an update server in the xml it is added to the update sites


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

@brianteeman
Copy link
Contributor

I realise now you meant not to parse the extensions that are listed in Discover but have not been installed yet


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

@brianteeman
Copy link
Contributor

I have tested this item ✅ successfully on 4d26e51


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

@brianteeman brianteeman added this to the Joomla! 3.6.0 milestone Apr 14, 2016
@conconnl
Copy link
Member

I have tested this item ✅ successfully on 4d26e51

I have tested it successfully.
Installed an extension, deleted the Update Site and recreated it by using the rebuild function.
Afterwards removed extension and the Update Site was removed too.


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

@brianteeman
Copy link
Contributor

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Apr 15, 2016
@insitevision
Copy link

I have tested this item ✅ successfully on 4d26e51

Installed components, plugins, and template with update sites.
Followed the test instruction, deleted the update sites. After rebuild the update sites were succesfully recreated.


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

@rdeutz rdeutz merged commit 4586b59 into joomla:staging May 2, 2016
@andrepereiradasilva andrepereiradasilva deleted the update-sites-manage branch May 2, 2016 06:49
@brianteeman brianteeman removed the RTC This Pull Request is Ready To Commit label May 11, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Language Change This is for Translators
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants