-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks good. Still probably going to do some text tweaks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @jespino
I tried to test this along with the dependent PRs and currently nothing happens when we click on the Upgrade button on a team edition. Please check this GIF to demonstrate this issue. Can you please check and correct the issue (or) please let me know if I am missing something here:
@srkgupta yes, clearly looks like broken, it should be showing an spinner there! maybe the merges has broken it. I'll review it. |
@srkgupta now is fixed, take into consideration that the server have to announce himself as a valid published version, for example 5.24, to solve that, at the server side you can go to |
@srkgupta I'm making some changes to the UI based on some feedback from @jasonblais |
Hi @jespino |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested the PR along with it's dependencies on Mac instance and now the system properly notifies the user that the upgrade is handled only on linux servers. Was unable to test this PR on a linux instance. @mgdelacroix will help test this on a linux instance and verify if the feature is working fine or not. Will be testing the PR on the test instance once the PR is merged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested on Linux:
Works as expected. After clicking "Upgrade to Enterprise Edition", I see a download progress, then prompt to restart server, and after restart I can confirm I've been upgraded to E20.
For a server binary without write permissions instructions on changing the binary permissions were displayed
For a server binary owned by root, similar instructions for a system administrator to perform were displayed.
@jespino
|
* Adding upgrader interface * Using the new client methods for upgrade/restart and adding translations * Fix lint * Pointing to the right branch in mattermost-redux * Pointing to the right branch in mattermost-redux * Fixing tests * i18n * Handling different text from previous upgrades * Fix tests * Adding error handling on starting upgrade * Fixing restart buton * Render markdown formatted error from the server * Adding telemetry to the upgrade process
* Adding upgrader interface (#5471) * Adding upgrader interface * Using the new client methods for upgrade/restart and adding translations * Fix lint * Pointing to the right branch in mattermost-redux * Pointing to the right branch in mattermost-redux * Fixing tests * i18n * Handling different text from previous upgrades * Fix tests * Adding error handling on starting upgrade * Fixing restart buton * Render markdown formatted error from the server * Adding telemetry to the upgrade process * Pointing the web app to the correct mattermost-redux commit
Summary
Adding upgrader interface
Related Pull Requests
Screenshots