Refactor the app to allow choosing between next minor and major update #391

VicDeo · 2018-08-28T16:01:21Z

Summary

Core upgrade (this part depends on owncloud/core#32491 that is still WIP)

major (10 to 11 and like this) still DOES upgrade apps from 1.0 to 2.0 (if available)
minor (10 to 10.1 and like this) does NOT apps upgrade from 1.0 to 2.0

CLI

market:upgrade still DOES upgrade 1.0 to 1.1
market:upgrade does NOT upgrade 1.0 to 2.0
market:upgrade --major DOES upgrade 1.0 to 2.0
market:install does NOT upgrade 1.0 to 2.0 if the app is already installed
market:install still DOES upgrade 1.0 to 1.1 if the app is already installed

Market app Web UI

Shows both 1.1 and 2.0 for 1.0, 1.1 is default

Market app cron job

Generates notifications both for 1.1 and 2.0

Screenshots

Updates tab

App Page

TODO

add --major option to CLI market:upgrade command
do not allow update to major releases with CLI market:install command
switch to major version of the app only in case there is a core major update is happening
(Depends on Pass an additional parameter on the core update core#32491)
Update app UI to show both minor and major updates
Allow update both to major and minor versions from UI

0.3.5 to 0.3.5

deo@jah-mobile:> php occ market:upgrade --local ../customgroups.tar.gz 
customgroups: ../customgroups.tar.gz has the same or older version of the app

0.3.5 to 1.3.5 (no --major)

deo@jah-mobile:> php occ market:upgrade --local ../customgroups.tar.gz 
customgroups: ../customgroups.tar.gz has a different major version, try with --major option

0.3.5 to 1.3.5 (with --major)

deo@jah-mobile:> php occ market:upgrade --local ../customgroups.tar.gz --major
customgroups: Installing new version from ../customgroups.tar.gz

after the merge

Documentation needs to be updated

codecov-io · 2018-08-28T16:13:20Z

Codecov Report

Merging #391 into master will increase coverage by 11.81%.
The diff coverage is 59.46%.

@@              Coverage Diff              @@
##             master     #391       +/-   ##
=============================================
+ Coverage     41.03%   52.84%   +11.81%     
- Complexity      229      278       +49     
=============================================
  Files            15       17        +2     
  Lines           775      931      +156     
=============================================
+ Hits            318      492      +174     
+ Misses          457      439       -18

Impacted Files	Coverage Δ	Complexity Δ
lib/Listener.php	`0% <0%> (ø)`	`4 <2> (ø)`	⬇️
lib/Application.php	`0% <0%> (ø)`	`11 <0> (+2)`	⬆️
lib/Controller/MarketController.php	`0% <0%> (ø)`	`42 <6> (+4)`	⬆️
lib/Command/UpgradeApp.php	`98.11% <100%> (+0.93%)`	`26 <6> (+8)`	⬆️
lib/Command/InstallApp.php	`97.5% <100%> (+0.67%)`	`19 <4> (+4)`	⬆️
lib/HttpService.php	`47.95% <47.95%> (ø)`	`29 <29> (?)`
lib/MarketService.php	`52.23% <59.87%> (+28.83%)`	`90 <62> (-10)`	⬇️
lib/VersionHelper.php	`72% <72%> (ø)`	`10 <10> (?)`
lib/CheckUpdateBackgroundJob.php	`98.21% <75%> (-1.79%)`	`15 <0> (+2)`
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eae34a3...79d1a44. Read the comment docs.

lib/MarketService.php

PVince81 · 2018-08-29T07:42:31Z

we'll also need the --major for occ market:update which I guess will use the same methods ?

VicDeo · 2018-10-23T20:44:32Z

Build is failing due to a version bump in core:

App "Market" cannot be installed because the following dependencies are not
fulfilled: ownCloud 10.1 or lower is required.

PVince81 · 2018-10-24T08:25:54Z

set max-version to 11.0.0.0 for now

VicDeo · 2018-10-24T15:54:56Z

given that I am not an
UI expert I appreciate if someone explains how this pages should look like if we have both minor an major updates...

Updates

App page

PVince81 · 2018-10-26T15:07:16Z

@felixheidecke any thoughts ?

We could split the "Update info" column into three:

Installed version
Minor update target
Major update target

Might then need two buttons, or make the update target clickable to select which one to update to.

Or a dropdown to select the version ?

PVince81 · 2018-10-26T15:07:48Z

and now I realize that whatever the UX will be it will have an impact on the info that the controller endpoints must return...

VicDeo · 2018-10-28T21:12:39Z

@PVince81 it has even more impact than you can imagine ;)
market:upgrade --list
updated to the following output

deo@jah-mobile:~> php occ market:upgrade --list
admin_audit : minor:0.9.0, major:1.0.1
enterprise_key : minor:0.1.4
onlyoffice : minor:1.4.0, major:2.0.3
theme-enterprise : major:2.0.2
windows_network_drive : minor:0.7.1

I managed to pass both versions to frontend and display them

Imo for two candidates the right side should mutate into the HTML select element

PVince81 · 2018-10-29T13:54:01Z

@VicDeo go for it :-D

Hint: https://getuikit.com/v2/docs/dropdown.html

VicDeo · 2018-10-29T18:07:22Z

@PVince81 another questionable thing is market:install command. It updates the app if it is already installed.
As market:install --major looks pretty non-sense IMHO it should refuse to operate on major versions. what do you think?

PVince81 · 2018-10-30T14:38:05Z

@VicDeo if market:install is only doing installations and not updates, then the switch --major doesn't make sense as there is no local version. Installing an app should always return the latest version I guess.

Is there a possibility to specify a version number to install like with Linux package managers ?

VicDeo · 2018-10-30T18:28:31Z

@PVince81 >Is there a possibility to specify a version number to install like with Linux package managers ?

no, it installs the most recent, or updates to the most recent if the app is already installed.

VicDeo · 2018-11-05T20:33:23Z

@PVince81 it's hard to say as I still suck in the frontend part :(

PVince81 · 2018-11-05T20:33:57Z

@VicDeo you can get some advice from @felixheidecke in the chat

VicDeo · 2018-11-05T21:11:53Z

@PVince81 @felixheidecke I updated the PR description with a new look for Updates tab

I appreciate the advice on the app details tab given that both releases could have missing dependencies section here...

VicDeo · 2018-11-05T21:13:18Z

What would be correct to use from the point of UI here, radiobutton?

felixheidecke · 2018-11-06T08:41:02Z

What would be correct to use from the point of UI here, radiobutton?

I am missing the context here. Is it in regard to the screenshot above? What is to be chosen?

VicDeo · 2018-11-06T08:43:13Z

@felixheidecke the context is providing two versions for update: minor and major,
see #387 for more details

felixheidecke · 2018-11-06T08:50:56Z

The dropdowns in the screenshot in the very first post look good. Don't they suffice?

PVince81

Looks good in general. See a few comments.

lib/Command/InstallApp.php

lib/HttpService.php

tests/unit/HttpServiceTest.php

VicDeo · 2018-11-13T16:40:12Z

@PVince81 answered everywhere

PVince81

👍

davitol · 2018-11-22T09:22:20Z

In this scenario:

Do we feel OK with the text shown? Is it clear enough for an admin?

password_policy: No compatible version for password_policy

(cc @pmaier1 )

pmaier1 · 2018-11-22T12:29:47Z

Do we feel OK with the text shown? Is it clear enough for an admin?

When exactly is this message shown? Only when there's a requirement for --major to be able to upgrade the app or also when there is no later version available at all or when the version is simply not compatible with the core version (not because of a major app update)? Anything else?

PVince81 · 2018-11-23T10:12:56Z

let's clarify UX and messages in #416

VicDeo added the 2 - Developing label Aug 28, 2018

VicDeo added this to the development milestone Aug 28, 2018

VicDeo self-assigned this Aug 28, 2018

VicDeo force-pushed the minor-first branch from ecf5b70 to 722acf3 Compare August 28, 2018 16:49

VicDeo mentioned this pull request Aug 28, 2018

Differentiate between minor and major updates #387

Closed

7 tasks

PVince81 reviewed Aug 29, 2018

View reviewed changes

lib/MarketService.php Outdated Show resolved Hide resolved

VicDeo force-pushed the minor-first branch from 722acf3 to b9b901e Compare October 23, 2018 18:20

VicDeo added 2 commits October 24, 2018 18:56

Process an additional parameter on core update

937aad0

Add --major to market:upgrade CLI command

d0c2188

VicDeo force-pushed the minor-first branch from 7c0fa2d to d0c2188 Compare October 24, 2018 15:56

Extract http requests into a separate class

5e8f01f

VicDeo force-pushed the minor-first branch 2 times, most recently from e6931d6 to 473c395 Compare October 29, 2018 17:24

VicDeo force-pushed the minor-first branch from 473c395 to be1bd8c Compare October 29, 2018 19:15

VicDeo force-pushed the minor-first branch from be1bd8c to 1b2b334 Compare October 30, 2018 15:47

VicDeo force-pushed the minor-first branch 3 times, most recently from 075fc2e to a5663c6 Compare November 1, 2018 21:23

VicDeo force-pushed the minor-first branch from 5d279ee to 8d67110 Compare November 5, 2018 20:45

VicDeo added 2 commits November 9, 2018 16:15

Extract both major and minor versions at once

35de515

Reorder and document methods

3af0af8

VicDeo force-pushed the minor-first branch from b7c1364 to 367a019 Compare November 9, 2018 13:15

VicDeo added 3 - To Review and removed 2 - Developing labels Nov 9, 2018

VicDeo changed the title ~~Process an additional parameter on core update~~ Refactor the app to allow choose between next minor and major update Nov 9, 2018

VicDeo changed the title ~~Refactor the app to allow choose between next minor and major update~~ Refactor the app to allow choosing between next minor and major update Nov 9, 2018

PVince81 reviewed Nov 9, 2018

View reviewed changes

lib/Command/InstallApp.php Show resolved Hide resolved

lib/HttpService.php Show resolved Hide resolved

tests/unit/HttpServiceTest.php Outdated Show resolved Hide resolved

tests/unit/HttpServiceTest.php Show resolved Hide resolved

VicDeo and others added 3 commits November 12, 2018 18:37

Extract dealing with versions

9a9945f

Allow selection of update version in details view

bdc20ce

Allow updating to the specified version from WebUI

79d1a44

VicDeo force-pushed the minor-first branch from 503c38c to 79d1a44 Compare November 12, 2018 15:37

PVince81 approved these changes Nov 16, 2018

View reviewed changes

PVince81 merged commit 9c374dc into master Nov 16, 2018

PVince81 deleted the minor-first branch November 16, 2018 10:52

PVince81 mentioned this pull request Nov 16, 2018

UX review minor/major #416

Open

PVince81 modified the milestones: development, QA Nov 22, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor the app to allow choosing between next minor and major update #391

Refactor the app to allow choosing between next minor and major update #391

VicDeo commented Aug 28, 2018 •

edited

Loading

codecov-io commented Aug 28, 2018 •

edited by codecov bot

Loading

PVince81 commented Aug 29, 2018

VicDeo commented Oct 23, 2018

PVince81 commented Oct 24, 2018

VicDeo commented Oct 24, 2018 •

edited

Loading

PVince81 commented Oct 26, 2018

PVince81 commented Oct 26, 2018

VicDeo commented Oct 28, 2018

PVince81 commented Oct 29, 2018

VicDeo commented Oct 29, 2018

PVince81 commented Oct 30, 2018

VicDeo commented Oct 30, 2018 •

edited

Loading

VicDeo commented Nov 5, 2018

PVince81 commented Nov 5, 2018

VicDeo commented Nov 5, 2018 •

edited

Loading

VicDeo commented Nov 5, 2018

felixheidecke commented Nov 6, 2018

VicDeo commented Nov 6, 2018

felixheidecke commented Nov 6, 2018

PVince81 left a comment

VicDeo commented Nov 13, 2018

PVince81 left a comment

davitol commented Nov 22, 2018

pmaier1 commented Nov 22, 2018 •

edited

Loading

PVince81 commented Nov 23, 2018

Refactor the app to allow choosing between next minor and major update #391

Refactor the app to allow choosing between next minor and major update #391

Conversation

VicDeo commented Aug 28, 2018 • edited Loading

Summary

Core upgrade (this part depends on owncloud/core#32491 that is still WIP)

CLI

Market app Web UI

Market app cron job

Screenshots

Updates tab

App Page

TODO

after the merge

codecov-io commented Aug 28, 2018 • edited by codecov bot Loading

Codecov Report

PVince81 commented Aug 29, 2018

VicDeo commented Oct 23, 2018

PVince81 commented Oct 24, 2018

VicDeo commented Oct 24, 2018 • edited Loading

PVince81 commented Oct 26, 2018

PVince81 commented Oct 26, 2018

VicDeo commented Oct 28, 2018

PVince81 commented Oct 29, 2018

VicDeo commented Oct 29, 2018

PVince81 commented Oct 30, 2018

VicDeo commented Oct 30, 2018 • edited Loading

VicDeo commented Nov 5, 2018

PVince81 commented Nov 5, 2018

VicDeo commented Nov 5, 2018 • edited Loading

VicDeo commented Nov 5, 2018

felixheidecke commented Nov 6, 2018

VicDeo commented Nov 6, 2018

felixheidecke commented Nov 6, 2018

PVince81 left a comment

Choose a reason for hiding this comment

VicDeo commented Nov 13, 2018

PVince81 left a comment

Choose a reason for hiding this comment

davitol commented Nov 22, 2018

pmaier1 commented Nov 22, 2018 • edited Loading

PVince81 commented Nov 23, 2018

VicDeo commented Aug 28, 2018 •

edited

Loading

codecov-io commented Aug 28, 2018 •

edited by codecov bot

Loading

VicDeo commented Oct 24, 2018 •

edited

Loading

VicDeo commented Oct 30, 2018 •

edited

Loading

VicDeo commented Nov 5, 2018 •

edited

Loading

pmaier1 commented Nov 22, 2018 •

edited

Loading