? Success

User tests: Successful: Unsuccessful:

avatar dbhurley
dbhurley
3 Sep 2014

Summary

This PR removes the weblinks extension from the core and changes it to an optional extension.

Expected behavior

New Installations

  • When installing a fresh Joomla, the weblinks extension will no longer be part of the installation.
  • The package can be installed afterwards using the webinstaller or by downloading it from the repo (https://github.com/joomla-extensions/weblinks/releases). This part needs to be set up after this PR is merged.

Updates

  • If weblinks is installed, it will not be removed. There will be a new updateserver in the database and weblinks will get its own updates going forward.
  • If weblinks has been uninstallled prior to the update, no files will be added to your installation anymore.

Testing Instructions

This PR can't be tested well using the Patchtester component. Test packages are available for download at http://developer.joomla.org/PR-packages/4214/. Use the full package to test new installations and the update one to test an update.

avatar dbhurley dbhurley - open - 3 Sep 2014
avatar jissues-bot jissues-bot - change - 3 Sep 2014
Labels Added: ?
avatar brianteeman
brianteeman - comment - 3 Sep 2014

I noticed this doesnt update ALL the sql scripts

On 3 September 2014 16:17, David Hurley notifications@github.com wrote:


You can merge this Pull Request by running

git pull https://github.com/joomla-projects/joomla-cms weblinksRemoval

Or view, comment on, or merge it at:

#4214
Commit Summary

  • Remove weblinks data from install SQL
  • Formatting in SQL files, remove weblinks extensions from install script
  • Admin mod_stats has a dependency on com_weblinks
  • Site mod_stats has a dependency on com_weblinks
  • Remove weblinks data from MySQL sample data sets
  • Provision the weblinks package manifest, it'll be needed for updates
  • Update queries to add the weblinks package extension and update server
  • Remove the weblinks code

File Changes

Patch Links:


Reply to this email directly or view it on GitHub
#4214.

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

avatar dbhurley
dbhurley - comment - 3 Sep 2014

Right, this doesn't affect the update sqls. Should only apply to new installs.

avatar mbabker
mbabker - comment - 3 Sep 2014

It's something that needs some thought and I'm not quite sure what the right answer is. If we pull it out, someone updating from 2.5 would still have the weblinks 2.5 schemas and that probably leaves it unusable until they update the weblinks package. Conversely, without the extension files, someone updating from 2.5 would have the updated database schemas but the 2.5 code which will probably leave it unusable until they update the package. Interesting catch-22.

avatar brianteeman
brianteeman - comment - 3 Sep 2014

No I meant the SAMPLE data sql sets for postgres and azure

On 3 September 2014 16:36, David Hurley notifications@github.com wrote:

Right, this doesn't affect the update sqls. Should only apply to new
installs.


Reply to this email directly or view it on GitHub
#4214 (comment).

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

avatar mbabker
mbabker - comment - 3 Sep 2014

Oh, those should be updated at some point then.

On Wed, Sep 3, 2014 at 11:37 AM, Brian Teeman notifications@github.com
wrote:

No I meant the SAMPLE data sql sets for postgres and azure

On 3 September 2014 16:36, David Hurley notifications@github.com wrote:

Right, this doesn't affect the update sqls. Should only apply to new
installs.


Reply to this email directly or view it on GitHub
#4214 (comment).

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/


Reply to this email directly or view it on GitHub
#4214 (comment).

avatar brianteeman
brianteeman - comment - 3 Sep 2014

@test
Clean install selecting NO sample data
After install go to the extensions manager and click on Database and you get
An error has occurred.
1146 Table '123.lbv8u_weblinks' doesn't exist SQL=SHOW COLUMNS IN lbv8u_weblinks WHERE Field = 'sid'
http://i.tee.mn/vhlS.png

avatar brianteeman
brianteeman - comment - 3 Sep 2014

@test
Clean install selecting Blog sample data it hangs at the installing sample data

avatar dbhurley
dbhurley - comment - 3 Sep 2014

Removed Weblinks from Azure and Postgres

avatar mbabker
mbabker - comment - 3 Sep 2014

I've removed the weblinks related update queries from the update files. Those should be handled by installing the updated weblinks package.

avatar brianteeman brianteeman - change - 3 Sep 2014
The description was changed
Status New Pending
avatar tristanbailey
tristanbailey - comment - 5 Sep 2014

First patch I have every tried testing so hope this is the right place and way to comment.

Added it to 3.3.3. The menu in components still shows and i get the expected 404 when i choose the links

It would not let me uninstall the module and component , but maybe this is meant to be patched before install and not in this way?

thanks for the work

t

avatar franz-wohlkoenig
franz-wohlkoenig - comment - 7 Sep 2014

"An error has occurred" after Klick on Menu "Weblinks" and "Weblinks > Links" (non Error-Message at ""Weblinks > Categories").

avatar brianteeman
brianteeman - comment - 7 Sep 2014

Testers you CANNOT use the com_patchtester for this. The removal of the weblinks component is only for NEW installs - so you need to download and install this https://github.com/joomla-projects/joomla-cms/archive/weblinksRemoval.zip

avatar Bakual
Bakual - comment - 7 Sep 2014

The update path needs to be tested as well, but can only be done with the beta once this is merged.
There is no easy way otherwise I think.

avatar tristanbailey
tristanbailey - comment - 8 Sep 2014

Thanks @brianteeman that's what I assumed after testing it. As it cant be reverted in the tester too so will trash this instance and try again.

avatar Bakual
Bakual - comment - 24 Sep 2014

Michael prepared testing packages so this can be tested better.
See http://developer.joomla.org/PR-packages/4214/
There are two packages. The full one is meant for fresh installations while the update one is to (surprise!) update an existing Joomla.

avatar betweenbrain
betweenbrain - comment - 24 Sep 2014

The full installation package (http://developer.joomla.org/PR-packages/4214/Joomla_3.4.0-dev-Development-Full_Package.zip) fails to install when installing with Sample Data. But, it installs fine without sample data, and seems to be functioning properly with 3PD extensions.

avatar mbabker
mbabker - comment - 24 Sep 2014

The sample data's in really bad shape, there's no real guarantee we can get it to work in the shape it's in right now short of taking the effort to rebuilding it all from scratch.

avatar blueforce blueforce - test_item - 25 Sep 2014 - Tested successfully
avatar blueforce
blueforce - comment - 25 Sep 2014

I've tested 3 szenarios:
1st: full 3.4 package without sampledata -> result: success
2nd: Update a 3.3.4 install without weblinks installed -> after Update to 3.4 the weblinks elemetns are correctly not there..
3rd: Update a 3.3.4 install with weblinks installed, after Update I've deinstalled all weblinks elements (Component, Module and Plugins) successful
I documented all these steps of the update-process for other testers here ->

This comment was created with the J!Tracker Application at http://issues.joomla.org/.

avatar blueforce blueforce - test_item - 25 Sep 2014 - Tested successfully
avatar mbabker mbabker - change - 26 Sep 2014
The description was changed
avatar Bakual
Bakual - comment - 26 Sep 2014

Packages have been rebuilt. Sample data should now work as well.
Please retest :+1:

avatar brianteeman
brianteeman - comment - 26 Sep 2014

Just installed the full release from http://developer.joomla.org/PR-packages/4214/ with testing data and weblinks is still present

avatar dbhurley
dbhurley - comment - 26 Sep 2014

My fault, I built with the wrong branch. It's being re-uploaded - hang on a minute :)

avatar Bakual
Bakual - comment - 26 Sep 2014

Still no good. David needs to practice a bit more :smile:

avatar dbhurley
dbhurley - comment - 26 Sep 2014

Haha, easy now. Just was attempting a different method for building the packages instead of by PR branch. The new packages are uploaded and manual review shows weblinks not present.

avatar brianteeman
brianteeman - comment - 26 Sep 2014

Install with the test sample data set results in the following error on login

×
Warning
Error loading component: com_weblinks, Component not found

avatar dbhurley
dbhurley - comment - 26 Sep 2014

@brianteeman Just for curiosity, have you tried other sample sets?

avatar brianteeman
brianteeman - comment - 26 Sep 2014

Working my way through them now
Learn Joomla English is OK
Default English has same error
Brochure English is OK
Blog English has same error
No data is OK

avatar dbhurley
dbhurley - comment - 26 Sep 2014

Ok, sounds like the same problems as before. @mbabker has pushed a commit this AM that we thought took care of those others. Thanks for the test.

avatar brianteeman
brianteeman - comment - 26 Sep 2014

It might be an unrelated bug but the install from web plugin fails to
install with this release

On 26 September 2014 14:49, David Hurley notifications@github.com wrote:

Ok, sounds like the same problems as before. @mbabker
https://github.com/mbabker has pushed a commit this AM that we thought
took care of those others. Thanks for the test.


Reply to this email directly or view it on GitHub
#4214 (comment).

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

avatar Bakual
Bakual - comment - 26 Sep 2014

It might be an unrelated bug but the install from web plugin fails to
install with this release

I guess it doesn't work with 3.4 yet in general. Usually they need to add it to the supported releases somehow on the server (I think).

avatar Bakual
Bakual - comment - 26 Sep 2014

Ok, sounds like the same problems as before. @mbabker has pushed a commit this AM that we thought took care of those others. Thanks for the test.

It's not the same issue. Before it failed during installation. Now it seems only to show a warning after login in the backend.

avatar brianteeman
brianteeman - comment - 26 Sep 2014

In addition if you check the zip file you will see that /components/com_weblinks is still present

avatar mbabker
mbabker - comment - 26 Sep 2014

The folder being present is a bad merge.

avatar mbabker
mbabker - comment - 26 Sep 2014

Warning
Error loading component: com_weblinks, Component not found

This is because our API checking for components needs an overhaul. The mod_stats modules display the number of weblinks, so to retain that capability, I wrapped the code querying that data in a JComponentHelper::isEnabled() check, which apparently is a bad idea if an extension isn't even installed. I've got a fix for this too...

avatar mbabker
mbabker - comment - 26 Sep 2014

Updated packages posted with the last commit adding a new API endpoint to check if a component is installed since the rest of JComponentHelper is inadequate to do that simple check.

avatar brianteeman
brianteeman - comment - 26 Sep 2014

The updated package no longer has components/com_weblinks and the fix in the stats module resolves that error.

Is there a way to test installing weblinks yet?

avatar dbhurley
dbhurley - comment - 26 Sep 2014

Not at the moment. We're merely testing this PR for removal of weblinks. We have a separate repo for weblinks already and we have the steps to include it in the JED. At the moment I don't believe those packages (zips) have been added and thus won't be discoverable yet by the web installer. That will need to be the next item to test.

avatar brianteeman
brianteeman - comment - 26 Sep 2014

That needs some work michael

On 26 September 2014 15:40, Michael Babker notifications@github.com wrote:

Package at
https://github.com/joomla-extensions/weblinks/releases/tag/3.4.0-dev


Reply to this email directly or view it on GitHub
#4214 (comment).

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

avatar mbabker
mbabker - comment - 26 Sep 2014

Fixed up as best as I can. There's still errors if you install as it's packaged because the language files are present in the CMS repo (as agreed upon) and they are copied in the weblinks repo. Without any of the package's files present, there's no errors in the install.

avatar brianteeman
brianteeman - comment - 26 Sep 2014

As stated the package works but with an error message because of the
language file issue

On 26 September 2014 16:18, Michael Babker notifications@github.com wrote:

Fixed up as best as I can. There's still errors if you install as it's
packaged because the language files are present in the CMS repo (as agreed
upon) and they are copied in the weblinks repo. Without any of the
package's files present, there's no errors in the install.


Reply to this email directly or view it on GitHub
#4214 (comment).

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

avatar Bakual
Bakual - comment - 26 Sep 2014

There's still errors if you install as it's packaged because the language files are present in the CMS repo (as agreed upon) and they are copied in the weblinks repo. Without any of the package's files present, there's no errors in the install.

What if we don't install the languages from com_weblinks into the root/language folder but instead inside the components/com_weblinks/language folder? Imho it's best practice to have them there as it allows overriding the language file and keepss the component files together in one place.

avatar dgt41
dgt41 - comment - 26 Sep 2014

@mbabker since the language files will always be in place why don’t you comment this line
https://github.com/joomla-extensions/weblinks/blob/master/build.xml#L62 ? Or what @Bakual suggests

avatar brianteeman
brianteeman - comment - 26 Sep 2014

+1 @bakual

On 26 September 2014 16:41, Dimitri Grammatiko notifications@github.com
wrote:

@mbabker https://github.com/mbabker since the language files will
always be in place why don’t you comment this line
https://github.com/joomla-extensions/weblinks/blob/master/build.xml#L62 ?
Or what @Bakual https://github.com/Bakual suggests


Reply to this email directly or view it on GitHub
#4214 (comment).

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

avatar mbabker
mbabker - comment - 26 Sep 2014

@dgt41 That line in the build.xml is for the package's language file actually, not the individual extensions. I don't believe we have that one in the CMS repo.

@Bakual As long as it all works fine for other languages since those files will be in the main language folders, that could work. It just means we have to make sure the language files stay in sync.

avatar Bakual
Bakual - comment - 26 Sep 2014

It just means we have to make sure the language files stay in sync.

We have to keep them in sync anyway, otherwise you would potentially overwrite a newer file with an older one.

avatar betweenbrain
betweenbrain - comment - 26 Sep 2014

What if we don't install the languages from com_weblinks into the root/language folder but instead inside the components/com_weblinks/language folder? Imho it's best practice to have them there as it allows overriding the language file and keeps the component files together in one place.

+1

avatar roland-d
roland-d - comment - 27 Sep 2014

What if we don't install the languages from com_weblinks into the root/language folder but instead inside the components/com_weblinks/language folder? Imho it's best practice to have them there as it allows overriding the language file and keepss the component files together in one place.

+:100:

avatar mbabker
mbabker - comment - 4 Oct 2014

Updated CMS packages are published to http://developer.joomla.org/PR-packages/4214/ which is this branch in sync with the main 3.4-dev branch. Also, an updated Weblinks package is published at https://github.com/joomla-extensions/weblinks/releases/tag/3.4.0-dev with the language change suggested above.

avatar roland-d
roland-d - comment - 4 Oct 2014

Testing new install:
1. No sample data: OK
2. Blog English: OK
3. Brochure English: OK
4. Default English: OK
5. Learn Joomla! English: OK
6. Test English: OK

Testing update install:
1. Update with Weblinks installed: OK Weblinks still works
2. Update with Weblinks not installed: OK Weblinks is not added

Testing Weblinks install: OK


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

avatar roland-d roland-d - test_item - 4 Oct 2014 - Tested successfully
avatar roland-d roland-d - change - 4 Oct 2014
Status Pending Ready to Commit
avatar ketchupmonki
ketchupmonki - comment - 5 Oct 2014

Tested in clean install, everything working as expected.

avatar Bakual Bakual - change - 6 Oct 2014
Labels Added: ?
avatar Bakual
Bakual - comment - 6 Oct 2014

Set to RTC since we have several good tests now. Thanks all!

avatar Bakual
Bakual - comment - 6 Oct 2014

Merged into 3.4-dev. Thanks all who contributed and tested!

avatar Bakual Bakual - close - 6 Oct 2014
avatar Bakual Bakual - change - 6 Oct 2014
Status Ready to Commit Closed
Closed_Date 0000-00-00 00:00:00 2014-10-06 19:24:51
avatar peterlose
peterlose - comment - 7 Oct 2014

I have some changes to the weblinks component, we should I do the PR?

avatar zero-24
zero-24 - comment - 7 Oct 2014

@losedk i think this is the right place now: https://github.com/joomla-extensions/weblinks

avatar peterlose
peterlose - comment - 7 Oct 2014

@zero-24 Thanks!

Add a Comment

Login with GitHub to post a comment