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

front: simplify and make simulation PDF download test more robust #11027

Open
wants to merge 4 commits into
base: dev
Choose a base branch
from

Conversation

emersion
Copy link
Member

@emersion emersion commented Mar 4, 2025

See individual commits.

@github-actions github-actions bot added the area:front Work on Standard OSRD Interface modules label Mar 4, 2025
@emersion emersion marked this pull request as ready for review March 4, 2025 11:58
@emersion emersion requested a review from a team as a code owner March 4, 2025 11:58
@emersion emersion requested a review from Maymanaf March 4, 2025 11:58
@emersion emersion self-assigned this Mar 4, 2025
emersion added 3 commits March 4, 2025 13:10
react-pdf will create an anchor element without an href attribute
because instance.url is null while the PDF is being generated:
https://github.com/diegomura/react-pdf/blob/f89f75c1f132ba19b54847c3ac23efec675f8d0a/packages/renderer/src/dom/PDFDownloadLink.js#L35

This results in a fetch(null) call, downloading the current HTML
page. Later, we try to parse the PDF file and fail with:

    InvalidPDFException: Invalid PDF structure.

Signed-off-by: Simon Ser <[email protected]>
This is less fragile and also ensures that clicking results in the
expected behavior (a file being downloaded).

Signed-off-by: Simon Ser <[email protected]>
Instead, return the final file path in downloadSimulation().

Signed-off-by: Simon Ser <[email protected]>
Copy link
Contributor

@Maymanaf Maymanaf left a comment

Choose a reason for hiding this comment

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

LGTM ✅

@Maymanaf Maymanaf added this pull request to the merge queue Mar 4, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:front Work on Standard OSRD Interface modules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants