nyaruka / goflow

Flow engine for RapidPro/TextIt.
Other
43 stars 19 forks source link

Support more components #1208

Closed norkans7 closed 7 months ago

codecov[bot] commented 8 months ago

Codecov Report

Attention: 6 lines in your changes are missing coverage. Please review.

Comparison is base (389b36b) 87.73% compared to head (02e4c76) 87.72%.

Files Patch % Lines
flows/actions/send_msg.go 80.00% 3 Missing and 3 partials :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1208 +/- ## ========================================== - Coverage 87.73% 87.72% -0.02% ========================================== Files 260 260 Lines 10877 10896 +19 ========================================== + Hits 9543 9558 +15 - Misses 915 917 +2 - Partials 419 421 +2 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

ericnewcomer commented 8 months ago

I like option 3 (the params object with uuid), feels the most natural and easiest to work with to me.

On Fri Jan 19, 2024, 06:41 PM GMT, Rowan Seymour @.***> wrote:

@rowanseymour commented on this pull request.

In flows/actions/send_msg.go https://github.com/nyaruka/goflow/pull/1208#discussion_r1459534753:

// Templating represents the templating that should be used if possible type Templating struct { - UUID uuids.UUID json:"uuid" validate:"required,uuid4" - Template assets.TemplateReference json:"template" validate:"required" - Variables []string json:"variables" engine:"localized,evaluated" + UUID uuids.UUID json:"uuid" validate:"required,uuid4" + Template assets.TemplateReference json:"template" validate:"required" + Variables []string json:"variables" engine:"localized,evaluated" + Params map[string]TemplateComponentVariables json:"params" Since we're now talking about how the editor will store things in a flow definition.. let's hear from @ericnewcomer https://github.com/ericnewcomer if any of the following solutions make more sense than others. Remember that localizable things in flow definitions are named properties of an object with a UUID, and every localization is an array of strings (tho often it's a single item array) because the field is a single item. And that the editor is going to see template objects that look like: { "uuid": "b7b35db9-d914-4cd3-a170-46eab02531bb", "params": { "body": [{"type": "text"}, {"type": "text"}], "button.0": [{"type": "text"}] } } So first way we could represent param values is that each is a localizable single value (I think this how you first had it in code): { "type": "send_msg", "text": "Register?", "templating": { "uuid": "5e7229b1-d196-42c3-9b08-125ab80c88ea", "template": { "uuid": "bd9d983c-c0eb-4c4e-b688-3af737c410c6", "name": "Register" }, "params": { "body": [ { "uuid": "1067f8e2-82f0-4378-9214-0f019365ddb7", "value": @." }, { "uuid": "7d771d7b-aab6-4cc6-a3df-360506f759de", "value": "sales" } ] "button.0": [ { "uuid": "b7b35db9-d914-4cd3-a170-46eab02531bb", "value": "OK" } ] } } } Or each component is the localizable object (this is how the code above has it): { "type": "send_msg", "text": "Register?", "templating": { "uuid": "5e7229b1-d196-42c3-9b08-125ab80c88ea", "template": { "uuid": "bd9d983c-c0eb-4c4e-b688-3af737c410c6", "name": "Register" }, "params": { "body": { "uuid": "1067f8e2-82f0-4378-9214-0f019365ddb7", "values": @.", "sales"] }, "button.0": { "uuid": "b7b35db9-d914-4cd3-a170-46eab02531bb", "values": ["OK"] } } } } Or make params the localizable object assuming no component can be called uuid: { "type": "send_msg", "text": "Register?", "templating": { "uuid": "5e7229b1-d196-42c3-9b08-125ab80c88ea", "template": { "uuid": "bd9d983c-c0eb-4c4e-b688-3af737c410c6", "name": "Register" }, "params": { "uuid": "1067f8e2-82f0-4378-9214-0f019365ddb7", "body": @.", "sales"], "button.0": ["OK"] } } } Or use templating as the localizable object (like how it is now with just variables). Probably safe as there's no component that's gonna be called template or uuid right? { "type": "send_msg", "text": "Register?", "templating": { "uuid": "5e7229b1-d196-42c3-9b08-125ab80c88ea", "template": { "uuid": "bd9d983c-c0eb-4c4e-b688-3af737c410c6", "name": "Register" }, "body": @.", "sales"], "button.0": ["OK"] } } So many ways to skin a cat as they say... — Reply to this email directly, view it on GitHub https://github.com/nyaruka/goflow/pull/1208#pullrequestreview-1833456397, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABTVRCYPMU3DCM5IDLNTPDYPK45RAVCNFSM6AAAAABCAEE5IGVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTQMZTGQ2TMMZZG4. You are receiving this because you were mentioned.Message ID: @.***>

norkans7 commented 7 months ago

Replaced by #1209