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

[Bots] Loop/Records before sending only sends original message #842

Closed
iammattmartin opened this Issue Dec 28, 2018 · 5 comments

Comments

Projects
2 participants
@iammattmartin
Copy link

iammattmartin commented Dec 28, 2018

We've got a bot that adds the previous messages in to the response sent via email. This has been working fine until 9.1 where now instead of saving the message and sending the history to the recipient, it ignores what message you enter, re-saves the first message in the ticket and sends that to the recipient.

Bot code below.

{
    "behavior": {
        "uid": "behavior_366",
        "title": "testt",
        "is_disabled": false,
        "is_private": false,
        "priority": 91,
        "event": {
            "key": "event.mail.before.sent",
            "label": "Before message sent from worker"
        },
        "variables": {
            "var_ticket_id": {
                "key": "var_ticket_id",
                "label": "Ticket ID",
                "type": "N",
                "is_private": "0",
                "params": []
            },
            "var_message_id": {
                "key": "var_message_id",
                "label": "Message ID",
                "type": "N",
                "is_private": "0",
                "params": []
            },
            "var_messages": {
                "key": "var_messages",
                "label": "Messages",
                "type": "ctx_cerberusweb.contexts.message",
                "is_private": "1",
                "params": []
            }
        },
        "nodes": [
            {
                "type": "switch",
                "title": "Check if supplier",
                "status": "live",
                "nodes": [
                    {
                        "type": "outcome",
                        "title": "Yes",
                        "status": "live",
                        "params": {
                            "groups": [
                                {
                                    "any": 0,
                                    "conditions": [
                                        {
                                            "condition": "group_name",
                                            "oper": "contains",
                                            "value": "Supplier"
                                        }
                                    ]
                                }
                            ]
                        },
                        "nodes": [
                            {
                                "type": "action",
                                "title": "Signature",
                                "status": "live",
                                "params": {
                                    "actions": [
                                        {
                                            "action": "_set_custom_var_snippet",
                                            "on": "group_id",
                                            "snippet_id": "94",
                                            "var": "signaturetext"
                                        },
                                        {
                                            "action": "_set_custom_var_snippet",
                                            "on": "group_id",
                                            "snippet_id": "95",
                                            "var": "signaturehtml"
                                        },
                                        {
                                            "action": "prepend_to_content",
                                            "content": "{% if content_format %}\r\n<strong>Our Reference:</strong> {{ticket_mask}}\r\n{% if ticket_custom_52 != NULL %}<strong>Supplier Reference:</strong> {{ticket_custom_52}} {% endif %}{% if ticket_custom_51 != NULL %}<strong>CLI:</strong> {{ticket_custom_51}} {% endif %}\r\n\r\n{% else %}\r\nOur Reference: {{ticket_mask}}\r\n{% if ticket_custom_52 != NULL %}Supplier Reference: {{ticket_custom_52}} {% endif %}{% if ticket_custom_51 != NULL %}CLI: {{ticket_custom_51}}\r\n{% endif %}{% endif %}",
                                            "mode": "sent"
                                        },
                                        {
                                            "action": "append_to_content",
                                            "content": "{% if content_format %}\r\n<p id=\"small\">{{signaturehtml}}</p>\r\n{% else %}\r\n\r\n{{signaturetext}}{% endif %}",
                                            "mode": "sent"
                                        }
                                    ]
                                }
                            },
                            {
                                "type": "action",
                                "title": "Load Messages",
                                "status": "live",
                                "params": {
                                    "actions": [
                                        {
                                            "action": "var_messages",
                                            "search_mode": "quick_search",
                                            "quick_search": "ticket.id:{{ticket_id}} sort:-created",
                                            "limit": "",
                                            "limit_count": "10",
                                            "mode": "replace",
                                            "worklist_model": null
                                        }
                                    ]
                                }
                            },
                            {
                                "type": "loop",
                                "title": "Loop Messages",
                                "status": "live",
                                "params": {
                                    "foreach_json": "{{var_messages|keys|json_encode}}",
                                    "as_placeholder": "message_id"
                                },
                                "nodes": [
                                    {
                                        "type": "action",
                                        "title": "Insert",
                                        "status": "live",
                                        "params": {
                                            "actions": [
                                                {
                                                    "action": "append_to_content",
                                                    "content": "{% set message = var_messages[message_id] %}\r\n{% set content = message.content|trim %}\r\n\r\nOn {{message.created|date('jS M Y')}} at {{message.created|date('g:i:s A')}}, {{message.headers.from}} wrote:\r\n{{content|trim|quote}}",
                                                    "mode": "sent"
                                                }
                                            ]
                                        }
                                    }
                                ]
                            }
                        ]
                    }
                ]
            }
        ]
    }
}

@jstanden jstanden added this to In Development in 9.1.3 Dec 28, 2018

@jstanden jstanden changed the title Loop/Records before sending only sends original message [Bots] Loop/Records before sending only sends original message Dec 28, 2018

@jstanden

This comment has been minimized.

Copy link
Owner

jstanden commented Dec 28, 2018

It looks like I can reproduce the issue with your example behavior.

I'm still investigating to issue a patch.

@jstanden

This comment has been minimized.

Copy link
Owner

jstanden commented Dec 28, 2018

It looks like this happens when the behavior uses a temporary variable with the same name as a placeholder. For instance, you're using content = message.content in the append action at the end.

If you rename the variable to something like message_content it avoids the issue.

{% set message = var_messages[message_id] %}
{% set message_content = message.content %}

On {{message.created|date('jS M Y')}} at {{message.created|date('g:i:s A')}}, {{message.headers.from}} wrote:
{{message_content|trim|quote}}

This has to do with how we merge our placeholders into the template builder (Twig) scope for conditions/actions. I'm still testing a few alternative approaches. But tweaking your behavior (above) will resolve the issue in the meantime.

@iammattmartin

This comment has been minimized.

Copy link

iammattmartin commented Dec 28, 2018

This worked - thanks!

I wonder if there should be some checking so conflicting names can't be used or at least a plugin or similar that could detect potentially problematic bots and variables?

@jstanden

This comment has been minimized.

Copy link
Owner

jstanden commented Dec 29, 2018

Yeah. Most behaviors just get a copy of the values. Normally, temporary variables within a template wouldn't conflict with placeholders. They'd be thrown away after the template compiles to text.

This is a special case where the 'Before sending worker message' events retain a reference/pointer from the original properties (To/Cc/Subject/Headers/Content) all the way through the behaviors. That allows the behavior to change the properties of the original message without adding a bunch of new actions. So changes to those placeholders do flow back up to the objects behind the placeholders.

I can see where this is happening, and I'm going to go about it a better way.

jstanden added a commit that referenced this issue Dec 29, 2018

* [Bots/Scripting] Fixed an issue in bot behavior scripts where tempo…
…rary variables could overwrite placeholders of the same name.

Fixes #842
@jstanden

This comment has been minimized.

Copy link
Owner

jstanden commented Dec 29, 2018

k, I found a better way to solve this. The temporary variables within a script won't affect placeholders of the same name on behaviors like this.

Thanks for catching this!

@jstanden jstanden closed this Dec 29, 2018

9.1.3 automation moved this from In Development to Completed! Dec 29, 2018

@jstanden jstanden added this to the 9.1.1 milestone Dec 29, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment