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 generating dashboard poster on production #4767

Merged
merged 1 commit into from
Jan 18, 2022
Merged

Conversation

javierm
Copy link
Member

@javierm javierm commented Jan 11, 2022

References

Objectives

Fix a bug which made it impossible to generate the proposals dashboard poster on production environments.

Notes

We aren't adding a test case because this bug is only present on production environments when assets have been precompiled.

@javierm javierm added the Bug label Jan 11, 2022
@javierm javierm added this to Reviewing in Consul Democracy via automation Jan 11, 2022
@javierm javierm self-assigned this Jan 11, 2022
@javierm javierm requested a review from Senen January 11, 2022 16:41
In commit 905ac48 we mentioned:

> Since we don't use `asset_path` to reference assets in the public
> folder, we can safely disable the `unknown_asset_fallback` option.

However, `asset_path` is used by the wicked_pdf gem when calling the
`wicked_pdf_stylesheet_link_tag` method. This method also checks the CSS
files, searching for `url()` calls and converting any relative URLs
referenced there to absolute URLs.

However, when compiling assets on production, our `application.css` file
contains the following line imported from Leaflet which says:

```
behavior: url(#default#VML);
```

When passing this URL to `asset_path` (which is something the wicked_pdf
gem does automatically), it doesn't find the URL, and so this call
crashes unless we enable then `unknown_asset_fallback` option.

Since the dashboard poster is a feature we might remove in the future,
we're avoiding changing a Rails global configuration just for this
feature. So, instead of enabling the `unknown_asset_fallback` option,
we're changing the `poster.pdf` view so it doesn't load all the CSS of
the application but only the CSS it needs.

Note we aren't adding a test case because this bug is only present on
production environments when assets have been precompiled.
Copy link
Member

@Senen Senen left a comment

Choose a reason for hiding this comment

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

Nice catch and solution!

Consul Democracy automation moved this from Reviewing to Testing Jan 18, 2022
@javierm javierm merged commit a53813f into master Jan 18, 2022
Consul Democracy automation moved this from Testing to Release 1.5.0 Jan 18, 2022
@javierm javierm deleted the poster_asset_path branch January 18, 2022 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants