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

[Templating] Realign platforms on invalid $when behavior #7432

Merged
merged 4 commits into from
May 4, 2022

Conversation

anna-dingler
Copy link
Contributor

@anna-dingler anna-dingler commented May 2, 2022

Related Issue

Fixes #6630

Description

Realign expected behavior for invalid boolean expressions for $when on JS and .NET Templating platforms.

Scenarios:

  • If the given string is not an expression (ex: "$when": "notAnExpression")
  • If the given expression is invalid/not found in data (ex: "$when": "${invalidExpression}")

In both scenarios, the $when condition should evaluate to false (drop the element) and issue a warning.

Sample Card

Test 1 - Not an Expression

{
    "type": "AdaptiveCard",
    "body": [
        {
            "type": "TextBlock",
            "size": "Medium",
            "weight": "Bolder",
            "text": "${title}",
            "$when": "NotAnExpression"
        }
    ],
    "$schema": "http://adaptivecards.io/schemas/adaptive-card.json",
    "version": "1.5"
}

Test 2 - Invalid Expression No Data

{
    "type": "AdaptiveCard",
    "body": [
        {
            "type": "TextBlock",
            "size": "Medium",
            "weight": "Bolder",
            "text": "${title}",
            "$when": "${InvalidExpression}"
        }
    ],
    "$schema": "http://adaptivecards.io/schemas/adaptive-card.json",
    "version": "1.5"
}

Test 3 - Invalid Expression with Data

Card

{
    "type": "AdaptiveCard",
    "body": [
        {
            "type": "TextBlock",
            "size": "Medium",
            "weight": "Bolder",
            "text": "${title}",
            "$when": "${InvalidExpression}"
        }
    ],
    "$schema": "http://adaptivecards.io/schemas/adaptive-card.json",
    "version": "1.5"
}

Data

{
   "test": "dummyData"
}

How Verified

Verified manually with adaptivecards-site and WPF Visualizer. I also added test cases to both platforms.

Microsoft Reviewers: Open in CodeFlow

@ghost
Copy link

ghost commented May 2, 2022

Hi @anna-dingler. Thanks for helping make the AdaptiveCards JS renderer + tooling better. As additional verification, once the JS build succeeds, please go to the test site to test out your website/designer changes.

@anna-dingler anna-dingler requested review from jwoo-msft, paulcam206, almedina-ms, beervoley and licanhua and removed request for jwoo-msft May 2, 2022 19:04
Copy link
Member

@jwoo-msft jwoo-msft left a comment

Choose a reason for hiding this comment

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

it looks good, my only reservation is the implementation of warning. I will almost say it is necessary to have this implemented before we make next release since we are changing the behavior, adding the warning will give the dev more flexibility to adapt to our changes.

@anna-dingler
Copy link
Contributor Author

it looks good, my only reservation is the implementation of warning. I will almost say it is necessary to have this implemented before we make next release since we are changing the behavior, adding the warning will give the dev more flexibility to adapt to our changes.

Thanks for reviewing! I'm working on updating the warning implementation now, so I'll make that PR before making the next release.

@anna-dingler anna-dingler merged commit d5020b1 into microsoft:main May 4, 2022
anna-dingler added a commit that referenced this pull request May 11, 2022
* Align templating platforms for invalid boolean behavior

* Formatting

* Add comment for exposing warnings to caller
anna-dingler added a commit that referenced this pull request May 12, 2022
* Align templating platforms for invalid boolean behavior

* Formatting

* Add comment for exposing warnings to caller
paulcam206 added a commit that referenced this pull request May 13, 2022
* [.NET Templating] Update Boolean expression evaluation (#7223)

* Update expansion of Boolean expressions

* Add unit test

* Add test for property that accepts boolean

* [.NET Templating] Resolve commas added in wrong place (#7225)

* Update expansion of Boolean expressions

* Add unit test

* Add test for property that accepts boolean

* Resolve extra commas in temaplting output

* [Templating] Realign platforms on invalid $when behavior (#7432)

* Align templating platforms for invalid boolean behavior

* Formatting

* Add comment for exposing warnings to caller

* [Templating] Expose warnings/errors to callers (#7437)

* In progress: log warnings

* Error log for .NET

* Add error logging for NodeJS

* Resolve comments on PR

* Consolidate ArrayList declarations and fix testing nuget package

* Rearrange Expand methods and add comments

* Refactor to add the errors to template instance

* Update warning message

* Get warnings from last template expansion

* Add markdown file changes

Co-authored-by: Paul Campbell <paulcam@microsoft.com>
michaelfarnsworth pushed a commit to michaelfarnsworth/AdaptiveCards that referenced this pull request Nov 10, 2022
)

* Align templating platforms for invalid boolean behavior

* Formatting

* Add comment for exposing warnings to caller
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants