Add app bundles to the apps page and unbundle enterprise apps #4454

LukasReschke · 2017-04-23T20:19:34Z

This adds the app bundles to the apps page and introduces the internal classes as a first step to getting there.

Bundles are currently hard-coded and will in the future likely be delivered by the AppStore as well. But for now, let's keep it simple. The problem is that if we go that way we need some cryptographic signing to prevent an attacker creating malicious bundles with their app. Otherwise, they could just upload a malicious app with their signature and replace the pre-defined bundles.

If an admin chooses a bundle the apps in the bundle will be downloaded (if necessary), installed and enabled. So right now I've created the "Enterprise" bundle which showcases local apps (and also discovered an independent bug: #4453) and a "Groupware" one which downloads the calendar and contacts app.

Todos

… some todos that I've written down might also be moved to later PRs:

Adjust and write new tests
Have @jospoortvliet and @karlitschek come up with sane bundles and descriptions
Make @jancborchardt fix the design
Check if it makes sense to add a OCC method to install and show bundles
Check if it makes sense to add this to the internal Nextcloud app store interface as well

mention-bot · 2017-04-23T20:19:36Z

@LukasReschke, thanks for your PR! By analyzing the history of the files in this pull request, we identified @MorrisJobke, @icewind1991 and @bartv2 to be potential reviewers.

LukasReschke · 2017-04-23T20:28:17Z

lib/private/Installer.php

 		$this->appFetcher = $appFetcher;
+		$this->appManager = $appManager;


I will remove that again because the AppManager method is broken as described at #4453

nickvergessen · 2017-04-24T13:52:09Z

lib/private/App/AppStore/Bundles/Bundle.php

+	 *
+	 * @return array
+	 */
+	public  abstract function getAppIdentifiers();


Double space hurts my eyes

Hehe ;-) – Resolved.

LukasReschke · 2017-04-25T13:39:08Z

Will adjust based on the just merged #3195

codecov · 2017-04-25T16:35:17Z

Codecov Report

Merging #4454 into master will decrease coverage by 16.4%.
The diff coverage is 7.44%.

@@             Coverage Diff              @@
##             master   #4454       +/-   ##
============================================
- Coverage      54.1%   37.7%   -16.41%     
- Complexity    21716   21872      +156     
============================================
  Files          1334    1349       +15     
  Lines         83213   83870      +657     
  Branches       1315    1333       +18     
============================================
- Hits          45025   31625    -13400     
- Misses        38188   52245    +14057

Impacted Files	Coverage Δ	Complexity Δ
lib/private/Repair.php	`0% <0%> (-30.99%)`	`19 <0> (ø)`
settings/ajax/updateapp.php	`0% <0%> (ø)`	`0 <0> (ø)`	⬇️
settings/ajax/disableapp.php	`0% <0%> (ø)`	`0 <0> (ø)`	⬇️
lib/private/Repair/NC12/InstallCoreBundle.php	`0% <0%> (ø)`	`6 <6> (?)`
lib/private/App/AppStore/Bundles/CoreBundle.php	`0% <0%> (ø)`	`2 <2> (?)`
lib/base.php	`3.09% <0%> (-0.01%)`	`165 <0> (+1)`
lib/private/Updater.php	`0% <0%> (-5.95%)`	`73 <0> (ø)`
...b/private/App/AppStore/Bundles/GroupwareBundle.php	`0% <0%> (ø)`	`2 <2> (?)`
.../private/App/AppStore/Bundles/EnterpriseBundle.php	`0% <0%> (ø)`	`2 <2> (?)`
settings/ajax/enableapp.php	`0% <0%> (ø)`	`0 <0> (ø)`	⬇️
... and 500 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 255c7df...de25352. Read the comment docs.

LukasReschke · 2017-04-25T16:35:37Z

core/Controller/SetupController.php

@@ -39,7 +39,7 @@ class SetupController {
 	/**
 	 * @param Setup $setupHelper
 	 */
-	function __construct(Setup $setupHelper) {
+	public function __construct(Setup $setupHelper) {


LukasReschke · 2017-04-25T16:35:51Z

lib/private/App/AppStore/Bundles/Bundle.php

+	 *
+	 * @return array
+	 */
+	public  abstract function getAppIdentifiers();


Hehe ;-) – Resolved.

LukasReschke · 2017-04-25T17:01:21Z

I did some more work on this, this implements now app bundling as well as the app unbundling.

To sum it up:

The apps "user_saml", "files_retention", "files_automatedtagging" and "files_accesscontrol" have been removed from the build script for 12 and uploaded to the app store: (https://apps.nextcloud.com/?order_by=last_release&ordering=desc). If the app has been installed before they will be downloaded automatically on the update.
There is a "core" bundle which is installed by default and consists of the new https://apps.nextcloud.com/apps/bruteforcesettings app. This bundle is not visible to the end-user.
The app management interface offers users to install the following two bundles:
- "Enterprise bundle" (admin_audit, user_ldap, files_retention, files_automatedtagging, user_saml, files_accesscontrol)
- "Groupware bundle" (calendar, contacts)

I'm not entirely happy with the interface yet and also it's kinda weird that you can only "enable" bundles but well, it works and enhancements are of course welcome.

Also, we should make sure someone comes up with nice screenshots and descriptions for the new apps at https://apps.nextcloud.com/?order_by=last_release&ordering=desc.

Feedback on the code and the implementation is already appreciated, meanwhile I'll clean up the source code a tad more :-)

LukasReschke · 2017-04-25T17:06:40Z

lib/private/App/AppStore/Bundles/EnterpriseBundle.php

+	 * {@inheritDoc}
+	 */
+	public function getDescription() {
+		return (string)$this->l10n->t('Apps for the Enterprise.');


Needs some marketing text.

cc @jospoortvliet

LukasReschke · 2017-04-25T17:07:01Z

lib/private/App/AppStore/Bundles/GroupwareBundle.php

+	 * {@inheritDoc}
+	 */
+	public function getDescription() {
+		return (string)$this->l10n->t('Apps for groupware functionalities.');


Needs some marketing text.

cc @jospoortvliet

karlitschek · 2017-04-25T17:13:55Z

Nice! The UI could use some love. :-) We definitely should also list the apps that are part of this bundle for transparency. Maybe screenshots,logos or something else graphical? @jancborchardt What do you think?
The core bundle is a good idea. As said before I'm not sure everyone will like it that code is automatically on install. But we can try.
Beside that looks good!
The UI in the setup wizard also needs some UI love.

LukasReschke · 2017-04-25T17:15:49Z

The UI in the setup wizard also needs some UI love.

I removed the UI from the installer again. Right now the only thing is in the admin page, should I add it again? I don't have a big preference. Hehe 😉
(adding it is around ~50 lines of code so that would also not be big magic 😉 )

karlitschek · 2017-04-25T17:40:15Z

I think it is ok to have it only on the apps page. as long as it looks nice and people find it. ;-) ;-)

jancborchardt · 2017-04-25T17:50:06Z

@LukasReschke layout-wise I was thinking to use the appstore-style layout like so?

Basically not using the .installed class, and then simply putting h2 inbetween the apps, with an »Enable all« button there. That way the individual apps below are also advertised and it’s very visible what gets enabled. Instead of a very short list of 3 bundles. ;)

<h2>Enterprise app bundle <button>Enable all</button></h2>

I assume you need to push code around before I can do detail layout fixes?

LukasReschke · 2017-04-25T18:18:49Z

@jancborchardt No idea how to do that in CSS and the JS that I've adjusted for that is also kinda horrible but here we go: 35665da

Can you modify 35665da#diff-008343a6a32a47c315bd61fa9d9a2f3dR76 until it looks good? Also add an "enable" button please, I'll then make sure that this one also will work ;)

LukasReschke · 2017-04-25T18:19:41Z

Basically, I have no idea how to do a sane newline here and all the other fancy stuff 😉

MorrisJobke · 2017-04-25T19:07:58Z

If the app has been installed before they will be downloaded automatically on the update.

What if the app store is not available?

Enterprise bundle" (admin_audit, user_ldap, files_retention, files_automatedtagging, user_saml, files_accesscontrol)

Could some apps be removed from the bundle? Because user_ldap and user_saml together makes sense only in very few cases.

LukasReschke · 2017-04-25T19:08:00Z

Layout is adjusted as discussed with @jancborchardt.

LukasReschke · 2017-04-25T19:11:31Z

What if the app store is not available?

Well. Then you need to download the app manually. That's life. Also moving some of the enterprise apps to the app store has the same conclusion: Someone needs to manually upload the apps. 😉
(which they also can script if they really want to, and if they are a customer of ours we can offer them an additional fully bundled TAR on our customer portal)

That said, nowadays I believe having the web root writable is actually rather good also since we want to move to automatic updates in the future…

Could some apps be removed from the bundle? Because user_ldap and user_saml together makes sense only in very few cases.

Talk to @karlitschek and @jospoortvliet about that ;) – I don't have big preferences :)

LukasReschke · 2017-04-25T19:24:08Z

lib/private/App/AppStore/Bundles/Bundle.php

+	 *
+	 * @return string
+	 */
+	public abstract function getDescription();


Let's kill that as the UI doesn't show it anyways.

jancborchardt · 2017-04-26T12:50:36Z

@eppfel I would say tho best do that in a separate pull request. This is mergeable as of the current state, which we should do and then any adjustments can be made as a follow-up.

So hopefully we can merge this before that meetup. :)

eppfel · 2017-04-26T12:56:04Z

Sure...

jancborchardt · 2017-04-26T16:43:58Z

@LukasReschke so here we wait for the Drone tests?

LukasReschke · 2017-04-26T16:45:58Z

@LukasReschke so here we wait for the Drone tests?

Yeah. If some fail we then check why and merge if they are unrelated. The joy of not having enough CI hardware 😉

Signed-off-by: Lukas Reschke <lukas@statuscode.ch>

Signed-off-by: Jan-Christoph Borchardt <hey@jancborchardt.net>

Signed-off-by: Lukas Reschke <lukas@statuscode.ch>

Signed-off-by: Jan-Christoph Borchardt <hey@jancborchardt.net>

Signed-off-by: Lukas Reschke <lukas@statuscode.ch>

MorrisJobke · 2017-04-26T20:45:55Z

I will merge this once CI is fine.

LukasReschke added the 2. developing Work in progress label Apr 23, 2017

LukasReschke added this to the Nextcloud 12.0 milestone Apr 23, 2017

LukasReschke requested review from rullzer and jancborchardt April 23, 2017 20:19

LukasReschke commented Apr 23, 2017

View reviewed changes

nickvergessen mentioned this pull request Apr 24, 2017

Nextcloud 12 overview #2672

Closed

59 tasks

nickvergessen reviewed Apr 24, 2017

View reviewed changes

LukasReschke assigned jancborchardt Apr 24, 2017

LukasReschke force-pushed the add-bundles-to-install-page branch from 0de0344 to b4a67d9 Compare April 25, 2017 16:35

LukasReschke force-pushed the add-bundles-to-install-page branch from b4a67d9 to 2d3b2dd Compare April 25, 2017 16:38

LukasReschke commented Apr 25, 2017

View reviewed changes

LukasReschke force-pushed the add-bundles-to-install-page branch from 2d3b2dd to 48a4504 Compare April 25, 2017 17:01

LukasReschke changed the title ~~Add app bundles to the install wizard~~ Add app bundles to the apps page and unbundle enterprise apps Apr 25, 2017

LukasReschke commented Apr 25, 2017

View reviewed changes

LukasReschke force-pushed the add-bundles-to-install-page branch from cb56200 to 56070a8 Compare April 25, 2017 19:26

LukasReschke unassigned jancborchardt Apr 25, 2017

LukasReschke added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Apr 26, 2017

LukasReschke and others added 18 commits April 26, 2017 20:07

Add app bundles

47cd976

Signed-off-by: Lukas Reschke <lukas@statuscode.ch>

Add titles

4b1ac3f

Signed-off-by: Lukas Reschke <lukas@statuscode.ch>

Don't sort on app bundles

0428254

Signed-off-by: Lukas Reschke <lukas@statuscode.ch>

Fix styling

da67264

Signed-off-by: Lukas Reschke <lukas@statuscode.ch>

Make disableapp.php accept arrays

732c92e

Signed-off-by: Lukas Reschke <lukas@statuscode.ch>

Make the emergency disable also accept arrays

d4ba819

Signed-off-by: Lukas Reschke <lukas@statuscode.ch>

Adjust tests

3f9aaac

Signed-off-by: Lukas Reschke <lukas@statuscode.ch>

Remove unrequired getDescription() method

78d0bb9

Signed-off-by: Lukas Reschke <lukas@statuscode.ch>

Add tests for bundle code

0651d66

Signed-off-by: Lukas Reschke <lukas@statuscode.ch>

Add spreed to Groupware bundle

a05295f

Signed-off-by: Lukas Reschke <lukas@statuscode.ch>

Add tests for repairstep

0c5a48c

Signed-off-by: Lukas Reschke <lukas@statuscode.ch>

Styling fixes to bundle header

b2c16b0

Signed-off-by: Jan-Christoph Borchardt <hey@jancborchardt.net>

Use .installed layout

b9ebdba

Signed-off-by: Lukas Reschke <lukas@statuscode.ch>

Style app bundles

1da371b

Signed-off-by: Jan-Christoph Borchardt <hey@jancborchardt.net>

Fix bundles header position

79c706c

Signed-off-by: Jan-Christoph Borchardt <hey@jancborchardt.net>

Add SocialSharingBundle

3df99d8

Signed-off-by: Lukas Reschke <lukas@statuscode.ch>

Update autoload map

d17a901

Signed-off-by: Lukas Reschke <lukas@statuscode.ch>

Fix tests

d0e0bc5

Signed-off-by: Lukas Reschke <lukas@statuscode.ch>

LukasReschke force-pushed the add-bundles-to-install-page branch from 8c461eb to d0e0bc5 Compare April 26, 2017 18:08

rullzer approved these changes Apr 26, 2017

View reviewed changes

MorrisJobke merged commit aad0794 into master Apr 26, 2017

MorrisJobke deleted the add-bundles-to-install-page branch April 26, 2017 21:20

MariusBluem mentioned this pull request Apr 28, 2017

Show default_enabled apps in apps management #4580

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add app bundles to the apps page and unbundle enterprise apps #4454

Add app bundles to the apps page and unbundle enterprise apps #4454

LukasReschke commented Apr 23, 2017 •

edited

Loading

mention-bot commented Apr 23, 2017

LukasReschke Apr 23, 2017

nickvergessen Apr 24, 2017

LukasReschke Apr 25, 2017

LukasReschke commented Apr 25, 2017

codecov bot commented Apr 25, 2017 •

edited

Loading

LukasReschke Apr 25, 2017

LukasReschke Apr 25, 2017

LukasReschke commented Apr 25, 2017 •

edited

Loading

LukasReschke Apr 25, 2017

LukasReschke Apr 25, 2017

karlitschek commented Apr 25, 2017

LukasReschke commented Apr 25, 2017

karlitschek commented Apr 25, 2017

jancborchardt commented Apr 25, 2017

LukasReschke commented Apr 25, 2017

LukasReschke commented Apr 25, 2017

MorrisJobke commented Apr 25, 2017 •

edited

Loading

LukasReschke commented Apr 25, 2017

LukasReschke commented Apr 25, 2017

LukasReschke Apr 25, 2017

jancborchardt commented Apr 26, 2017

eppfel commented Apr 26, 2017

jancborchardt commented Apr 26, 2017

LukasReschke commented Apr 26, 2017

MorrisJobke commented Apr 26, 2017

		$this->appFetcher = $appFetcher;
		$this->appManager = $appManager;

Add app bundles to the apps page and unbundle enterprise apps #4454

Add app bundles to the apps page and unbundle enterprise apps #4454

Conversation

LukasReschke commented Apr 23, 2017 • edited Loading

Todos

mention-bot commented Apr 23, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LukasReschke commented Apr 25, 2017

codecov bot commented Apr 25, 2017 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LukasReschke commented Apr 25, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

karlitschek commented Apr 25, 2017

LukasReschke commented Apr 25, 2017

karlitschek commented Apr 25, 2017

jancborchardt commented Apr 25, 2017

LukasReschke commented Apr 25, 2017

LukasReschke commented Apr 25, 2017

MorrisJobke commented Apr 25, 2017 • edited Loading

LukasReschke commented Apr 25, 2017

LukasReschke commented Apr 25, 2017

Choose a reason for hiding this comment

jancborchardt commented Apr 26, 2017

eppfel commented Apr 26, 2017

jancborchardt commented Apr 26, 2017

LukasReschke commented Apr 26, 2017

MorrisJobke commented Apr 26, 2017

LukasReschke commented Apr 23, 2017 •

edited

Loading

codecov bot commented Apr 25, 2017 •

edited

Loading

LukasReschke commented Apr 25, 2017 •

edited

Loading

MorrisJobke commented Apr 25, 2017 •

edited

Loading