All committers have signed the CLA.

Look good! AFAICT this isn't the "new info" one, but just the "mattermost-specific" one. Which I'm quite happy with!

Questions/remarks:

• Re. duplication: I'd much prefer to (as you did) just copy/paste and adapt; understanding the duplication is expected and straightforward (more so than trying to parse some complicated call tree); at least at this stage this is the way.

• If/once logic for environment/release is introduced, it should be conditional (upon that information being available)

• For choices, I'd prefer to factor that out into a method as done here; this avoids completely unnecessary "migrations" (that don't affect the DB at all). If figuring out what I mean by that is annoying to you I'm perfectly happy to do that work myself.

• There's a few unnecessary reformats in the commit; in general I'm not a big fan of those as they make judging things harder but I don't hate them so much that I'm going to ask you to revert now :-)

• just checking: isn't the channel information configured/configurable via the webhook url?

Look good! AFAICT this isn't the "new info" one, but just the "mattermost-specific" one.
Which I'm quite happy with!

"isn't the new info" I don't quite understand but I take the "quite happy with" :)

• Re. duplication: I'd much prefer to (as you did) just copy/paste and adapt; understanding the duplication is expected and straightforward (more so than trying to parse some complicated call tree); at least at this stage this is the way.

Cool. Yeah, it felt like the better approach than cramming both into one, making a mess.

• If/once logic for environment/release is introduced, it should be conditional (upon that information being available)

The way I would suggest to implement it is to have a default format that could be overridden. So there really isn't a need for conditionals. We could start with a simpler format - if that is what you have in mind.

Question is whether that should be a configuration or an env setting.
Since we already have config settings for the channel, it might make sense to skip the env var and also add it as config?
WDYT?

• For choices, I'd prefer to factor that out into a method as done here; this avoids completely unnecessary "migrations" (that don't affect the DB at all). If figuring out what I mean by that is annoying to you I'm perfectly happy to do that work myself.

Yeah, I wasn't so happy to introduce a migration just for that.
It sure is the cleaner route but - we can also model it a little weaker.
As you like.

• There's a few unnecessary reformats in the commit; in general I'm not a big fan of those as they make judging things harder but I don't hate them so much that I'm going to ask you to revert now :-)

I am not a fan of that either, sorry.
I had auto format turned on in the editor and when I pressed save I was "uff!" and it was too late.
Since it's mostly copied code I was hoping to get away with it.

Might be good to have some auto formatting for the project to avoid this pain.

• just checking: isn't the channel information configured/configurable via the webhook url?

You can configure only the default channel. The actual channel you have in the payload.
Otherwise you would have to setup dozens of webhooks just to post to different channels.
Doable - but painful.

"isn't the new info" I don't quite understand but I take the "quite happy with" :)

I meant "actually exposing environment/release" which you were also commenting on elsewhere.

The way I would suggest to implement it is to have a default format that could be overridden. So there really isn't a need for conditionals. We could start with a simpler format - if that is what you have in mind.

Question is whether that should be a configuration or an env setting. Since we already have config settings for the channel, it might make sense to skip the env var and also add it as config? WDYT?

Settings or not, the code should be robust (and the result non-ugly) for the fact that this information can dynamically be there or not.

Yeah, I wasn't so happy to introduce a migration just for that. It sure is the cleaner route but - we can also model it a little weaker. As you like.

I'm a fan of "strong model, but no (or rather: a single) migration" which is what the linked code does.

Might be good to have some auto formatting for the project to avoid this pain.

《mumbles inaudibly》

• just checking: isn't the channel information configured/configurable via the webhook url?

You can configure only the default channel. The actual channel you have in the payload. Otherwise you would have to setup dozens of webhooks just to post to different channels. Doable - but painful.

Great; channels Bugsink-side it is.

Yeah, I wasn't so happy to introduce a migration just for that. It sure is the cleaner route but - we can also model it a little weaker. As you like.

I'm a fan of "strong model, but no (or rather: a single) migration" which is what the linked code does.

OK, let's follow that then.

Might be good to have some auto formatting for the project to avoid this pain.

《mumbles inaudibly》

Hehehe

I personally find it freeing not having to worry about it anymore.
I've made have good experiences with it in different teams and projects.
But I know this can be a touchy subject.

Let's pretend I never mentioned it ;-)

You can configure only the default channel. The actual channel you have in the payload. Otherwise you would have to setup dozens of webhooks just to post to different channels. Doable - but painful.

Great; channels Bugsink-side it is.

Alright. 👍

Alright. Did some reverts and refactoring. I think it's getting close to what I had in mind.

Not quite sure what display_name is supposed to be. Didn't test that yet.

But maybe have another look if this make sense or what needs to be changed.

I don't understand why the db test failed now that the migration is gone

Hm... 0004_alter_messagingserviceconfig_kind.py isn't even part of the PR

Just had to rebase to lastest head.

Can you have another look what else needs to be fixed for a merge? @vanschelven

I'm sorry I let this linger for a bit longer than I said I would.

I'd be super-happy to take a mattermost-specific backend; but as it stands this PR just "does too much", mostly as per this comment;

I'm sorry if I didn't express myself clearly enough over there, but I really meant it when I said that I'm always very worried about taking on new point of configuration, and I really don't see the upside here.

I'd much rather "just" have the mattermost backend and potentially a richer one (as per the general ideas in #233) than yet another layer of configuration.

I'm sorry if I didn't express myself clearly enough over there, but I really meant it when I said that I'm always very worried about taking on new point of configuration, and I really don't see the upside here.

I'd much rather "just" have the mattermost backend and potentially a richer one (as per the general ideas in #233) than yet another layer of configuration.

I really don't see the downside here - if a value is missing it would be just missing. I cannot imagine a breakable change. I just do not see the pain for users or maintainers. Could you elaborate on your concerns a little more?

But if the status quo is out of the question:

What about just NOT exposing the format strings to the config and keep the rest as is?
It would become an implementation detail.

If that's not enough I need a little more guidance on what changes you would like to see.

PS: Any idea why the test are failing? Seems to be unrelated.

@vanschelven any pointers?

Should I just remove the format config?

remove the format config

that would be much appreciated!

Other than that, I'm generally more slow to respond that I personally want to; in other words "it's not you it's me" 😄

@vanschelven I have commented out the configuration of the format strings

If this is what you have in mind we can also remove the comments.
But I thought it would be good to take one step at a time.

Let me know if that works for you.

@vanschelven could you give some guidance if something else needs to be changed?

We would really would love to see proper mattermost support.

@tcurdt sorry for letting this linger for so long; other things were demanding my attention, but I finally got round to picking it up. When I did, I simply "hijacked" the PR to #278 such that I could freely "take it from here".

The version that made it to main in the end is more or less a copy of the Slack backend, but speaking the mattermost protocol.

Templating didn't make it, and as per above it's unlikely that it will do so "soon".

Channels could make it to a future version, but if they do should probably be done for all backends that support them at once.

Similar for color-coding and other things that I might have overly aggressively pruned.

Last but not least: thank you very much for your contribution! 🎉

Well, the very least we need channel support so seems like the contribution limbo wasn't all that useful in the end. A shame.

thing is, the provided PR did not actually provide a UI for channels-support...

thing is, the provided PR did not actually provide a UI for channels-support...

class MattermostConfigForm(forms.Form):
    webhook_url = forms.URLField(required=True)
    channel = forms.CharField(
        required=False,
        help_text='Optional: Override channel (e.g., "town-square" or "@username" for DMs)',
    )

This needs some more UI plumbing I assume?

UI plumbing

Yes, I'll pick that up as part of #281