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

Replace relying on variables to generate new templating #1205

Merged
merged 1 commit into from
Jan 17, 2024

Conversation

norkans7
Copy link
Contributor

No description provided.

@norkans7 norkans7 requested a review from rowanseymour January 17, 2024 14:45
Copy link

codecov bot commented Jan 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (eefb48d) 87.72% compared to head (abcf3dd) 87.72%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1205   +/-   ##
=======================================
  Coverage   87.72%   87.72%           
=======================================
  Files         260      260           
  Lines       10870    10870           
=======================================
  Hits         9536     9536           
  Misses        915      915           
  Partials      419      419           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@norkans7 norkans7 force-pushed the variables-params-alt2 branch from 0548d41 to ecf7ab0 Compare January 17, 2024 14:52
@norkans7 norkans7 force-pushed the variables-params-alt2 branch from ecf7ab0 to abcf3dd Compare January 17, 2024 14:55
@@ -122,7 +122,18 @@ func (a *SendMsgAction) Execute(run flows.Run, step flows.Step, logModifier flow
}

evaluatedText = translation.Substitute(evaluatedVariables)
templating = flows.NewMsgTemplating(a.Templating.Template, evaluatedVariables, translation.Namespace())

params := make(map[string][]flows.TemplateParam, 1)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just moved building the params here and pass that to the flows.NewMsgTemplating instead of passing a list of variables

params := make(map[string][]flows.TemplateParam, 1)

// TODO add support for params in other components besides body
if len(evaluatedVariables) > 0 {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think here is the right place since we can have access to the param types in the template translations once we support other components and parameter types

@rowanseymour rowanseymour merged commit ff8b2a7 into main Jan 17, 2024
7 checks passed
@rowanseymour rowanseymour deleted the variables-params-alt2 branch January 17, 2024 16:10
@github-actions github-actions bot locked and limited conversation to collaborators Jan 17, 2024
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.

2 participants