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.

Sign up for GitHub

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

Jump to bottom

Added support for displaying a text if the user is previewing the pdf #904

Merged
merged 11 commits into from
Nov 15, 2024
Merged

Added support for displaying a text if the user is previewing the pdf #904

merged 11 commits into from
Nov 15, 2024

Conversation

adamhaeger
Copy link
Contributor

@adamhaeger adamhaeger commented Nov 11, 2024 •
edited
Loading

Added support for displaying a text if the user is previewing the pdf before the form is submitted.

Frontend PR is dependent on this PR: Altinn/app-frontend-react#2710

Description

Please see acceptance critereas here: Altinn/app-frontend-react#1804

  • Added isPreview parameter to PdfService.GeneratePdf
  • Enabled PDF preview endpoint in production
  • Added logic to display the preview text in the footer instead of the footer when previewing
  • Added default preview text
  • Added support for custom preview text

Related Issue(s)

  • #{1804}

Verification

  • Your code builds clean without any errors or warnings
  • Manual testing done (required)
  • Relevant automated test added (if you find this hard, leave it and we'll help out)
  • All tests run green

Documentation

  • User documentation is updated with a separate linked PR in altinn-studio-docs. (if applicable)

@adamhaeger adamhaeger mentioned this pull request Nov 11, 2024
Merged
19 tasks
@adamhaeger adamhaeger added kind/feature-request New feature or request feature Label Pull requests with new features. Used when generation releasenotes and removed kind/feature-request New feature or request labels Nov 11, 2024
ivarne
ivarne reviewed Nov 11, 2024
View reviewed changes
Copy link
Member

@ivarne ivarne left a comment

Choose a reason for hiding this comment

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

A few code style issues. Looks like you also need to remove the test that ensures that this don't run in production.

src/Altinn.App.Core/Internal/Pdf/IPdfService.cs Outdated Show resolved Hide resolved
src/Altinn.App.Core/Internal/Pdf/PdfService.cs Outdated Show resolved Hide resolved
src/Altinn.App.Core/Internal/Pdf/PdfService.cs Outdated Show resolved Hide resolved
test/Altinn.App.Api.Tests/Data/apps/tdd/task-action/config/texts/resource.nb.json Outdated Show resolved Hide resolved
github-advanced-security[bot]
github-advanced-security bot found potential problems Nov 15, 2024
View reviewed changes
src/Altinn.App.Core/Internal/Pdf/PdfService.cs

string? footerContent = null;

if (isPreview == true)

Check notice

Code scanning / CodeQL

Unnecessarily complex Boolean expression Note

The expression 'A == true' can be simplified to 'A'.
martinothamar
martinothamar approved these changes Nov 15, 2024
View reviewed changes
Copy link
Contributor

@martinothamar martinothamar left a comment

Choose a reason for hiding this comment

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

LGTM

src/Altinn.App.Core/Internal/Pdf/PdfService.cs Show resolved Hide resolved
@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Nov 15, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
61.4% Coverage on New Code (required ≥ 65%)
50.0% Condition Coverage on New Code (required ≥ 65%)

See analysis details on SonarQube Cloud

@adamhaeger adamhaeger merged commit 17655e5 into main Nov 15, 2024
10 of 11 checks passed
@adamhaeger adamhaeger deleted the feat/1804-pdf-preview branch November 15, 2024 12:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@martinothamar martinothamar martinothamar approved these changes

@ivarne ivarne ivarne left review comments

Assignees
No one assigned
Labels
feature Label Pull requests with new features. Used when generation releasenotes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@adamhaeger @ivarne @martinothamar