X Tutup
Skip to content

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

Merged
phil-davis merged 4 commits intomasterfrom
fix/pdf-preview-first-page
Feb 9, 2024
Merged

fix: create previews from first page on multi-page documents#41045
phil-davis merged 4 commits intomasterfrom
fix/pdf-preview-first-page

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

@sonarqubecloud
Copy link

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

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 phil-davis force-pushed the fix/pdf-preview-first-page branch from 6b83926 to bce605b Compare February 9, 2024 07:19
@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

@sonarqubecloud
Copy link

sonarqubecloud bot commented Feb 9, 2024

@phil-davis phil-davis merged commit 96303f6 into master Feb 9, 2024
@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

X Tutup