Skip to content

Conversation

@skjnldsv
Copy link
Member

Fix #8363

Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
<div class="icon-external"></div>
<h2><?php p($l->t('No external storage configured')); ?></h2>
<p><a href="<?php p(link_to('', 'index.php/settings/personal#files_external' )); ?>"><?php p($l->t('You can add external storages in the personal settings')); ?></a></p>
<p><a href="<?php p(link_to('', 'index.php/settings/admin/externalstorages' )); ?>"><?php p($l->t('You can add external storages in the personal settings')); ?></a></p>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The message here doesn't make much sense. It is shown, for personal external storage settings and if the administrator disabled the possibility to configure external storages in the personal settings there is nothing users can do about that. Also the link to admin/externalstorages only makes sense for admins.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it should be something like: "You cannot configure external storages, since it has been disabled by the administrator"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@juliushaertl It's not only about disabled by the admin, if you're the admin but you did not set up one at the moment, the page is still empty :)

"No external storage configured or you don't have the permission to configure them"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine by me, better than referring to personal settings when you are on personal settings 😉

Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
@codecov
Copy link

codecov bot commented Feb 23, 2018

Codecov Report

Merging #8511 into master will decrease coverage by 1.68%.
The diff coverage is 0%.

@@             Coverage Diff              @@
##             master    #8511      +/-   ##
============================================
- Coverage     53.49%   51.81%   -1.69%     
- Complexity    24076    25375    +1299     
============================================
  Files          1443     1601     +158     
  Lines         80512    95041   +14529     
  Branches          0     1377    +1377     
============================================
+ Hits          43067    49241    +6174     
- Misses        37445    45800    +8355
Impacted Files Coverage Δ Complexity Δ
apps/files_external/templates/list.php 0% <ø> (ø) 0 <0> (?)
apps/files_external/templates/settings.php 0% <0%> (ø) 0 <0> (?)
apps/files_external/js/settings.js 58.49% <0%> (ø) 0 <0> (?)
.../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%> (ø)
settings/Controller/EncryptionController.php 54.71% <0%> (-38.84%) 8% <0%> (ø)
settings/Controller/GroupsController.php 64.61% <0%> (-35.39%) 9% <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 349 more

@skjnldsv skjnldsv dismissed juliusknorr’s stale review February 23, 2018 11:13

Edited. Please update review :)

@skjnldsv skjnldsv requested a review from juliusknorr February 23, 2018 11:14
Copy link
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested and works 👍

@MorrisJobke MorrisJobke merged commit 07a6821 into master Feb 26, 2018
@MorrisJobke MorrisJobke deleted the ext-storage-error-warning-fix branch February 26, 2018 14:36
@skjnldsv
Copy link
Member Author

backport in #8561

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants