-
Notifications
You must be signed in to change notification settings - Fork 71
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
Set HX user_data parameter as flow name #400
Conversation
Codecov Report
@@ Coverage Diff @@
## main #400 +/- ##
==========================================
- Coverage 72.02% 71.95% -0.08%
==========================================
Files 95 95
Lines 8330 8348 +18
==========================================
+ Hits 6000 6007 +7
- Misses 1736 1747 +11
Partials 594 594
Continue to review full report at Codecov.
|
@norkans7 @ericnewcomer maybe before fixing this, we start queuing messages in mailroom with their flow name and use that? |
Sure, I had just assumed that was broken because the parameter was changed(or we had that wrong all the time) |
I can make the change on the mailroom side but are we good with flow name? UUID? I'm leaning toward name.. |
I think mailroom sends both the same as the archiver so we are very specific and HX we can only send one of those if that is what is needed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We actually want to change the user_data
param to be the flow name
backends/rapidpro/msg.go
Outdated
@@ -518,6 +518,8 @@ type DBMsg struct { | |||
SessionWaitStartedOn_ *time.Time `json:"session_wait_started_on,omitempty"` | |||
SessionStatus_ string `json:"session_status,omitempty"` | |||
|
|||
Flow_ json.RawMessage `json:"flow,omitempty"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no reason to not parse it all in one go.. you can even use a anon struct like
Flow_ *struct {
UUID string `json:"uuid"`
Name string `json:"name"`
} `json:"flow"`
``
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What type will Flow()
will return there?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
never mind no need for anymore
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if you need a type just add something like MsgFlowRef
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am confused on how to use that I will try that again tomorrow
"to": "+250788383383", | ||
"ret_id": "10", | ||
"datacoding": "8", | ||
"user_data": "", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A message not from a flow will have the user_data as empty string
msg.go
Outdated
Name string `json:"name"` | ||
} | ||
|
||
var NilMsgFlowRef = MsgFlowRef{"", ""} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't want to serialize messages with "flow": {"uuid": "", "name": ""}
.. should be a pointer on Msg
which can be nil if there's no flow.
While you're changing this let's call it just FlowReference
for simplicity and consistency with mailroom/goflow.
handlers/test.go
Outdated
Flow *struct { | ||
UUID string | ||
Name string | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not use the FlowReference
type here?
Flow: &struct { | ||
UUID string | ||
Name string | ||
}{UUID: "9de3663f-c5c5-4c92-9f45-ecbc09abcc85", Name: "Favorites"}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use FlowReference
type
depends on https://github.com/nyaruka/mailroom/pull/571