UX: improve UI of software update page & display more info. #214
Conversation
This PR will refresh the software update page so that self-hosters can more easily understand the plugins they have installed and which plugins need updates, and so that they are able to quickly and easily update them (or update everything).
There was a problem hiding this comment.
This is looking really great so far 👌
I wonder if we could add a general system spec for this update system? Even if to start with for this PR we made an extremely basic one that navigated us to this page and confirmed the top Discourse row of the update list has the expected version and details. A spec that actually tests the update process itself and also plugin details would be amazing, but it could be quite tricky 🤔
I think we could probably link these commit hashes to their relevant repos too -- just keep them the same grey text style though
| @@ -1,62 +1,72 @@ | |||
| <tr> | |||
| <tr class="repo {{if @repo.hasNewVersion 'new-version'}}"> | |||
There was a problem hiding this comment.
Can you please convert this component to a .gjs component while you are at it?
admin/assets/javascripts/discourse/components/docker-manager/repo-status.hbs
Outdated
Show resolved
Hide resolved
| return this.plugin.nameTitleized; | ||
| } | ||
|
|
||
| let name = this.name |
There was a problem hiding this comment.
I'm just wondering why you need this other stuff? Everything on this page is either Discourse or a plugin isn't it?
There was a problem hiding this comment.
I needed this since the docker_manager plugin is a hidden plugin and I was fetching plugin information only for the visible plugins. It's no longer the case after the commit 662fee0
| get author() { | ||
| if (this.plugin) { | ||
| return this.plugin.author; | ||
| } else if (this.name === "docker_manager") { |
There was a problem hiding this comment.
This is unnecessary -- docker_manager is an official plugin owned by us, so it should do this by default with this.plugin.author:
There was a problem hiding this comment.
Same as #214 (comment). I needed this since the docker_manager plugin is a hidden plugin and I was fetching plugin information only for the visible plugins. It's no longer the case after the commit 662fee0
| <div class="updates-heading"> | ||
| <h3>{{i18n "admin.docker.update_title"}}</h3> | ||
| {{#unless this.outdated}} | ||
| <button |
There was a problem hiding this comment.
Why not use DButton for this? This one has currently got slightly different styling to the Update buttons in the list
| @@ -21,6 +21,13 @@ def repos | |||
| official: Plugin::Metadata::OFFICIAL_PLUGINS.include?(r.name), | |||
| } | |||
|
|
|||
| plugin = Discourse.visible_plugins.find { |p| p.path == "#{r.path}/plugin.rb" } | |||
There was a problem hiding this comment.
Need to be careful with this...the page errors completely on the client if you give it a wrong path:
There was a problem hiding this comment.
Or maybe this is an existing problem -- either way we should account for this.
There was a problem hiding this comment.
This is fixed by the commit 8449511. Now it will redirect to the 404 page if plugin not found
| import { helper as buildHelper } from "@ember/component/helper"; | ||
| import { htmlSafe } from "@ember/template"; | ||
|
|
||
| export default buildHelper(function (params) { |
There was a problem hiding this comment.
I don't think in this case you need buildHelper. I think you could do:
export default function commitUrl(cssClass, version, prettyVersion, url) {
}Then you can get rid of your const line below too
Co-authored-by: Martin Brennan <mjrbrennan@gmail.com>
Co-authored-by: Martin Brennan <mjrbrennan@gmail.com>
Co-authored-by: Martin Brennan <mjrbrennan@gmail.com>
We need it to get the docker_manager plugin information.
|
@martin-brennan all the requested changes are now completed. |
There was a problem hiding this comment.
Sorry but we recently changed the plugins list in core to:
- Remove the title link
- Add an external link icon to "Learn more"
Can you please bring this page up to date with those changes before merge?
This is probably a preexisting issue, but if you set UNICORN_WORKERS=1 (I had this on randomly) we fail an upgrade gracefully but we also encounter this error, can you please fix this to do to_i on num_workers_spun_down?
/home/mb/repos/docker_manager/lib/docker_manager/upgrader.rb:148:in `rescue in upgrade': undefined method `positive?' for nil:NilClass (NoMethodError)
if num_workers_spun_down.positive? && !reloaded
^^^^^^^^^^
from /home/mb/repos/docker_manager/lib/docker_manager/upgrader.rb:140:in `upgrade'
from /home/mb/repos/docker_manager/scripts/docker_manager_upgrade.rb:19:in `block in <main>'
from /home/mb/repos/docker_manager/scripts/docker_manager_upgrade.rb:6:in `fork'
from /home/mb/repos/docker_manager/scripts/docker_manager_upgrade.rb:6:in `<main>'
Also not sure why but for me locally unicorn_launcher_pid and unicorn_master_pid was always blank, see:
ps aux | rg unicorn
mb 66728 0.2 0.0 107068 28424 pts/3 S+ 12:37 0:00 ruby /home/mb/repos/discourse/bin/unicorn
mb 66733 10.2 1.2 2412700 395968 pts/3 Sl+ 12:37 0:09 unicorn master -N -c /home/mb/repos/discourse/config/unicorn.conf.rb
mb 67511 1.8 1.2 2447908 413020 pts/3 Sl+ 12:37 0:01 unicorn worker[0] -N -c /home/mb/repos/discourse/config/unicorn.conf.rb
mb 67514 1.8 1.2 2447756 413660 pts/3 Sl+ 12:37 0:01 unicorn worker[1] -N -c /home/mb/repos/discourse/config/unicorn.conf.rb
mb 67519 1.7 1.2 2446984 414316 pts/3 Sl+ 12:37 0:01 unicorn worker[2] -N -c /home/mb/repos/discourse/config/unicorn.conf.rb
mb 68883 0.0 0.0 9696 6400 pts/5 S+ 12:39 0:00 rg unicorn
Anyway though that could be fixed in a separate PR if needed, the main fix is to just change the list a bit to have the correct new format then we can merge this 🎉
This PR will refresh the software update page so that self-hosters can more easily understand the plugins they have installed and which plugins need updates, and so that they are able to quickly and easily update them (or update everything).
Before
After