[FR] customize the link target

[dlindberg]

First, thank you, this alleviates a tremendous amount of screaming across the office. But email is a bit slow for responses, so at least in our office, there is more screaming that could be eliminated.

The specific use case I'm thinking of is that instead of opening an email opening up a slack conversation with the other person editing the entry would let people resolve the situation faster (hopefully stop people from just yelling, hey can you close some entry).

But that seems too specific, the more basic idea is that the root of the URL could be set as part of the config. And an additional config would tell the plugin what custom filed should be appended to that root. It would let people integrate it with whatever tools their office happens to use (Slack, Hangouts, Teams, or whatever is new next week) without the plugin needing to actually know about any of them.

If you think it is a good idea / good solution I'd be happy to try and put together a basic PR on it too if that would be helpful.

[marionnewlevant]

That is an excellent idea. Maybe a bit of twig that took the user and generated the message? People could put their slack id in the user as a custom field (or whatever is needed that week). A pull request would be awesome.

On Mon, May 27, 2019 at 8:02 AM Dane Lindberg notifications@github.com wrote:

First, thank you, this alleviates a tremendous amount of screaming across the office. But email is a bit slow for responses, so at least in our office, there is more screaming that could be eliminated.

The specific use case I'm thinking of is that instead of opening an email opening up a slack conversation https://api.slack.com/docs/deep-linking#client with the other person editing the entry would let people resolve the situation faster (hopefully stop people from just yelling, hey can you close some entry).

But that seems too specific, the more basic idea is that the root of the URL could be set as part of the config. And an additional config would tell the plugin what custom filed should be appended to that root. It would let people integrate it with whatever tools their office happens to use (Slack, Hangouts, Teams, or whatever is new next week) without the plugin needing to actually know about any of them.

If you think it is a good idea / good solution I'd be happy to try and put together a basic PR on it too if that would be helpful.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/marionnewlevant/craft-snitch/issues/14?email_source=notifications&email_token=AAFJZNPIG5SPEVEUXOEPXCLPXPZXLA5CNFSM4HP4XVG2YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4GWBWA3A, or mute the thread https://github.com/notifications/unsubscribe-auth/AAFJZNNQ3W7LWY6BFFL7VLLPXPZXLANCNFSM4HP4XVGQ .

[dlindberg]

Ahh, I like the idea of just making it a custom twig thing. Then people can do whatever they want and it handles an even broader class of things.

Not sure if there is a way to reliably evaluate message with just a token or as twig without causing a breaking change. That seems like the simplest solution if it works reliably

I’ve certainly seen a number of plugins that have you configure a custom route to a twig template, so probably a couple of options on implementation. One or the other should work.

[marionnewlevant]

Could have a new twigMessage config, and use the old one if that doesn't exist. I haven't looked at it hard, but for most things I would think just a bit of twig in the config file should work. It could have the user and the entry to work with, and go from there.

On Tue, May 28, 2019 at 4:21 AM Dane Lindberg notifications@github.com wrote:

Ahh, I like the idea of just making it a custom twig thing. Then people can do whatever they want and it handles an even broader class of things.

Not sure if there is a way to reliably evaluate message with just a token or as twig without causing a breaking change. That seems like the simplest solution if it works reliably

I’ve certainly seen a number of plugins that have you configure a custom route to a twig template, so probably a couple of options on implementation. One or the other should work.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/marionnewlevant/craft-snitch/issues/14?email_source=notifications&email_token=AAFJZNIPLU7ADDQ76MA56GLPXUISBA5CNFSM4HP4XVG2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWLZU6Q#issuecomment-496474746, or mute the thread https://github.com/notifications/unsubscribe-auth/AAFJZNLN426J6GG7XUTZPALPXUISBANCNFSM4HP4XVGQ .

[dlindberg]

I managed to spend a little bit of time looking at it this evening. Rendering a template from a string looks to be relatively straightforward using the renderString($string, $variables) method.

It looks like currently the message is rendered completely in JavaScript. My thought is to adjust that so and add another controller actionGetMessage() and an AJAX call that would get called if the message for a specific user doesn't already exist. getMessage could then operate on the message string by seeing if the {user} token exists. If it doesn't it would just parse it as a twig template Craft::$app->users->getUserById($id) passed to it as the variable. I think that should let the change be backwards compatible without needing an extra config section.

I'll need to work at it a little bit more, but I think it should work.

[marionnewlevant]

I wish I knew how many people are actually using that config value that lets you change the message. Quite possibly it is few enough that not being backwards compatible wouldn't be a big deal. I think on the whole I would rather keep things simple than maintain backwards compatibility.

On Fri, May 31, 2019 at 7:53 PM Dane Lindberg notifications@github.com wrote:

I managed to spend a little bit of time looking at it this evening. Rendering a template from a string looks to be relatively straightforward using the renderString($string, $variables) https://github.com/craftcms/cms/blob/master/src/web/View.php#L448-L458 method.

It looks like currently the message is rendered completely in JavaScript. My thought is to adjust that so and add another controller actionGetMessage() and an AJAX call that would get called if the message for a specific user doesn't already exist. getMessage could then operate on the message string by seeing if the {user} token exists. If it doesn't it would just parse it as a twig template Craft::$app->users->getUserById($id) passed to it as the variable. I think that should let the change be backwards compatible without needing an extra config section.

I'll need to work at it a little bit more, but I think it should work.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/marionnewlevant/craft-snitch/issues/14?email_source=notifications&email_token=AAFJZNIXQKOIZKHMIMTV623PYHQEBA5CNFSM4HP4XVG2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWWXGYI#issuecomment-497906529, or mute the thread https://github.com/notifications/unsubscribe-auth/AAFJZNIWUN33H4NLAEZIQETPYHQEBANCNFSM4HP4XVGQ .

[dlindberg]

Oh, you're right that would introduce a gross code split...

It is a bunch more code changes, but what if all the html creation moves to the server side.

It would need the controller actionGetMessage() that takes the userId as it's request parameter and returns the rendered message string by calling a new service:

    /**
     * Rough code, not tested yet, could also have typos
     * 
     * @param int $id
     * @return string
     * @throws \Twig\Error\LoaderError
     * @throws \Twig\Error\SyntaxError
     */
    public function renderCollisionMessage(int $id)
    {
        /**
         * Convert {user} token into twig templpate equivalent
         */
        $message = \str_replace(
            ' {user} ',
            ' <a href="mailto:{{ user.email }}">{{ user.friendlyName}}</a> ',
            Snitch::$plugin->getSettings()['message']
        );

        return (Craft::$app->getView())->renderString(
            '<div data-snitching="' . $id . '"><div>' . $message . '<span>&times;</span></div></div>',
            ['user' => Craft::$app->users->getUserById($id)]
        );
    }

Then only the user IDs would need to be passed until the script decided it needs to alert, so userData could just return an array of IDs (yay skip the DB call for user details here) and actionGetConfig wouldn't need to send up the message config.

I think that ends up necessitating a whole bunch of changes in the JavaScript bundle that I've not gotten into yet...

[marionnewlevant]

Version 3.0.0 Switched the config to messageTemplate with twig, old message config is ignored. Server returns the parsed message, parsed with user in the context. No easy way to hand the entry to twig for parsing along with user - might be a field, might be something else.