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

Desktop manual check for updates #45757

Merged
merged 3 commits into from
Sep 23, 2020

Conversation

nsakaimbo
Copy link
Contributor

Description

This PR adds a manual "Check for Updates" menu item. If an update is available, this feature will follow the established updater flow for each platform (download and install on Mac, Windows; and navigate to WordPress Desktop homepage for Linux). If an update is not available, an "Update not available" dialog is displayed to the user.

check-for-updates-menu
check-for-updates-update-available
check-for-updates-no-update

To Test

Build and run this branch and amend the version number in desktop/package.json to verify behavior for update available/not available. (When testing locally, don't forget to set CONFIG_ENV === release.),

@nsakaimbo nsakaimbo added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Sep 18, 2020
@matticbot
Copy link
Contributor

@matticbot
Copy link
Contributor

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

Base automatically changed from linux-manual-updater-test to desktop/release/6.1.0 September 21, 2020 17:48
@mokagio
Copy link
Contributor

mokagio commented Sep 22, 2020

@nsakaimbo hey there 👋

I've been trying to test this, but I can't get it to build.

From the desktop/ directory, I tried make build and it fails with:

Config file config/secrets.json does not exist. Please create one.

I tried make config. It failed with the same error.

I took my chances and cp config/empty-secrets.json config/secrets.json from the repo root. The failure changed to

desktop_oauth_client_id must be 43452 in secrets.json in release build

From what I can see, I need to have some secrets in my repo. I can see CI decrypts some stuff too, by I don't have that key.

What can I do to get past this?

@nsakaimbo
Copy link
Contributor Author

nsakaimbo commented Sep 22, 2020

Hi, @mokagio! Thanks for helping test!

Hopefully these steps should help:

  1. Generate correct secrets.json with: openssl aes-256-cbc -md md5 -d -in desktop/resource/calypso/secrets.json.enc -out config/secrets.json -k "${CALYPSO_SECRETS_ENCRYPTION_KEY}" as we do in Circle (we need better documentation for this, or a separate item in the Makefile to generate these keys)
  2. export CONFIG_ENV=release
  3. make -f desktop/Makefile config
  4. yarn run build-desktop or make -f desktop/Makefile build to build the app

@nsakaimbo nsakaimbo force-pushed the desktop-manual-check-for-updates branch from edd66d9 to 96f1b49 Compare September 22, 2020 19:24
@nsakaimbo
Copy link
Contributor Author

Update: just opened PR #45856 that adds a convenience script to generate the app secrets.

Copy link
Contributor

@mokagio mokagio left a comment

Choose a reason for hiding this comment

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

Built on HEAD, got this

image

Changed the version to 5.0.0 and clicked "Check for Updates...", got this

image

So, all good 👍

A refinement suggestion for another day, maybe it was my network connection, but it took a while between me clicking the menu item and the sheet being presented. Maybe we could have a loading UI, to let the users know something is happening?

@mokagio
Copy link
Contributor

mokagio commented Sep 23, 2020

This is probably related to me opening the WordPress binary from the .dmg a few times and not closing it properly before trying out a new version, but I just noticed my Mac was super slow (the fans were also about to fly away, but I had my headphones on and didn't notice) and the reason turned out to be this

Screen Shot 2020-09-23 at 11 47 05 am

Look at the CPU load drop once I killed those "WordPress.com Helper (Renderer)" processes:

image

@nsakaimbo
Copy link
Contributor Author

nsakaimbo commented Sep 23, 2020

A refinement suggestion for another day, maybe it was my network connection, but it took a while between me clicking the menu item and the sheet being presented. Maybe we could have a loading UI, to let the users know something is happening?

This is a good observation and I had the same thought. As an improvement, hopefully there's some sort of spinner or loading-indicator element in Calypso we can use to provide the user more feedback while they wait.

@nsakaimbo
Copy link
Contributor Author

nsakaimbo commented Sep 23, 2020

Re: the CPU usage of the Desktop app - I haven't encountered this before, but I wouldn't be terribly surprised as Calypso (renderer process) itself is fairly heavy. Also interesting why in your case there were several renderer processes loaded. The app has logic that prevents multiple instances from being launched. 🤔 Definitely something we're going to want to take a look at, sooner rather than later.

@nsakaimbo nsakaimbo merged commit 547b7ee into desktop/release/6.1.0 Sep 23, 2020
@nsakaimbo nsakaimbo deleted the desktop-manual-check-for-updates branch September 23, 2020 14:40
@matticbot matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Sep 23, 2020
@mokagio
Copy link
Contributor

mokagio commented Sep 24, 2020

@nsakaimbo in regards to the update. If the Calypso route doesn't work, we could always do something native like Sparkle does:

2020-09-24 15-16-24 2020-09-24 15_18_43

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants