Skip to content
This repository has been archived by the owner on Nov 1, 2023. It is now read-only.

Validate notification template #2655

Merged

Conversation

tevoinea
Copy link
Member

@tevoinea tevoinea commented Nov 25, 2022

Summary of the Pull Request

What is this about?

Adds a new endpoint /ValidateScriban which can be used to test scriban templates.

PR Checklist

Info on Pull Request

What does this include?

  • A new endpoint /ValidateScriban. Which will attempt to render a template using scriban, return the rendered template and the rendering context used.
  • New model: TemplateRenderContext. This simply gives a name to the anonymous type we were already using before for rendering templates.
  • New request/response objects. Used for receiving and returning rendering data.
  • Allow disabling jinja migration in the render function. The ValidateScriban endpoint should not be attempting to support jinja, therefore we need a way to disable the functionality. This will be modifiable by feature flag as well once Add support for feature flags #2620 merges.

Validation Steps Performed

How does someone test & validate?

  1. Start azure functions
  2. Do a post request with the following body:
{
  "template": "{{ report.crash_site }} - {{ report.executable }}",
}
  1. Validate it was rendered correctly.
  2. Do a post request with the following body:
{
  "template": "{{ report.NOTAREALPROPERTY }} - {{ report.executable }}",
}
  1. Validate an error was thrown.
  2. Do a post request with the following body:
{
  "template": "{{ report.crash_site }} - {{ report.executable }}",
  "context": {
    "report": {
      "crash_site": "ABC - 123  - THIS IS AN AWESOME CRASH SITE"
    }
  }
}
  1. Validate that the new crash site string was used.

In all of the above cases, validate that the returned TemplateRenderContext was correct for the input as well.

@codecov-commenter
Copy link

codecov-commenter commented Nov 25, 2022

Codecov Report

Merging #2655 (95eefc5) into main (b356c85) will increase coverage by 2.38%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #2655      +/-   ##
==========================================
+ Coverage   29.27%   31.66%   +2.38%     
==========================================
  Files         289      168     -121     
  Lines       35774    23247   -12527     
==========================================
- Hits        10474     7361    -3113     
+ Misses      25300    15886    -9414     
Impacted Files Coverage Δ
src/ApiService/ApiService/OneFuzzTypes/Model.cs
src/ApiService/ApiService/OneFuzzTypes/Requests.cs
...rc/ApiService/ApiService/OneFuzzTypes/Responses.cs
src/ApiService/ApiService/ServiceConfiguration.cs
...Service/ApiService/onefuzzlib/notifications/Ado.cs
...piService/onefuzzlib/notifications/GithubIssues.cs
...vice/onefuzzlib/notifications/NotificationsBase.cs
src/ApiService/ApiService/onefuzzlib/Containers.cs
.../ApiService/ApiService/onefuzzlib/RequestAccess.cs
...ce/ApiService/TestHooks/DiskOperationsTestHooks.cs
... and 111 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@tevoinea tevoinea merged commit 3f37fd2 into microsoft:main Dec 8, 2022
@mgreisen mgreisen mentioned this pull request Dec 9, 2022
@microsoft microsoft locked as resolved and limited conversation to collaborators Jan 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants