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

[4.0] Install from Web #19319

Merged
merged 26 commits into from
Jan 20, 2018
Merged

Conversation

brianteeman
Copy link
Contributor

@brianteeman brianteeman commented Jan 7, 2018

History

This functionality was developed and released for Joomla 3.2 (https://community.joomla.org/blogs/leadership/install-from-web-and-jed-listings.html) over 4 years ago. Although the original intention was that this would be part of the core it was released as an external plugin. One of the main reasons for that decision was the belief by the PLT at the time that

There are, however, a few concerns that I have. Currently the https://appscdn.joomla.org/ site that hosts the server side of the app store code is hosted with Rochen. From my own tests, Rochen's networks can't handle the type of traffic that we hope the JoomlaAppStore will be receiving.

I think after 4 years it is fair to say that either those concerns were unfounded and/or they have been resolved by the hosts

Decoupled

Being an external plugin managed by a separate team can not be called a success. Requests for fixes have been very slow, there has been zero development for several years and there are multiple open issues that have had zero response from the IFW team for many years.

It is not installable in J4 (xml issue on the update server)
has lots of code that will not be needed in j4 (checking for hathor)

Aim of this PR

[ ] Adds IFW to the core
[ ] The plugin is now enabled by default in the last position
[ ] Removes code and references to hathor
[ ] With the plugin in the core then it can be more easily developed and things like the jquery dependencies can be removed
[ ] J3 will still use the decoupled plugin - no change there
[ ] Namespace
[ ] Jquery
[ ] Extract view to own file

Testing

Apply PR
Run the sql below (replacing #__ with your db prefix


INSERT INTO `#__extensions` (`extension_id`, `package_id`, `name`, `type`, `element`, `folder`, `client_id`, `enabled`, `access`, `protected`, `manifest_cache`, `params`, `checked_out`, `checked_out_time`, `ordering`, `state`, `namespace`) VALUES
(486, 0, 'plg_installer_webinstaller', 'plugin', 'webinstaller', 'installer', 0, 1, 1, 0, '', '{"tab_position":"1"}', 0, '0000-00-00 00:00:00', 0, 0, '')

@joomla-cms-bot joomla-cms-bot added Language Change This is for Translators PR-4.0-dev labels Jan 7, 2018
@brianteeman
Copy link
Contributor Author

This will now at least load. Looks like there is still a js error which is completely beyond me.
@Fedik @DGT41 any help appreciated

@mbabker
Copy link
Contributor

mbabker commented Jan 17, 2018

A couple notes:

Because of an architectural decision in 2013, the remote server is rendering the markup shown in the backend UI. Without re-engineering things, markup for the 4.0 template will need to be added to that repo and we'll need to use something to communicate the right markup to render (we already send the Joomla version so we can probably use that).

All other media assets (CSS/JS) are self contained in the plugin. So feel free to do what you please with those.

@brianteeman
Copy link
Contributor Author

Thanks @mbabker

For now I would just like to be able to get rid of the javascript errors

@mbabker
Copy link
Contributor

mbabker commented Jan 18, 2018

@brianteeman I pushed ec441a5 onto this PR doing a round of updates for the plugin (tried to do it as a PR to your branch but GitHub kept 404'ing on the save action).

@mbabker
Copy link
Contributor

mbabker commented Jan 18, 2018

@brianteeman Check brianteeman#69 as that merges @DGT41 initial JavaScript with me debugging things to get it working. It ain't pretty but it doesn't have any JS errors now.

@rdeutz
Copy link
Contributor

rdeutz commented Jan 18, 2018

Years ago I re-wrote the plugin, but it couldn't be used because the JED API wasn't able to handle request as needed. It was and it is a stupid idea to render markup on the remote server. We shouldn't do this misstake again.

@brianteeman
Copy link
Contributor Author

@mbabker -thanks. I get those 404 sometimes as well never tracked it down

@rdeutz I remember as I tested it for you. As far as I know the JED API hasnt changed. For now I just want to get it working as is and then its in a working position and people can improve upon it

brianteeman and others added 2 commits January 18, 2018 10:48
* [4.0] Remove grunt in favour of native node build scripts (joomla#18446)

* These are junk

* First pass on converting JS

* Fix path to loader

* CS tweaks for IFW JS, move event listener registration into success callback so that the changed DOM elements correctly receive them, change how custom events are triggered so they'll actually work

* Make sure elements exist in DOM
@brianteeman
Copy link
Contributor Author

@DGT41 thanks for your JS and thanks @mbabker for converting it into a PR

@brianteeman
Copy link
Contributor Author

So now it seems to be working and we just have markup issues to resolve. I am not too familiar with the IFW server and what is the most sensible way forward now. Any suggestions?
[ ] Convert markup served by IFW server to bs4?
[ ] Add bs2 css to the plugin and keep the server code the same?

@mbabker
Copy link
Contributor

mbabker commented Jan 18, 2018

I started https://github.com/joomla-extensions/install-from-web-server/compare/j4-support to deal with the UI changes (the plugin already sends the CMS version information with each request, I'm just using that to make decisions on which templates to load now). The grid views are functional, need to work on the list views and single extension view next. I'm basically doing a quick and dirty BS2 -> BS3 -> BS4 type migration to get the markup sorted with default elements, someone with an eye for design can make things prettier later.

screen shot 2018-01-18 at 7 12 29 am

@brianteeman
Copy link
Contributor Author

@mbabker ok thats great - in that case this can probably be merged for now and then improved upon when that work is completed?

@mbabker
Copy link
Contributor

mbabker commented Jan 18, 2018

We shouldn't do this misstake again.

@rdeutz I think when we are in a state where we can rewrite the architecture, we should update everything all at once (the standalone plugin for 3.x and the core plugin in 4.x). Let's not work ourselves into a point where we're maintaining two versions of the architecture for the next 3 years. So for now, the "easiest" thing is to keep using the same architecture (with good and bad decisions involved in it) and adapt for the new major version.

@mbabker
Copy link
Contributor

mbabker commented Jan 18, 2018

@brianteeman For now I would call this a "ready to merge" state as the plugin itself is now fully functional and any further changes are just part of the rest of the refactoring of the CMS codebase for 4.x (i.e. the JavaScript decoupling from jQuery) or the work is happening outside the CMS and doesn't block work here.

@brianteeman
Copy link
Contributor Author

thanks for minifying the script - it was on my todo list but i forgot after drowning my sorrows as a result of a meeting tonight

@mbabker
Copy link
Contributor

mbabker commented Jan 19, 2018

Check and merge brianteeman#70 please. Does a lot of the cleanup stuff that needed to happen anyway (I finished the server side layouts then figured I'd do another round of effort on the plugin to get it up to spec).

* Migrate JS vars to use the script options storage

* Remove the plugin event allowing to dynamic toggle JED alert

* PHPCS pass

* Put back plugin group import, migrate installer plugin to use the same event other plugins do so display is controlled the same way consistently

* Activate tab immediately in first spot

* Oops
@brianteeman
Copy link
Contributor Author

thanks @mbabker all merged and crossed of my todo list for tomorrow

@mbabker
Copy link
Contributor

mbabker commented Jan 19, 2018

Shooting from the hip, the only real work left here before things are "clean":

  • Get the rest of the initialization JavaScript out of the document head and into the client.js file
  • @DGT41 favorite framework decoupling
  • Handing the server side layouts over to the UI team for improvements (https://github.com/joomla-extensions/install-from-web-server/compare/j4-support is feature complete as far as migrating to support both 3.x and 4.x goes and having all layouts in place (minus the unused advanced search layout), the server generated markup is almost exclusively BS4 at this point without any additional styling that the 3.x version has with the client.css file though).

Then at some point whenever JED supports it we should probably update the server's API queries into the JED to filter out extensions that aren't compatible with whatever major version is in use.

@brianteeman
Copy link
Contributor Author

my 2c would be to merge now and do further improvements later

@mbabker
Copy link
Contributor

mbabker commented Jan 19, 2018

Agree wholly. I'll merge up the layout work to the live server after this PR is merged too so it's not completely busted.

@brianteeman
Copy link
Contributor Author

thanks - i plan to check the server stuff tomorrow

@brianteeman
Copy link
Contributor Author

@mbabker I started to update the css but then saw you hadnt finished with the layouts/markup on the server so I wont do anymore until thats finished

@brianteeman
Copy link
Contributor Author

@wilsonge Can you merge this please. Its very hard to test unless you create your own IFW server and it will help to make the testing of the IFW changes if this is merged - thanks

@mbabker
Copy link
Contributor

mbabker commented Jan 20, 2018

I just pushed up the work for the layout changes. So now with this PR applied you should get the J4 optimized UI.

@brianteeman
Copy link
Contributor Author

Thanks @mbabker

Tomorrow I will try and push some fixes I have locally for the server.

@wilsonge wilsonge merged commit 0af86c7 into joomla:4.0-dev Jan 20, 2018
@wilsonge
Copy link
Contributor

Merged as requested :)

@wilsonge wilsonge added this to the Joomla 4.0 milestone Jan 20, 2018
@brianteeman
Copy link
Contributor Author

Thanks!!

@brianteeman brianteeman deleted the install_from_web_40 branch January 20, 2018 18:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Language Change This is for Translators
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants