481539 - Implement variables and wiki links in Live Chat

Status: RESOLVED WONTFIX

(support.mozilla.org Graveyard :: Chat, enhancement)

Description

[Matthew Middleton (:zzxc)]

Live Chat should support %VARIABLE% style responses to allow helpers to link customized CSAT links during the chat session, rather than waiting until ending the chat session.  Other uses might include customizing responses for the user's operating system or giving out a customized forum link.  These will be used mainly in canned responses.

Possible variables to include:
%sessionID% - sessionID of the current chat
%agentNick% - nickname of the helper
%userNick% - nickname of the user
%csatLink% - CSAT url for the current chat

Tikiwiki-style links, such as ((Safe Mode)), should also be supported.  These will allow easy linking to knowledge base articles and are already familiar to most sumo contributors.  It may also be desirable to allow __bold__ and ''italic'' formatting as well.

Comment 1

[Paul Craciunoiu [:paulc]]

I like shorter variable names, I think they're easier to remember.
%agent%, %user%, %id%, %link% -- of course, this depends on how likely it is to have the types of URL extended or such. But if this basic info will *always* stay the same, it may be good to give the variables short names.

Comment 2

[Matthew Middleton (:zzxc)]

Attachment: Patch v1

This implements the initial four wiki variables (nick, agent, forum, and csat) and enables processing of TikiWiki syntax.

Currently, the CSAT and KB links are hardcoded, but these will need to be implemented as configuration options for SUMO productization.

Comment 3

[Christopher Lynch [:Christopher]]

I especially like the idea of TikiWiki links ex. ((Safe Mode)) but the variables are good ideas too.

Comment 4

[Matthew Middleton (:zzxc)]

Attachment: Return null when variable not matched

A slight change to the previous patch for WikiReplacer functions to return null when a variable is not found, to preserve variables that shouldn't be modified like %appdata%.

Comment 5

[Austin King [:ozten]]

Comment on attachment 386364 [details] [diff] [review]
Return null when variable not matched

Good work, this will be a cool feature! I am r- for 1 specific change and the list of suggestions are optional feedback. All line # below are in WikiFilter.java .

Please change:
Line 90 - extract a method from the 30 lines of code here, so that case 2: just calls that method and has a break.

you can get ride of the fall-through in case 1 by calling this method from case 1 and case 2.

Observation:
I agree that the urls should be pulled out into config, as soon as possible, but not a blocker for code review.

Suggestions:
Wiki line 30, 61 WikiPeplacer.isValidKeyword - why does it take a ChatSession as an argument? Do you see this use case in the next release, otherwise remove it.

line: 61 - consider using a Map instead of an array. Not an issue with 4 variables, but if you had
{ "agent": 0, "csat": 1, "forum":2, "nick": 3} then you can just check to see if the Map contains the key instead of having a for loop and doing a search through the list of variable names.

Line 154-162:
Do you have unit tests for your regular expressions? Maybe make them accessible as members or methods on the object and then you can unit test them for good and bad input.

Observation:
Line 176: Using the replaceByGroup method, we will walk through the string once per type of variable syntax which the system supports. This is a good way to build the feature initially. If performance becomes an issue, we may want to tokenize the line on whitespace and walk through the tokens using regexp or even simple sub string comparisons against the list of variable syntax. This way we would only process a line once regardless of how many variables we support.

Comment 6

[Kadir Topal [:atopal]]

I'm WONTFIXing this, since we don't have Live Chat at the moment and even if we decide to have one in the future it would be using different technology.