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

Clear any theming prefixed cache on cache buster increase #8983

Merged
merged 2 commits into from
Mar 26, 2018
Merged

Conversation

juliusknorr
Copy link
Member

@juliusknorr juliusknorr commented Mar 26, 2018

Regression from #8716, where the baseUrl was added to the theming cache prefix. When clearing on cache buster increase, the cache was not cleared properly, because it was missing the baseUrl.

This PR will make sure we clear cached values for any baseUrl prefix.

Fixes #8888

Also requires to be backported to 13.

Signed-off-by: Julius Härtl <jus@bitgrid.net>
@juliusknorr
Copy link
Member Author

I guess out acceptance tests didn't catch that, since they are not running with any cache backend.

Signed-off-by: Julius Härtl <jus@bitgrid.net>
@codecov
Copy link

codecov bot commented Mar 26, 2018

Codecov Report

Merging #8983 into master will decrease coverage by 19.68%.
The diff coverage is 0%.

@@              Coverage Diff              @@
##             master    #8983       +/-   ##
=============================================
- Coverage     53.58%   33.89%   -19.69%     
  Complexity    23981    23981               
=============================================
  Files          1443     1443               
  Lines         80307    79277     -1030     
=============================================
- Hits          43030    26874    -16156     
- Misses        37277    52403    +15126
Impacted Files Coverage Δ Complexity Δ
apps/theming/lib/ThemingDefaults.php 20.89% <0%> (-70.9%) 46 <0> (ø)
core/Command/TwoFactorAuth/Disable.php 0% <0%> (-100%) 4% <0%> (ø)
apps/files/lib/Activity/Settings/FileRestored.php 0% <0%> (-100%) 8% <0%> (ø)
...ware/Security/Exceptions/NotConfirmedException.php 0% <0%> (-100%) 1% <0%> (ø)
apps/user_ldap/lib/GroupPluginManager.php 0% <0%> (-100%) 17% <0%> (ø)
apps/federation/lib/Hooks.php 0% <0%> (-100%) 4% <0%> (ø)
lib/private/Hooks/LegacyEmitter.php 0% <0%> (-100%) 1% <0%> (ø)
apps/files_trashbin/appinfo/app.php 0% <0%> (-100%) 0% <0%> (ø)
apps/comments/tests/Unit/Activity/ListenerTest.php 0% <0%> (-100%) 2% <0%> (ø)
apps/dav/lib/CalDAV/PublicCalendarObject.php 0% <0%> (-100%) 1% <0%> (ø)
... and 636 more

@skjnldsv 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 Mar 26, 2018
@rullzer rullzer merged commit fa09e0e into master Mar 26, 2018
@rullzer rullzer deleted the fix-8888 branch March 26, 2018 19:46
@mairsebastian
Copy link

so i just need to replace the file ? sorry for that question ..

@ExaconAT
Copy link

Don't know

@juliusknorr
Copy link
Member Author

@mairsebastian @ExaconAT You can just change the line in apps/theming/lib/ThemingDefaults.php as shown in the diff: https://github.com/nextcloud/server/pull/8983/files#diff-5ba7c2fd4066c431ab229e984115f9b6

@mairsebastian
Copy link

hello @juliushaertl i dont have this file or this Directory "test"

apps/theming/tests/ThemingDefaultsTest.php

@mairsebastian
Copy link

sry me again, after changing the first line, nextcloud is not working anymore. any ideas?

@juliusknorr
Copy link
Member Author

@mairsebastian You should restore the file from originally installed zip archive. When you try to patch your Nextcloud instance, you can ignore the ThemingDefaultsTest.php file. All it requires is to change one line in apps/theming/lib/ThemingDefaults.php to get rid of the 'getScssVariables' method argument.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish bug feature: theming regression
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fresh Install NC 13.0.1 Ubuntu Server, cannot change Theme, nothing happens
5 participants