Fix preview of theming

.drone.yml

@@ -540,6 +540,13 @@ pipeline:
       when:
         matrix:
           TESTS-ACCEPTANCE: app-files
+  acceptance-app-theming:
+      image: nextcloudci/integration-php7.0:integration-php7.0-4
+      commands:
+        - tests/acceptance/run-local.sh --timeout-multiplier 10 --nextcloud-server-domain acceptance-app-theming --selenium-server selenium:4444 allow-git-repository-modifications features/app-theming.feature
+      when:
+        matrix:
+          TESTS-ACCEPTANCE: app-theming
   acceptance-login:
       image: nextcloudci/integration-php7.0:integration-php7.0-4
       commands:
@@ -685,6 +692,8 @@ matrix:
       TESTS-ACCEPTANCE: access-levels
     - TESTS: acceptance
       TESTS-ACCEPTANCE: app-files
+    - TESTS: acceptance
+      TESTS-ACCEPTANCE: app-theming
     - TESTS: acceptance
       TESTS-ACCEPTANCE: login
     - TESTS: jsunit

apps/theming/js/settings-admin.js

@@ -30,20 +30,20 @@ function setThemingValue(setting, value) {
 		OC.generateUrl('/apps/theming/ajax/updateStylesheet'), {'setting' : setting, 'value' : value}
 	).done(function(response) {
 		hideUndoButton(setting, value);
-		preview(setting, value);
+		preview(setting, value, response.data.serverCssUrl);
 	}).fail(function(response) {
 		OC.msg.finishedSaving('#theming_settings_msg', response);
 		$('#theming_settings_loading').hide();
 	});
 
 }
 
-function preview(setting, value) {
+function preview(setting, value, serverCssUrl) {
 	OC.msg.startAction('#theming_settings_msg', t('theming', 'Loading preview…'));
-	var stylesheetsLoaded = 2;
+	var stylesheetsLoaded = 1;
 	var reloadStylesheets = function(cssFile) {
 		var queryString = '?reload=' + new Date().getTime();
-		var url = OC.generateUrl(cssFile) + queryString;
+		var url = cssFile + queryString;
 		var old = $('link[href*="' + cssFile.replace("/","\/") + '"]');
 		var stylesheet = $("<link/>", {
 			rel: "stylesheet",
@@ -62,8 +62,12 @@ function preview(setting, value) {
 		stylesheet.appendTo("head");
 	};
 
-	reloadStylesheets('/css/core/server.css');
-	reloadStylesheets('/apps/theming/styles');
+	if (serverCssUrl !== undefined) {
+		stylesheetsLoaded++;
+
+		reloadStylesheets(serverCssUrl);
+	}
+	reloadStylesheets(OC.generateUrl('/apps/theming/styles'));
 
 	// Preview images
 	var timestamp = new Date().getTime();
@@ -218,7 +222,7 @@ $(document).ready(function () {
 				var input = document.getElementById('theming-'+setting);
 				input.value = response.data.value;
 			}
-			preview(setting, response.data.value);
+			preview(setting, response.data.value, response.data.serverCssUrl);
 		});
 	});
 

apps/theming/lib/Controller/ThemingController.php

@@ -73,6 +73,8 @@ class ThemingController extends Controller {
 	private $appData;
 	/** @var SCSSCacher */
 	private $scssCacher;
+	/** @var IURLGenerator */
+	private $urlGenerator;
 
 	/**
 	 * ThemingController constructor.
@@ -87,6 +89,7 @@ class ThemingController extends Controller {
 	 * @param ITempManager $tempManager
 	 * @param IAppData $appData
 	 * @param SCSSCacher $scssCacher
+	 * @param IURLGenerator $urlGenerator
 	 */
 	public function __construct(
 		$appName,
@@ -98,7 +101,8 @@ public function __construct(
 		IL10N $l,
 		ITempManager $tempManager,
 		IAppData $appData,
-		SCSSCacher $scssCacher
+		SCSSCacher $scssCacher,
+		IURLGenerator $urlGenerator
 	) {
 		parent::__construct($appName, $request);
 
@@ -110,6 +114,7 @@ public function __construct(
 		$this->tempManager = $tempManager;
 		$this->appData = $appData;
 		$this->scssCacher = $scssCacher;
+		$this->urlGenerator = $urlGenerator;
 	}
 
 	/**
@@ -172,7 +177,8 @@ public function updateStylesheet($setting, $value) {
 			[
 				'data' =>
 					[
-						'message' => $this->l10n->t('Saved')
+						'message' => $this->l10n->t('Saved'),
+						'serverCssUrl' => $this->urlGenerator->linkTo('', $this->scssCacher->getCachedSCSS('core', '/core/css/server.scss'))
 					],
 				'status' => 'success'
 			]
@@ -303,7 +309,8 @@ public function undo($setting) {
 				'data' =>
 					[
 						'value' => $value,
-						'message' => $this->l10n->t('Saved')
+						'message' => $this->l10n->t('Saved'),
+						'serverCssUrl' => $this->urlGenerator->linkTo('', $this->scssCacher->getCachedSCSS('core', '/core/css/server.scss'))
 					],
 				'status' => 'success'
 			]

apps/theming/tests/Controller/ThemingControllerTest.php

@@ -70,6 +70,8 @@ class ThemingControllerTest extends TestCase {
 	private $appData;
 	/** @var SCSSCacher */
 	private $scssCacher;
+	/** @var IURLGenerator */
+	private $urlGenerator;
 
 	public function setUp() {
 		$this->request = $this->createMock(IRequest::class);
@@ -85,6 +87,7 @@ public function setUp() {
 			->willReturn(123);
 		$this->tempManager = \OC::$server->getTempManager();
 		$this->scssCacher = $this->createMock(SCSSCacher::class);
+		$this->urlGenerator = $this->createMock(IURLGenerator::class);
 
 		$this->themingController = new ThemingController(
 			'theming',
@@ -96,53 +99,102 @@ public function setUp() {
 			$this->l10n,
 			$this->tempManager,
 			$this->appData,
-			$this->scssCacher
+			$this->scssCacher,
+			$this->urlGenerator
 		);
 
 		return parent::setUp();
 	}
 
-	public function dataUpdateStylesheet() {
+	public function dataUpdateStylesheetSuccess() {
 		return [
-			['name', str_repeat('a', 250), 'success', 'Saved'],
-			['name', str_repeat('a', 251), 'error', 'The given name is too long'],
-			['url', str_repeat('a', 500), 'success', 'Saved'],
-			['url', str_repeat('a', 501), 'error', 'The given web address is too long'],
-			['slogan', str_repeat('a', 500), 'success', 'Saved'],
-			['slogan', str_repeat('a', 501), 'error', 'The given slogan is too long'],
-			['color', '#0082c9', 'success', 'Saved'],
-			['color', '#0082C9', 'success', 'Saved'],
-			['color', '0082C9', 'error', 'The given color is invalid'],
-			['color', '#0082Z9', 'error', 'The given color is invalid'],
-			['color', 'Nextcloud', 'error', 'The given color is invalid'],
+			['name', str_repeat('a', 250), 'Saved'],
+			['url', str_repeat('a', 500), 'Saved'],
+			['slogan', str_repeat('a', 500), 'Saved'],
+			['color', '#0082c9', 'Saved'],
+			['color', '#0082C9', 'Saved'],
 		];
 	}
 
 	/**
-	 * @dataProvider dataUpdateStylesheet
+	 * @dataProvider dataUpdateStylesheetSuccess
 	 *
 	 * @param string $setting
 	 * @param string $value
-	 * @param string $status
 	 * @param string $message
 	 */
-	public function testUpdateStylesheet($setting, $value, $status, $message) {
+	public function testUpdateStylesheetSuccess($setting, $value, $message) {
 		$this->themingDefaults
-			->expects($status === 'success' ? $this->once() : $this->never())
+			->expects($this->once())
 			->method('set')
 			->with($setting, $value);
 		$this->l10n
 			->expects($this->once())
 			->method('t')
 			->with($message)
 			->willReturn($message);
+		$this->scssCacher
+			->expects($this->once())
+			->method('getCachedSCSS')
+			->with('core', '/core/css/server.scss')
+			->willReturn('/core/css/someHash-server.scss');
+		$this->urlGenerator
+			->expects($this->once())
+			->method('linkTo')
+			->with('', '/core/css/someHash-server.scss')
+			->willReturn('/nextcloudWebroot/core/css/someHash-server.scss');
 
-		$expected = new DataResponse([
-			'data' => [
-				'message' => $message,
-			],
-			'status' => $status,
-		]);
+		$expected = new DataResponse(
+			[
+				'data' =>
+					[
+						'message' => $message,
+						'serverCssUrl' => '/nextcloudWebroot/core/css/someHash-server.scss',
+					],
+				'status' => 'success',
+			]
+		);
+		$this->assertEquals($expected, $this->themingController->updateStylesheet($setting, $value));
+	}
+
+	public function dataUpdateStylesheetError() {
+		return [
+			['name', str_repeat('a', 251), 'The given name is too long'],
+			['url', str_repeat('a', 501), 'The given web address is too long'],
+			['slogan', str_repeat('a', 501), 'The given slogan is too long'],
+			['color', '0082C9', 'The given color is invalid'],
+			['color', '#0082Z9', 'The given color is invalid'],
+			['color', 'Nextcloud', 'The given color is invalid'],
+		];
+	}
+
+	/**
+	 * @dataProvider dataUpdateStylesheetError
+	 *
+	 * @param string $setting
+	 * @param string $value
+	 * @param string $message
+	 */
+	public function testUpdateStylesheetError($setting, $value, $message) {
+		$this->themingDefaults
+			->expects($this->never())
+			->method('set')
+			->with($setting, $value);
+		$this->l10n
+			->expects($this->once())
+			->method('t')
+			->with($message)
+			->willReturn($message);
+
+		$expected = new DataResponse(
+			[
+				'data' =>
+					[
+						'message' => $message,
+					],
+				'status' => 'error',
+			]
+		);
 		$this->assertEquals($expected, $this->themingController->updateStylesheet($setting, $value));
 	}
 
@@ -411,13 +463,24 @@ public function testUndo() {
 			->method('undo')
 			->with('MySetting')
 			->willReturn('MyValue');
+		$this->scssCacher
+			->expects($this->once())
+			->method('getCachedSCSS')
+			->with('core', '/core/css/server.scss')
+			->willReturn('/core/css/someHash-server.scss');
+		$this->urlGenerator
+			->expects($this->once())
+			->method('linkTo')
+			->with('', '/core/css/someHash-server.scss')
+			->willReturn('/nextcloudWebroot/core/css/someHash-server.scss');
 
 		$expected = new DataResponse(
 			[
 				'data' =>
 					[
 						'value' => 'MyValue',
 						'message' => 'Saved',
+						'serverCssUrl' => '/nextcloudWebroot/core/css/someHash-server.scss',
 					],
 				'status' => 'success'
 			]
@@ -444,6 +507,16 @@ public function testUndoDelete($value, $filename) {
 			->method('undo')
 			->with($value)
 			->willReturn($value);
+		$this->scssCacher
+			->expects($this->once())
+			->method('getCachedSCSS')
+			->with('core', '/core/css/server.scss')
+			->willReturn('/core/css/someHash-server.scss');
+		$this->urlGenerator
+			->expects($this->once())
+			->method('linkTo')
+			->with('', '/core/css/someHash-server.scss')
+			->willReturn('/nextcloudWebroot/core/css/someHash-server.scss');
 		$folder = $this->createMock(ISimpleFolder::class);
 		$file = $this->createMock(ISimpleFile::class);
 		$this->appData
@@ -466,6 +539,7 @@ public function testUndoDelete($value, $filename) {
 					[
 						'value' => $value,
 						'message' => 'Saved',
+						'serverCssUrl' => '/nextcloudWebroot/core/css/someHash-server.scss',
 					],
 				'status' => 'success'
 			]

tests/acceptance/config/behat.yml

@@ -17,6 +17,7 @@ default:
         - NotificationContext
         - SettingsContext
         - SettingsMenuContext
+        - ThemingAppContext
         - UsersSettingsContext
   extensions:
     Behat\MinkExtension:

tests/acceptance/features/app-theming.feature

@@ -0,0 +1,23 @@
+Feature: app-theming
+
+  Scenario: changing the color updates the header color
+    Given I am logged in as the admin
+    And I visit the settings page
+    And I open the "Theming" section
+    And I see that the color selector in the Theming app has loaded
+    And I see that the header color is "0082C9"
+    When I set the "Color" parameter in the Theming app to "C9C9C9"
+    Then I see that the parameters in the Theming app are eventually saved
+    And I see that the header color is "C9C9C9"
+
+  Scenario: resetting the color updates the header color
+    Given I am logged in as the admin
+    And I visit the settings page
+    And I open the "Theming" section
+    And I see that the color selector in the Theming app has loaded
+    And I set the "Color" parameter in the Theming app to "C9C9C9"
+    And I see that the parameters in the Theming app are eventually saved
+    And I see that the header color is "C9C9C9"
+    When I reset the "Color" parameter in the Theming app to its default value
+    Then I see that the parameters in the Theming app are eventually saved
+    And I see that the header color is "0082C9"