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

fix: create previews from first page on multi-page documents #41045

Merged
merged 4 commits into from Feb 9, 2024

Conversation

DeepDiver1975
Copy link
Member

@DeepDiver1975 DeepDiver1975 commented Oct 12, 2023

Description

Previews are now generated from the first page and no longer from the last page.

Related Issue

Motivation and Context

How Has This Been Tested?

  • test environment:
  • test case 1:
  • test case 2:
  • ...

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Database schema changes (next release will require increase of minor version instead of patch)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:
  • Changelog item, see TEMPLATE

@sonarcloud
Copy link

sonarcloud bot commented Oct 12, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@owncloud owncloud deleted a comment from update-docs bot Feb 5, 2024
@phil-davis
Copy link
Contributor

@DeepDiver1975 this seems like a good thing - merge?

@DeepDiver1975
Copy link
Member Author

this need some more testing - as per #41042 it is not working ...... no idea if the code has issues or there have been issues in applying the patch.

@pako81
Copy link
Contributor

pako81 commented Feb 5, 2024

Confirmed current code is not properly working. It seems setIteratorIndex should be used in this case, so:

$bp->readImageFile($stream);
$bp->setIteratorIndex(0);

this way previews are correctly generated from the first page of the document.

@phil-davis
Copy link
Contributor

works for me. This PR is quite old, so I will rebase to get up-to-date CI with current master. Then I will try to add an acceptance test - we already have thumbnail tests, so I should be able provide a multi-page PDF as a fixture along with the expected thumbnail of the first page.

@phil-davis
Copy link
Contributor

    When user "Alice" downloads the preview of "/Preview-test.pdf" with width "612" and height "792" using the WebDAV API # FeatureContext::downloadPreviewOfFiles()
    Then the HTTP status code should be "200"                                                                             # FeatureContext::thenTheHTTPStatusCodeShouldBe()
      HTTP status code 404 is not the expected value 200
      Failed asserting that 404 matches expected '200'.

Hmm, this passed for me locally. I suppose there is some setting of the max preview size or something that needs to be done to get a "large" preview like in this test.

@phil-davis
Copy link
Contributor

Hmm, this passed for me locally. I suppose there is some setting of the max preview size or something that needs to be done to get a "large" preview like in this test.

Preview of PDFs is not enabled by default. I had to set it in system config enabledPreviewProviders

Copy link

sonarcloud bot commented Feb 9, 2024

@phil-davis phil-davis merged commit 96303f6 into master Feb 9, 2024
3 checks passed
@delete-merged-branch delete-merged-branch bot deleted the fix/pdf-preview-first-page branch February 9, 2024 12:43
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.

PDF Preview only shows last section of pdf
3 participants