Skip to content

Fix JavaScript pack output for SAML post-binding template#4592

Merged
aduth merged 1 commit intomasterfrom
aduth-fix-saml-post-js
Jan 15, 2021
Merged

Fix JavaScript pack output for SAML post-binding template#4592
aduth merged 1 commit intomasterfrom
aduth-fix-saml-post-js

Conversation

@aduth
Copy link
Contributor

@aduth aduth commented Jan 15, 2021

Why: So that users are redirected automatically when completing SAML authentication.

In #4553, all JavaScript packs were changed to use the javascript_packs_tag_once helper, which is intended to be used in rendering all JavaScript chunks once at the bottom of the page. However, this must be complemented by a call to the render_javascript_pack_once_tags helper. This exists in the base template, but since the saml_post_binding view doesn't inherit from this template, the helper is never called, and therefore the script is never added to the page.

Since we don't need to worry as much about deduplicating script chunks on this page, we can simply call directly to Webpacker's javascript_packs_with_chunks_tag helper. Alternatively, we could add the call to render_javascript_pack_once_tags somewhere within the page template (ideally toward the bottom of markup).

In making these changes, I checked to confirm there are no other instances of views not inheriting from the base template where script enqueuing via javascript_packs_tag_once is occurring.

**Why:** So that users are redirected automatically when completing SAML authentication.

In #4553, all JavaScript packs were changed to use the `javascript_packs_tag_once` helper, which is intended to be used in rendering all JavaScript chunks once at the bottom of the page. However, this must be complemented by a call to the `render_javascript_pack_once_tags` helper. This exists in the base template, but since the `saml_post_binding` view doesn't inherit from this template, the helper is never called, and therefore the script is never added to the page.

Since we don't need to worry as much about deduplicating script chunks on this page, we can simply call directly to Webpacker's `javascript_packs_with_chunks_tag` helper.
Copy link
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

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

LGTM

@aduth aduth merged commit d28d059 into master Jan 15, 2021
@aduth aduth deleted the aduth-fix-saml-post-js branch January 15, 2021 13:59
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.

3 participants