Migrate apps management to Vue.js #9565

juliusknorr · 2018-05-23T17:59:51Z

This PR will move the apps management to vue, heavily based on the work done by @skjnldsv in #8824

General improvements to the existing feature set:

Implement sidebar with app details Detail pages for apps #3193 Show app details in apps management via right sidebar #2138
Add search across categories Find an app by name within nextcloud #6868
Move ajax endpoints to proper Controller Move endpoints to from apps management apps to AppSettingsController #7931
Make categories enabled/disabled/update filters, to reduce request Settings Apps Redesign Overview #3194
Fix "Your apps" does not show available updates "Your apps" does not show available updates #8703
Avoid updating multiple apps at once Updating multiple apps at once results in some not being updated #7581

skjnldsv · 2018-05-24T05:26:43Z

Weeeeeeee! Let me know if you need help!
I'll probably move the new components to the repo as soon as I can :)

codecov · 2018-05-25T13:52:55Z

Codecov Report

Merging #9565 into master will decrease coverage by 1.37%.
The diff coverage is 14.45%.

@@             Coverage Diff              @@
##             master    #9565      +/-   ##
============================================
- Coverage     53.34%   51.96%   -1.38%     
- Complexity    24451    25743    +1292     
============================================
  Files          1478     1633     +155     
  Lines         81470    95346   +13876     
  Branches          0     1307    +1307     
============================================
+ Hits          43459    49551    +6092     
- Misses        38011    45795    +7784

Impacted Files	Coverage Δ	Complexity Δ
settings/js/apps.js	`3.44% <ø> (ø)`	`0 <0> (?)`
settings/routes.php	`0% <0%> (ø)`	`0 <0> (ø)`	⬇️
lib/private/legacy/app.php	`62.36% <0%> (-0.28%)`	`196 <0> (ø)`
lib/private/App/AppManager.php	`95.62% <100%> (+0.02%)`	`61 <0> (ø)`	⬇️
settings/Controller/AppSettingsController.php	`19.62% <15.09%> (-2.6%)`	`79 <75> (-10)`
.../tests/Unit/Collaboration/CommentersSorterTest.php	`25.55% <0%> (-66.45%)`	`6% <0%> (ø)`
apps/sharebymail/tests/SettingsTest.php	`52.17% <0%> (-47.83%)`	`3% <0%> (ø)`
lib/private/Security/RateLimiting/Limiter.php	`55.55% <0%> (-44.45%)`	`5% <0%> (ø)`
...b/private/App/AppStore/Fetcher/CategoryFetcher.php	`69.23% <0%> (-30.77%)`	`1% <0%> (ø)`
...ps/comments/tests/Unit/AppInfo/ApplicationTest.php	`69.56% <0%> (-30.44%)`	`4% <0%> (ø)`
... and 348 more

skjnldsv · 2018-05-25T22:14:06Z

settings/src/store/apps.js

+
+const mutations = {
+
+	APPS_API_FAILURE(state, error) {


Any reason not to use the global API_Failure?

I introduced this because the apps management controller endpoints are no OCS API at the moment, so the regular API_FAILURE will not work. I didn't move them to proper OCS since I wanted to keep backend code changes in the beginning. If nobody protests, i would tackle that in a follow up PR.

Guess who will earn a beer in Berlin if he makes that happen ;) 🍺

SO many beers :D

juliusknorr · 2018-06-02T08:17:42Z

Ready for review. @nextcloud/designers @nextcloud/javascript

rullzer · 2018-06-02T12:06:53Z

If I enable an app. The state is toggled in the sidebar. But not in the main view
I like the list overview (with the details). But a lot of the thumbnails in the list are to small to be meaningfull
For apps without a screenshot (and rating) the sidebar looks a bit off. Large space between the title and the author etc.

rullzer · 2018-06-02T12:03:06Z

settings/src/components/appDetails.vue

+		</div>
+
+		<p class="documentation">
+			<a class="appslink" :href="appstoreUrl" v-if="!app.internal" target="_blank" rel="noreferrer noopener">Im Store anzeigen ↗</a>


Lets write that in English ;)
And make it translatatble

ChristophWurst

Good stuff, but quite a big PR to review 🙈

I commented on a few spots that might need polishing.

ChristophWurst · 2018-06-04T06:10:24Z

lib/private/App/AppManager.php

@@ -37,6 +37,7 @@
 use OCP\App\IAppManager;
 use OCP\App\ManagerEvent;
 use OCP\ICacheFactory;
+use OCP\IConfig;


the import on L35 is unused then, I guess 😉

Right, removed :)

ChristophWurst · 2018-06-04T06:11:14Z

lib/private/App/AppManager.php

-		$this->appConfig->setValue($appId, 'enabled', 'no');
-
-		// run uninstall steps
-		$appData = $this->getAppInfo($appId);


was this removed intentionally?

ChristophWurst · 2018-06-04T06:12:29Z

lib/private/App/AppStore/Manager.php

+	}
+
+	public function getApp(string $appId) {
+		return $this->apps[$appId];


null checks, e.g. with ?? syntax would be great

ChristophWurst · 2018-06-04T06:17:22Z

settings/Controller/AppSettingsController.php

-		return new JSONResponse($this->getAllCategories());
+	public function listApps(): JSONResponse {
+
+		$this->fetchApps();


IMO this class should be split into multiple at some point. There's so much going on here and the code is quite hard to understand for me.

Toally with you on that. It will get a lot simpler once those methods are moved to proper OCS endpoints, but I'd do that in a follow up PR as mentioned in #9565 (comment)

ChristophWurst · 2018-06-04T06:19:58Z

settings/src/store/users.js

@@ -197,6 +197,21 @@ const actions = {
 			.catch((error) => context.commit('API_FAILURE', error));
 	},

+	getGroups(context) { /* { offset, limit, search } */


what do these comments mean?

I think it's a removed ES6 destructuring part of the function that was set aside?

juliusknorr · 2018-06-05T16:03:10Z

If I enable an app. The state is toggled in the sidebar. But not in the main view
@rullzer Cannot reproduce that. Can you provide some steps how and which enable button you have clicked?

I like the list overview (with the details). But a lot of the thumbnails in the list are to small to be meaningfull

I've changed the behavior there, so we always show the app icon in the list view.

All other comments should be fixed.

rullzer

I like it!

Lets get this in. And fix tiny things later if we find them!

rullzer · 2018-06-05T19:19:53Z

CI not happy

Signed-off-by: Julius Härtl <[email protected]> Move app management ajax code to AppSettingsController Signed-off-by: Julius Härtl <[email protected]>

Signed-off-by: Julius Härtl <[email protected]>

juliusknorr · 2018-06-06T12:40:40Z

@rullzer Fixed the CI failures. 😉

MorrisJobke

Tested and works 👍

skjnldsv · 2018-06-06T16:07:33Z

🎉 ❤️

juliusknorr added 2. developing Work in progress feature: apps management labels May 23, 2018

juliusknorr added this to the Nextcloud 14 milestone May 23, 2018

juliusknorr added the enhancement label May 23, 2018

juliusknorr force-pushed the feature/noid/app-settings branch from 7a59cf0 to a40d969 Compare May 25, 2018 13:52

skjnldsv reviewed May 25, 2018

View reviewed changes

juliusknorr force-pushed the feature/noid/app-settings branch 2 times, most recently from 7704564 to 4c27804 Compare May 30, 2018 15:13

juliusknorr added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jun 2, 2018

juliusknorr requested review from rullzer, skjnldsv, MorrisJobke, ChristophWurst and danxuliu June 2, 2018 08:16

juliusknorr force-pushed the feature/noid/app-settings branch from e90021c to 1340a58 Compare June 2, 2018 08:17

rullzer requested changes Jun 2, 2018

View reviewed changes

ChristophWurst reviewed Jun 4, 2018

View reviewed changes

juliusknorr force-pushed the feature/noid/app-settings branch 2 times, most recently from a5fe179 to 36ce686 Compare June 5, 2018 15:48

rullzer approved these changes Jun 5, 2018

View reviewed changes

juliusknorr added 3 commits June 6, 2018 11:40

Move to AppFramework Controller

8594fdc

Signed-off-by: Julius Härtl <[email protected]> Move app management ajax code to AppSettingsController Signed-off-by: Julius Härtl <[email protected]>

Add file skeleton for app settings

125d1d3

Signed-off-by: Julius Härtl <[email protected]>

Remove default categories from route endpoint

d9d5e97

Signed-off-by: Julius Härtl <[email protected]>

juliusknorr added 6 commits June 6, 2018 11:40

Remove unneeded class

29f1c9b

Signed-off-by: Julius Härtl <[email protected]>

Make view in store text translatable

9d6aa33

Signed-off-by: Julius Härtl <[email protected]>

Add search/offset/limit parameters to group fetching

523cd0a

Signed-off-by: Julius Härtl <[email protected]>

Add bundled code

0088326

Signed-off-by: Julius Härtl <[email protected]>

Remove unused import

facce73

Signed-off-by: Julius Härtl <[email protected]>

Remove space if no rating is available

c30af9b

Signed-off-by: Julius Härtl <[email protected]>

juliusknorr force-pushed the feature/noid/app-settings branch from edece7c to c97d6d5 Compare June 6, 2018 09:44

juliusknorr added 2 commits June 6, 2018 11:49

Keep AppConfig for now

f30c14a

Signed-off-by: Julius Härtl <[email protected]>

Fix AppSettingsControllerTest

23ca2a4

Signed-off-by: Julius Härtl <[email protected]>

juliusknorr force-pushed the feature/noid/app-settings branch from c97d6d5 to 23ca2a4 Compare June 6, 2018 09:49

Remove old app settings test

bf03d43

Signed-off-by: Julius Härtl <[email protected]>

MorrisJobke approved these changes Jun 6, 2018

View reviewed changes

MorrisJobke merged commit b49f8e4 into master Jun 6, 2018

MorrisJobke deleted the feature/noid/app-settings branch June 6, 2018 16:05

skjnldsv added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Jun 6, 2018

skjnldsv mentioned this pull request Jun 6, 2018

Vue apps issues #9767

Closed

7 tasks

juliusknorr mentioned this pull request Jun 6, 2018

Missing svg filter mixin for apps management #9771

Merged

This was referenced Jun 18, 2018

Appstore: allow search across all categories #9898

Closed

Search-as-you-type in apps management does not work anymore #9959

Closed

Apps-Management doesn't show missing dependencies #4581

Closed

klawdhfzasjhaa mentioned this pull request Sep 20, 2019

Add Release notes Infobutton to Apps page #6684

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate apps management to Vue.js #9565

Migrate apps management to Vue.js #9565

juliusknorr commented May 23, 2018 •

edited

Loading

skjnldsv commented May 24, 2018

codecov bot commented May 25, 2018 •

edited

Loading

skjnldsv May 25, 2018

juliusknorr May 30, 2018

rullzer May 30, 2018

skjnldsv Jun 5, 2018

juliusknorr commented Jun 2, 2018

rullzer commented Jun 2, 2018

rullzer Jun 2, 2018

ChristophWurst left a comment

ChristophWurst Jun 4, 2018

juliusknorr Jun 5, 2018

ChristophWurst Jun 4, 2018

ChristophWurst Jun 4, 2018

ChristophWurst Jun 4, 2018

juliusknorr Jun 5, 2018

ChristophWurst Jun 4, 2018

skjnldsv Jun 4, 2018

juliusknorr commented Jun 5, 2018

rullzer left a comment

rullzer commented Jun 5, 2018

juliusknorr commented Jun 6, 2018

MorrisJobke left a comment

skjnldsv commented Jun 6, 2018 •

edited

Loading

Migrate apps management to Vue.js #9565

Migrate apps management to Vue.js #9565

Conversation

juliusknorr commented May 23, 2018 • edited Loading

skjnldsv commented May 24, 2018

codecov bot commented May 25, 2018 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

juliusknorr commented Jun 2, 2018

rullzer commented Jun 2, 2018

Choose a reason for hiding this comment

ChristophWurst left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

juliusknorr commented Jun 5, 2018

rullzer left a comment

Choose a reason for hiding this comment

rullzer commented Jun 5, 2018

juliusknorr commented Jun 6, 2018

MorrisJobke left a comment

Choose a reason for hiding this comment

skjnldsv commented Jun 6, 2018 • edited Loading

juliusknorr commented May 23, 2018 •

edited

Loading

codecov bot commented May 25, 2018 •

edited

Loading

skjnldsv commented Jun 6, 2018 •

edited

Loading