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 ListItem styling in PDF exports #1781

Closed
wants to merge 5 commits into from

Conversation

charludo
Copy link
Contributor

Short description

Fixes styling of nested divs in lis, which now have proper line height and list-style icons.

Proposed changes

  • fix styling (ul li div should display as inline-block instead of block)
  • opened PR with xhtml2pdf/xhtml2pdf to fix this upstream

Side effects

  • none

Resolved issues

Fixes: #1350 (partly)


Pull Request Review Guidelines

@charludo charludo requested a review from a team as a code owner October 19, 2022 08:20
@charludo charludo linked an issue Oct 19, 2022 that may be closed by this pull request
6 tasks
@charludo
Copy link
Contributor Author

Tests are failing because the test-PDFs need to be regenerated; not quite sure how I do this? Where do I get the page content that's used for the Circle-CI tests?

@timobrembeck
Copy link
Member

timobrembeck commented Oct 19, 2022

Tests are failing because the test-PDFs need to be regenerated; not quite sure how I do this? Where do I get the page content that's used for the Circle-CI tests?

On CircleCI, the same test data is used as locally: integreat_cms/cms/fixtures/test_data.json

In order to reset your local database, execute the dev tool ./dev-tools/prune_database.sh and restart the server. It should then automatically load the initial test data.

@charludo
Copy link
Contributor Author

Tests are failing because the test-PDFs need to be regenerated; not quite sure how I do this? Where do I get the page content that's used for the Circle-CI tests?

On CircleCI, the same test data is used as locally: integreat_cms/cms/fixtures/test_data.json

In order to reset your local database, execute the dev tool ./dev-tools/prune_database.sh and restart the server. It should then automatically load the initial test data.

Thanks for the help! Did that, and redownloaded & pushed the new PDFs. Tests are still failing though, it looks like the PDF's slugs have changed...? Not on my local install though, hm.

@timobrembeck
Copy link
Member

Thanks for the help! Did that, and redownloaded & pushed the new PDFs. Tests are still failing though, it looks like the PDF's slugs have changed...? Not on my local install though, hm.

Sorry for the poor documentation on this topic, the export uses a quite specific subset of pages for the test cases.

You can re-generate the PDFs you need with a little workaround:

  1. Clear the pdf cache: rm -rf integreat_cms/pdf
  2. Run the tests locally with INTEGREAT_CMS_SECRET_KEY=dummy pipenv run pytest tests/pdf/test_pdf_export.py --disable-warnings --ds=integreat_cms.core.docker_settings
  3. Exploit the fact that the PDF cache is shared between your local server and the test runner and execute cp -r integreat_cms/pdf/* tests/pdf/files/

@timobrembeck
Copy link
Member

OK, if that didn't help, I have no idea what's going on 🤔

@charludo
Copy link
Contributor Author

OK, if that didn't help, I have no idea what's going on thinking

It's weird. Why would the pdf slug differ between Circle CI and a local install? Locally, the tests pass as well.

@timobrembeck
Copy link
Member

It's weird. Why would the pdf slug differ between Circle CI and a local install? Locally, the tests pass as well.

The hash is calculated here, before the PDF is even generated, so I don't get how changing a bit of css and uploading new files can even change the hash here 😅

Moreover, this branch with exactly the same PDFs succeeds.
Maybe we should escalate this problem to the CircleCI support so they can tell us why the same git state results in different CI results 😆

@charludo charludo closed this Oct 20, 2022
@charludo charludo deleted the enhancement/improve-pdf-export branch October 20, 2022 05:42
@charludo
Copy link
Contributor Author

charludo commented Oct 20, 2022

The hash is calculated here, before the PDF is even generated, so I don't get how changing a bit of css and uploading new files can even change the hash here sweat_smile

I assumed one of the arguments of the hash function differed, but wasn't able to determine which one. However...

Moreover, this branch with exactly the same PDFs succeeds. Maybe we should escalate this problem to the CircleCI support so they can tell us why the same git state results in different CI results laughing

...that is indeed fascinating. Looks like the branch name is causing the issue - I've renamed this one and now the tests pass. My only guess is this name has been used before, and CircleCI somehwo trips up on this...? Still doesn't really make sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve PDF export
2 participants