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

Properly detect thumbnail availability in PreviewPlugin #31785

Merged
merged 1 commit into from
Jun 14, 2018

Conversation

PVince81
Copy link
Contributor

Description

Use preview manager's "isAvailable" method.
Also fixes isAvailable to properly recognize IProvider2 instances.

Related Issue

Fixes #31763

Motivation and Context

How Has This Been Tested?

Manual test with external storage where previews are disabled (cog icon): network console shows 404 for every preview. Outside of external storage, previews still render.

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

.htaccess Outdated
#### DO NOT CHANGE ANYTHING ABOVE THIS LINE ####

ErrorDocument 403 /core/templates/403.php
ErrorDocument 404 /core/templates/404.php
Copy link
Contributor

Choose a reason for hiding this comment

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

by accident?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah... somehow .htaccess keeps on sneaking in

Copy link
Contributor

Choose a reason for hiding this comment

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

I do that to myself quite often - running some tests, setting up stuff, whatever, makes the additions to the end of .htaccess
Then without thinking I git add -A and git commit - when I need to first git checkout .htaccess to get rid of the changed file.

Copy link
Member

Choose a reason for hiding this comment

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

chmod a-w .htaccess and you are save forever

Use preview manager's "isAvailable" method.
Also fixes isAvailable to properly recognize IProvider2 instances.
@PVince81 PVince81 force-pushed the previews-regression-mountpoint branch from 1f35648 to d3630bd Compare June 14, 2018 14:59
@PVince81
Copy link
Contributor Author

Fixed unit test and php-cs

@codecov
Copy link

codecov bot commented Jun 14, 2018

Codecov Report

Merging #31785 into master will increase coverage by 0.03%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #31785      +/-   ##
============================================
+ Coverage      63.2%   63.24%   +0.03%     
- Complexity    18453    18455       +2     
============================================
  Files          1158     1158              
  Lines         69302    69305       +3     
  Branches       1261     1261              
============================================
+ Hits          43802    43831      +29     
+ Misses        25130    25104      -26     
  Partials        370      370
Flag Coverage Δ Complexity Δ
#javascript 52.47% <ø> (ø) 0 <ø> (ø) ⬇️
#phpunit 64.47% <100%> (+0.04%) 18455 <1> (+2) ⬆️
Impacted Files Coverage Δ Complexity Δ
apps/dav/lib/Server.php 51.07% <100%> (ø) 24 <0> (ø) ⬇️
apps/dav/lib/Files/PreviewPlugin.php 91.48% <100%> (+0.58%) 16 <1> (+1) ⬆️
lib/private/PreviewManager.php 72.05% <100%> (+17.64%) 51 <0> (+1) ⬆️
lib/private/Preview/Image.php 87.5% <0%> (+12.5%) 5% <0%> (ø) ⬇️

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 d3841cf...d3630bd. Read the comment docs.

Copy link
Member

@DeepDiver1975 DeepDiver1975 left a comment

Choose a reason for hiding this comment

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

looks good

@lock
Copy link

lock bot commented Jul 30, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jul 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Disable 'Enable previews' option do not stop showing the thumbnails for an external storage mountpoint
5 participants