/autolink command to edit configuration #71

levb · 2019-08-02T02:11:49Z

This PR was prompted by a plethora of tickets for autolink config changes on the community servers. Since there is no UI in the foreseeable future, and modifying configuration otherwise is difficult, I figured these would be useful tools for the more advanced administrators.

The command can be turned off entirely with a config setting in the system console.

/autolink help output:

Mattermost Autolink Plugin Administration

is either the Name of a link, or its number in the /autolink list output. A partial Name can be specified, but some commands require it to be uniquely resolved.

/autolink list - list all configured links.
/autolink list <linkref> - list a specific link.
/autolink test <linkref> test-text... - test a link on a sample.
/autolink enable <linkref> - enable a link.
/autolink disable <linkref> - disable a link.
/autolink add <name> - add a new link, named .
/autolink delete <linkref> - delete a link.
/autolink set <linkref> <field> value... - sets a link's field to a value. The entire command line - after is used for the value, unescaped, leading/trailing whitespace trimmed.

Example:

/autolink add Visa
/autolink disable Visa
/autolink set Visa Pattern (?P<VISA>(?P<part1>4\d{3})[ -]?(?P<part2>\d{4})[ -]?(?P<part3>\d{4})[ -]?(?P<LastFour>[0-9]{4}))
/autolink set Visa Template VISA XXXX-XXXX-XXXX-$LastFour
/autolink set Visa WordMatch true
/autolink test Visa 4356-7891-2345-1111 -- (4111222233334444)
/autolink enable Visa

- Using `\b` instead of a homebrewed word separator list - Since `\b` does not consume, removed the second pass ReplaceAll - Made tests more realistic by running MessageWillBePosted, to cover markdown inspection - Added word boundary tests - Cleanup

…link into lev-MM-15965-boundaries

…link into lev-command-config

Also - fixed the help message - fixed incorrect parsing of "false" - moved command [un-]registration OnConfigurationChanged

levb · 2019-08-02T02:27:44Z

server/config.go

+		*conf = c
+	})
+
+	go func() {


The go func() {} here is a suggestion to work around mattermost/mattermost#11780 which may not be available until mattermost-server 5.15 or later. Thanks @lieut-data for the suggestion.

levb · 2019-08-02T02:32:47Z

server/link.go

+
+	pattern := l.Pattern
+	template := l.Template
+	multipass := false


This (multipass) probably warrants a comment. Previously, word boundaries were defined as custom regexp's. These patterns consume characters, so they can not be used with ReplaceAll functions. Example, if the pattern is defined as "X", "foo X bar" the X will match, but in "foo X X bar" only the first X matches, the match consumes the subsequent space, and then the second X doesn't match.

see the Replace() method for more.

Can this be a comment in the code?

With better naming, didn't need to comment as much

levb · 2019-08-02T02:36:14Z

server/command.go

+func saveConfigLinks(p *Plugin, links []Link) error {
+	conf := p.getConfig()
+	conf.Links = links
+	appErr := p.API.SavePluginConfig(conf.ToConfig())


.ToConfig() here converts the Link structs into a tree of map[string]interface{} and []interface{} for GOB/RPC compatibility.

aaronrothschild · 2019-08-02T03:51:58Z

@levb I'm surprised there's no interactive regex editor. ;)

Is there a spinmint or test server I can try this code out on interactively?

levb · 2019-08-02T04:19:07Z

@aaronrothschild I spun one up on the related PR, mattermost/mattermost#11780

aaronrothschild · 2019-08-02T23:04:01Z

@levb some questions/comments:

In /autolink list we should display if they are active or not
in /autolink help results there is /autolink test Vi 4356-7891-2345-1111 -- (4111222233334444) the Vi should presumably be Visa
What does "WordMatch" flag do?
What is "field" in this command used for? "/autolink set value... - sets a link's field to a value. The entire command line after is used for the value, unescaped, leading/trailing whitespace trimmed."

DHaussermann

Once @aaronrothschild is stratified please merge, I can test this post merge.

levb · 2019-08-07T03:37:46Z

@aaronrothschild

In /autolink list we should display if they are active or not

it kind of does, the disabled links are crossed out

in /autolink help results there is /autolink test Vi 4356-7891-2345-1111 -- (4111222233334444) the Vi should presumably be Visa

Vi is a partial match, should work.

What does "WordMatch" flag do?

Uses the more standard \b word boundaries. "\b - at ASCII word boundary (\w on one side and \W, \A, or \z on the other)". We historically implemented a custom set for the left and right boundaries, and it can (has) lead to confusion. I think we should eventually move towards making \b the default, and deprecating the old-style separators, but not yet.

What is "field" in this command used for? "/autolink set value... - sets a link's field to a value. The entire command line after is used for the value, unescaped, leading/trailing whitespace trimmed."

The name of the field. Like, for Name, Pattern, or DisableNonWordPrefix

mickmister

A big improvement in the UX of the plugin, great job @levb 👍

crspeller

One non blocking nit.

crspeller · 2019-08-06T20:57:48Z

server/link.go

+
+	pattern := l.Pattern
+	template := l.Template
+	multipass := false


Can this be a comment in the code?

DHaussermann · 2019-08-13T03:45:46Z

@levb or @aaronrothschild I see that this PR is still open. Can you please advise if we intend to package this for the 5.14 Release?

levb · 2019-08-13T13:00:37Z

This is for after 5.14 @DHaussermann, but I am merging this now, and will cut a release shortly. I'd like to deploy it onto community this week, once we confirm it on ci-extensions.

* Update README for #43, #71 * Added Regular expression tips

Lev Brouk added 8 commits July 10, 2019 05:55

Merge branch 'master' of github.com:mattermost/mattermost-plugin-auto…

a56253d

…link into lev-MM-15965-boundaries

added config commands

7e99caf

Merge branch 'master' of github.com:mattermost/mattermost-plugin-auto…

4d02687

…link into lev-command-config

fixed paths

c6c3c7e

Legacy word boundaries and tests

923a3c9

Added EnableAdminCommand config setting

b8b3f0c

Also - fixed the help message - fixed incorrect parsing of "false" - moved command [un-]registration OnConfigurationChanged

Added /autolink test, fixed deadlock,

7453c47

levb added 1: PM Review Requires review by a product manager 2: Dev Review Requires review by a core committer labels Aug 2, 2019

levb requested review from aaronrothschild, lieut-data, crspeller, DHaussermann and jfrerich and removed request for lieut-data August 2, 2019 02:11

levb commented Aug 2, 2019

View reviewed changes

levb mentioned this pull request Aug 2, 2019

MM-17489: fixed racy tryExecutePluginCommand mattermost/mattermost#11780

Merged

DHaussermann approved these changes Aug 2, 2019

View reviewed changes

jfrerich requested review from lieut-data and removed request for jfrerich August 4, 2019 17:10

aaronrothschild approved these changes Aug 7, 2019

View reviewed changes

aaronrothschild removed the 1: PM Review Requires review by a product manager label Aug 7, 2019

levb requested review from mickmister and removed request for lieut-data August 7, 2019 03:31

mickmister approved these changes Aug 7, 2019

View reviewed changes

crspeller approved these changes Aug 8, 2019

View reviewed changes

server/link.go Outdated

pattern := l.Pattern

template := l.Template

multipass := false

Copy link

Contributor

crspeller Aug 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be a comment in the code?

Lev Brouk added 2 commits August 12, 2019 18:11

/autolink help formatting, style

4e74314

PR feedback: renamed multipass to canReplaceAll, commented

be37ac9

hanzei added 4: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a core committer labels Aug 13, 2019

hanzei assigned aaronrothschild and levb Aug 13, 2019

levb merged commit 39be0a4 into mattermost-community:master Aug 13, 2019

levb deleted the lev-command-config branch August 13, 2019 13:02

This was referenced Aug 21, 2019

Allow configuration changes via the System Console #31

Open

Unify handling of opening and closing parentheses #40

Closed

Allow channel admins create custom groups #58

Closed

Escaping non special chars in the Pattern #63

Closed

jasonblais mentioned this pull request Aug 21, 2019

Update README for #43, #71 #57

Closed

levb mentioned this pull request Aug 21, 2019

Add /autolink import/export capability #73

Open

rohanjulka19 added a commit to rohanjulka19/mattermost-plugin-autolink that referenced this pull request Sep 1, 2019

Update README for mattermost-community#43, mattermost-community#71

114d119

rohanjulka19 added a commit to rohanjulka19/mattermost-plugin-autolink that referenced this pull request Sep 1, 2019

Update README for mattermost-community#43, mattermost-community#71

9544412

levb pushed a commit that referenced this pull request Sep 6, 2019

Update README for #43, #71 (#74)

8d31acb

* Update README for #43, #71 * Added Regular expression tips

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

/autolink command to edit configuration #71

/autolink command to edit configuration #71

levb commented Aug 2, 2019

levb Aug 2, 2019

levb Aug 2, 2019

crspeller Aug 6, 2019

levb Aug 13, 2019

levb Aug 2, 2019

aaronrothschild commented Aug 2, 2019 •

edited

Loading

levb commented Aug 2, 2019

aaronrothschild commented Aug 2, 2019

DHaussermann left a comment

levb commented Aug 7, 2019

mickmister left a comment

crspeller left a comment

crspeller Aug 6, 2019

DHaussermann commented Aug 13, 2019

levb commented Aug 13, 2019

/autolink command to edit configuration #71

/autolink command to edit configuration #71

Conversation

levb commented Aug 2, 2019

Mattermost Autolink Plugin Administration

levb Aug 2, 2019

Choose a reason for hiding this comment

levb Aug 2, 2019

Choose a reason for hiding this comment

crspeller Aug 6, 2019

Choose a reason for hiding this comment

levb Aug 13, 2019

Choose a reason for hiding this comment

levb Aug 2, 2019

Choose a reason for hiding this comment

aaronrothschild commented Aug 2, 2019 • edited Loading

levb commented Aug 2, 2019

aaronrothschild commented Aug 2, 2019

DHaussermann left a comment

Choose a reason for hiding this comment

levb commented Aug 7, 2019

mickmister left a comment

Choose a reason for hiding this comment

crspeller left a comment

Choose a reason for hiding this comment

crspeller Aug 6, 2019

Choose a reason for hiding this comment

DHaussermann commented Aug 13, 2019

levb commented Aug 13, 2019

aaronrothschild commented Aug 2, 2019 •

edited

Loading