Add Mention Config. Implements #14, #15 and #20

[NeunEinser]

This pull request makes mentions fully configurable. This includes embeds themselves.

• Different mention types with different requirements can be configured to trigger embeds (Have different mention types invoke different embeds #15).
  • Mention types can require or forbid urls, prefixes and keywords (Don't mention ticket(s) if the message contains a certain keyword #14).
• Filter feeds also need to be assigned to a specified embed type now.
• The threshold for when a mention command's embed gets grouped is configurable as well now
  • The same is configurable per filter feed (Provide an option to not group tickets into one message in feeds #20)
  • (new) The same is now also configurable per mention type.
    • Inside a mention type, this is the maximum number of mentions in an entire message, disregarding their mention_type.
    • In case multiple mention types match a mention, the first one which allows for sending a big embed is chosen. If no mention type has a sufficiently high value, a grouped embed is created.
• The maximum amount of tickets in a grouped embed is now configurable.
  • The same is configurable per filter feed (Provide an option to not group tickets into one message in feeds #20)
  • Configuring this per mention type doesn't really make sense, as a message can contain multiple mention types, but which one should we use for this value?
• (new) A default embed which is used when no embed is given, or for the BugCommand can be configured.
• The mention command now deletes any message that solely consists of mentions.
  • Example: If MC-1 MC-2 MC-3 all trigger a mention, and the message does not contain anything else, the message will be deleted.
  • This also takes urls, required keywords and required prefixes into account.
• (new) The mention command has a more meaningful debug output now
  • The output follows the scheme of (mention1, mention2, ...), (mention1, mention2, ...), ...
  • Parenthesis group mentions that triggered for one mention type.
  • Mentions may appear duplicated if they matched different mention types.
  • A mention appears as the complete match.
    • Example: (MC-1, MC-2), (https://bugs.mojang.com/browse/MC-3), (!MC-4, !MC-5)
  • A mention type will be proceeded by the required_keyword, if available.
    • Example: !keyword(mention1, ...)
• Grouped embeds use the description now instead of fields, making them more concise
  • see also this message thread from #bot-commands
  • The final look is this:
    
• Old global mention configurations from Improvements to mention ticket command #11 have been removed.

Mention types allow for these configurations:

// Rules for invoking different embeds when mentioning a ticket.
// In case multiple types match, the one defined first will be used.
"mention_types": [
	{
		// Optional. If set to true, this mention type will only trigger if the mentioned ticket was provided as URL.
		// Default: false
		"require_url": false,

		// Optional. If set to true, this mention type will not trigger if the mentioned ticket was provided as URL.
		// Default: false
		"forbid_url": false,

		// Optional. If set, this mention type will only trigger if the mentioned ticket was prefixed with the given string.
		// If require_url is set to true, the prefix applies to the entire url.
		// Default: no required prefix.
		"required_prefix": "<prefix>",

		// Optional. If set, this mention type will not trigger if the mentioned ticket was prefixed with one of the given characters.
		// If require_url is set to true, the prefix applies to the entire url.
		// Default: no forbidden prefix.
		"forbidden_prefix": "<prefix>",

		// Optional. If set, this mention type will only trigger if the message includes a keyword somewhere in the message.
		// Default: no required keyword.
		"required_keyword": "<keyword>",

		// Optional. If set, this mention type will not trigger if the message includes a keyword somewhere in the message.
		// Default: no forbidden keyword.
		"forbidden_keyword": "<keyword>",

		// Optional. Number of ungrouped mentions that will produce an individual embed for this mention type.
		// If this number is exceeded, a single embed including all ticket keys and summaries will be generated instead.
		// A number smaller than 1 means no limit.
		// Default: Same as max_ungrouped_mentions from root.
		"max_ungrouped_mentions": 1,

		// Optional. An embed name defined in embed_types.
		// This embed will be used to display the mention.
		// Default: the embed defined in `default_embed`
		"embed": "<embed name>"
	}
	// ... You can add more mention types here
]

Embeds can be configured like this:

// Map of custom embed types
"embed_types": {
	"<name of an embed type>": {

		// Optional. Whether to include the embed title
		// Default: false
		"title": false,

		// Optional. Settings for the description. Either below's object or boolean.
		// When true, defaults apply, when false no description is set.
		// Default: false
		"description": {

			// If set, the description cannot be longer than n characters.
			// Maximum value (and default value) is 2048 (restriction by Discord api)
			"max_characters": 1234,

			// If set, the description cannot include more than n line breaks.
			"max_line_breaks": 2,

			// If set, everything from the description that matches any of the given regex strings will be stripped.
			"exclude": [ "<a regex string>" ]
		},

		// Optional. Whether the author should be included.
		// Default: false
		"author": false,
		
		// Optional. Whether the embed should link to the mentioned ticket.
		// Default: false
		"url": false,
		
		// Optional. Whether the embed should include the first image.
		// Default: false
		"thumbnail": false,

		// The color the embed should have
		"color": "DiscordColorResolvable",

		// Optional. A list of additional fields that should be added. Can be set to false for no fields.
		// Default: false
		"fields": [
			{
				// Type of the field.
				// Currently available: status, large_status, field, user, joined_array, array_count, duplicate_count, date, from_now
				"type": "<type>",

				// Label the field should appear with.
				"label": "<label>",

				// Path to the field in the jira json response. See jira api documentaion for more details.
				// Path members are seperated with dots (.) And the path starts at ticketResponse.fields
				// Not needed for status, large_status and duplicate_count
				"path": "<path-to-field>",

				// Optional. Whether the field should be inline. A maximum of 3 fields will appear next to each other when set.
				// Default: true
				"inline": false,

				// Optional. Currently only for joined_array.
				// Path for object in the array that should be used for display.
				// Default: Print entire object. 
				"inner_path": "<inner path>"
			}
			// ... You can add more fields here
		]
	}
	// ... You can define more embed types here
}

The verbose embed is missing the creator. In the current implementation, the creator is only shown when different from the reporter.
Currently, it is not possible to define conditions.

[NeunEinser]

Some stuff to think about:

Currently, the field types are pretty hardcoded.
It would be nice if you could combine literal texts and fields (still with implemented field types) into one field text.
This would e.g. make user obsolete as you could just combine literal and field types to form the string [${ user.displayName }](https://bugs.mojang.com/secure/ViewProfile.jspa?name=${ encodeURIComponent( user.name ) })

With that it would be also cool to allow for conditions like exists, is_empty, equals, and condition combiners like and/or/not to control when either a part of a field or an entire field should be shown

The same could be done for the currently hardcoded other parts of the Discord-Embed.
Stuff like description, url, thumbnail, etc. are hardcoded boolean types (except for description which at least allows for defining how much of the description should be shown).

The grouped embed is currently not configurable at all, though that isn't too much of a priority as there isn't that much potentially customizable so far anyways.

And finally an inconsistency that could be worked out before merge: As stated in the first message, the bot now deletes any message where everything in the message is either an whitespace or something that resolves entirely as mention.
There is one exception to this: Keywords.

• In case of a forbidden keyword, the keyword is not counted as matched when evaluated internally. Reason being is that another mention type might have the same forbidden keyword. If the keyword was removed, the message would still trigger however.
  • This leads to the case when a forbidden keyword is forbidden in one mention type, but not another. Then adding that keyword was likely done in order to provoke the other embed.
• In case of a required keyword, a different mention type of the same message might require the same keyword. Because of this, the required keyword is not stripped and it will cause the message to not get deleted at all.

A way to fix this would be to let all mention types try to match the entire message and then later prune duplicated tickets. This might also make it possible to add max_ungrouped_mentions per mention type.

[violine1101]

Woah, this is amazing! Thank you so much for this!

This went much further than I first imagined. I thought of renaming SingleMention to DetailedMention and then have multiple different other subclasses of Mention (such as MinimalMention) and just have them invoked by different configurable prefixes.

This also means that I need to rethink how I want to manage the whole config part. Server moderators should be able to choose which embeds they want to use and which prefixes should (or should not) trigger embeds by a command. With this custom config system being a bit more complex than I anticipated, this might be a bit more difficult to realize.

As it is quite a large change, I'd take a more detailed look at it after we've merged #19 into master.

[NeunEinser]

Yeah, I also thought about how to make it runtime-configurable.

Especially if I end up adding those things I theorized about, it's not gonna be easy.
In the earlier stages it might be best to just have a !config <key> <json>, but for people using the bot that are not as technical, we would need to figure out something.

[NeunEinser]

So, all the problems I still saw with this are fixed now.

The only thing I talked about in here which is not implemented is less hardcoded field types by allowing for combining multiple types and having conditions for different parts of a field or an entire field.

But I think the pr is big enough already and we can always make it more customizable later.

[violine1101]

Okay, I took a more detailed look at this change and there are a few things I want to say about this.

First of all, it makes everything configurable on a very atomic level. I'm not really sure if this is desirable. The definition for the different embed types need to be configured in a very rigid way in the settings.json file, making the TS code more complicated in the process.

For instance, is it necessary to be able to give fields different labels? Usually when you want a field, it will already have a name that we know of. Also, as you already mentioned, it's impossible to alter the label depending on the field's context due to the embed type config's rigid format.

Another issue where the JSON config is more hindering than helping is the fact that conditions cannot be easily implemented, like you already mentioned in your first comment.

Additionally, there will be the problem of making this work with guild- and channel-based configuration. Of course we can just say that every server has the same pool of embed types that is defined in the settings.json file, but then what's the point of having it configurable in the first place instead of just programming it in the TS files? If every server has control over all the small details of every embed, how do we design the configuration of that? How do we save it in the database? With the request channels, this was not as much of a problem since we will be the only server using them.

Instead, it might make sense to just have the embed types in different classes that can have conditions, dynamic labels, and that we can easily change and add. If they were all hidden behind the settings.json, we couldn't really collaboratively work on them either.

Now to the mention types: How should the bot act if two different mention types are triggered at once? Should it post both (this is what it currently does)? Or the first one in the list? This is not really intuitive. I propose to simplify this heavily by just having a map of mention type → embed type that might look like this:

"mention_types": {
    "url": "eigenbot",
    "none": "preview",
    "!": "none",
    ">": "detailed"
}

The keyword would just be a global option that prevents all types of embeds. Usually that would only be used for "none" type mentions, I don't think it would be used much for others. Thus I think just a simple option like "escape_keyword": "!nomention" would suffice. if it finds that string anywhere inside of a message, the bot will just ignore it. Simple as that. I don't think we need a "required keyword" option, I don't really see it being used much.

This PR adresses multiple issues at once, making it quite a mammoth change in total. I'd have liked to merge the other two changes, but since they're combined in this PR, we won't be able to do that.

Lastly, I really appreciate the effort you put into this and I feel really bad for writing this all up 😕 and I hope you can understand where I'm coming from.

[NeunEinser]

For instance, is it necessary to be able to give fields different labels? Usually when you want a field, it will already have a name that we know of.

I could use that as default value, but I do think it could be needed for some cases, especially when using stuff like joined_array, or when expanding on this to allow for combining different fields together.
I also do not really see the extra complexity added by that, could you perhaps explain that further? To me, it's just one easy thing to add which seemed useful ;)

Another issue where the JSON config is more hindering than helping is the fact that conditions cannot be easily implemented, like you already mentioned in your first comment.

I could still do that, I had some ideas for that, but I didn't want to expand the scope of this pr even further for now, without some input at the very least ;)

Additionally, there will be the problem of making this work with guild- and channel-based configuration. Of course we can just say that every server has the same pool of embed types that is defined in the settings.json file, but then what's the point of having it configurable in the first place instead of just programming it in the TS files?

So, that's something we should talk about more.
I have absolutely no idea what your exact vision of the guild config is.
So far I just assumed some fields being moved from the BotConfig to the guild config, with the same logic, perhaps using some sort of file-based storage in JSON format.
The settings.json seemed to kind of imply that some stuff would just be moved to a per-guild json.

Now, the way you talk about it now, it seems like you intend to use some other kind of storage.
Of course, I didn't have that other kind of database in mind, there is just nothing in the code indicating in which direction this is going to go.

Of course I intended for guilds to be able to configure embeds, that's why I implemented it that way.

Now to the mention types: How should the bot act if two different mention types are triggered at once? Should it post both (this is what it currently does)? Or the first one in the list? This is not really intuitive.

No, it currently uses the first one it can use. To me this behaviour is very intuitive, why would you want to trigger 2 different embeds on the same message (especially considering ideas as presented in #13).

There is some complexity however added by allowing to define max_grouped_mentions per mention type. The idea behind that is that you might not want to trigger many embeds for a detailed mention type, while you might want to allow multiple detailed embeds for a shortened type instead of grouping them.

The way I implemented it, first a set of mention types that apply for a certain ticked ID are stored in a map from ID to Set.
Then, in the run method, for each ticket the first mention type where # total mentions in the message <= maxGroupedMentions for the embed type is picked, or if none is present for at least one ID, a grouped emebd is being sent instead.
This is what might have confused you, as at first all mention types that apply are collected.

I don't think we need a "required keyword" option, I don't really see it being used much.

Well, there might be cases where you want to trigger a more detailed embed, for example when using a !verbose keyword. That was my whole idea behind that, being able to precisely configure when which embed should trigger.

If it's more desired to keep it simple, we could think about whether that's actually needed.
I think it would be a good idea to discuss some of these things on Discord anyways, with everyone.

I'd have liked to merge the other two changes

Well, It made sense to consider things like #20 while redesigning how mentions are handled.
Assuming that is one of those, what's the 2nd thing you'd consider merge-able, btw?

Lastly, I really appreciate the effort you put into this and I feel really bad for writing this all up 😕 and I hope you can understand where I'm coming from.

I kind of didn't expect you to merge it yesterday anyways, just because it's a big one, it doesn't come to me as a surprise some things needed further discussion, in fact I am pretty happy that someone took a look and gave me some input ;)

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ violine1101
❌ NeunEinser
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[violine1101]

I'm closing this PR. It's become too old to be mergeable.

I'm sorry, it's my fault that this happened; when I initially wrote the bot, I didn't expect to not be the only one working on it, and thus kept the first issues kinda vague and didn't think of sharing what my vision and plans were.

We can still take ideas and code from this PR when we finally add support for custom embeds, which I'd still like to do after we've set up proper support for having the bot on more than one server.