fix: Remove join filters and related workarounds in replacement card #1808
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
danielamorse
approved these changes
Apr 7, 2020
|
PR was released with v2.21.0 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Jira
http://vjira2:8080/browse/BDS-2059
Summary
Adds support for render arrays to replacement card
Details
This PR replaces
{{content | join}}(and all the related workarounds, such as{{ card_replacement_body_content | join | replace({"UTF-8": ""}) | raw }}) with a solution that supports render arrays, doesn't require any workarounds, and is consistent with our docs and all our other components. It's also quite simple:
{{ content }}.FAQ
Why was the
joinfilter used to begin with?I think it was just a mistake. There's no usage of
| joinin the original card. There are some usages of the join filter directly in blueprints. Maybe this was an attempt at a shortcut for that and we just forgot to take it out?Is this backwards breaking?
Yes. Oddly, there were no usages of replacement card in pattern lab that depended upon the join filter-- removing it didn't require any changes. However, there are some places in the Academy site that do (I created a PR to address those-- once it's merged, there should be no side effects of this change downstream https://gitlab.com/pegadigital/pa/pegaacademy/-/merge_requests/80).
Why is this not backwards compatible or in a major release then?
It would be a pain to deprecate it instead (what do you call the new variable if not
content?) and because replacement card is only actively in use on Academy. I'm operating under the assumption that replacement card is still technically experimental.What's the utf-8 bug referenced in the comments?
This happens when a Drupal Markup object is passed through the
joinfilter. If you imagine the markup object looking something like {'Text string', 'UTF-8'} (the second parameter being the encoding), you can understand why. Printing the markup object directly does not have this problem.Was this the problem @charginghawk was solving in this PR?
Apparently not. That code was using the old card, which doesn't use join. So something else was wrong there.
How to test