From 9ce374dc2d6782cde2e4f050a8a0c5ad8bdd9169 Mon Sep 17 00:00:00 2001 From: Sumukha Hegde Date: Thu, 8 Apr 2021 00:26:16 +0530 Subject: [PATCH] [CHORE] Refactor mention tracking logic (#2997) * [Improvement] Improve mentions This PR focuses on improving command, emoji, channel and user mentions. * [Tests] Added e2e tests for mention improvement * [Improvement] Modify slash command mention logic. Added slash command with argument preview Slash command should show only if the message bigins with / * Return data on search for empty text * Minor fixes * Update e2e tests * Minor fix * [FIX] allow command mentioning in between text Co-authored-by: Diego Mello --- app/containers/MessageBox/Mentions/index.js | 2 +- app/containers/MessageBox/constants.js | 1 + app/containers/MessageBox/index.js | 101 ++++++++---------- app/lib/rocketchat.js | 7 -- app/utils/avatar.js | 2 +- app/views/RoomsListView/index.js | 1 + e2e/helpers/app.js | 2 + e2e/tests/assorted/01-e2eencryption.spec.js | 1 - e2e/tests/assorted/02-broadcast.spec.js | 1 - e2e/tests/assorted/04-setting.spec.js | 6 +- e2e/tests/assorted/05-joinpublicroom.spec.js | 1 - .../assorted/08-joinprotectedroom.spec.js | 1 - e2e/tests/onboarding/06-roomslist.spec.js | 1 - e2e/tests/room/02-room.spec.js | 23 +++- e2e/tests/room/03-roomactions.spec.js | 1 - e2e/tests/room/04-discussion.spec.js | 1 - e2e/tests/room/05-threads.spec.js | 1 - e2e/tests/room/07-markasunread.spec.js | 1 - e2e/tests/room/08-roominfo.spec.js | 1 - 19 files changed, 79 insertions(+), 76 deletions(-) diff --git a/app/containers/MessageBox/Mentions/index.js b/app/containers/MessageBox/Mentions/index.js index fb845b2a..cb99dcdd 100644 --- a/app/containers/MessageBox/Mentions/index.js +++ b/app/containers/MessageBox/Mentions/index.js @@ -18,7 +18,7 @@ const Mentions = React.memo(({ mentions, trackingType, theme }) => { data={mentions} extraData={mentions} renderItem={({ item }) => } - keyExtractor={item => item.id || item.username || item.command || item} + keyExtractor={item => item.rid || item.name || item.command || item} keyboardShouldPersistTaps='always' /> diff --git a/app/containers/MessageBox/constants.js b/app/containers/MessageBox/constants.js index d5ef03e2..8fe37fc8 100644 --- a/app/containers/MessageBox/constants.js +++ b/app/containers/MessageBox/constants.js @@ -1,4 +1,5 @@ export const MENTIONS_TRACKING_TYPE_USERS = '@'; export const MENTIONS_TRACKING_TYPE_EMOJIS = ':'; export const MENTIONS_TRACKING_TYPE_COMMANDS = '/'; +export const MENTIONS_TRACKING_TYPE_ROOMS = '#'; export const MENTIONS_COUNT_TO_DISPLAY = 4; diff --git a/app/containers/MessageBox/index.js b/app/containers/MessageBox/index.js index 9a075e31..1fc874f9 100644 --- a/app/containers/MessageBox/index.js +++ b/app/containers/MessageBox/index.js @@ -41,7 +41,8 @@ import { MENTIONS_TRACKING_TYPE_EMOJIS, MENTIONS_TRACKING_TYPE_COMMANDS, MENTIONS_COUNT_TO_DISPLAY, - MENTIONS_TRACKING_TYPE_USERS + MENTIONS_TRACKING_TYPE_USERS, + MENTIONS_TRACKING_TYPE_ROOMS } from './constants'; import CommandsPreview from './CommandsPreview'; import { getUserSelector } from '../../selectors/login'; @@ -354,58 +355,48 @@ class MessageBox extends Component { // eslint-disable-next-line react/sort-comp debouncedOnChangeText = debounce(async(text) => { const { sharing } = this.props; - const db = database.active; const isTextEmpty = text.length === 0; - // this.setShowSend(!isTextEmpty); + if (isTextEmpty) { + this.stopTrackingMention(); + return; + } this.handleTyping(!isTextEmpty); + const { start, end } = this.selection; + const cursor = Math.max(start, end); + const txt = cursor < text.length ? text.substr(0, cursor).split(' ') : text.split(' '); + const lastWord = txt[txt.length - 1]; + const result = lastWord.substring(1); - if (!sharing) { - // matches if their is text that stats with '/' and group the command and params so we can use it "/command params" - const slashCommand = text.match(/^\/([a-z0-9._-]+) (.+)/im); - if (slashCommand) { - const [, name, params] = slashCommand; + const commandMention = text.match(/^\//); // match only if message begins with / + const channelMention = lastWord.match(/^#/); + const userMention = lastWord.match(/^@/); + const emojiMention = lastWord.match(/^:/); + + if (commandMention && !sharing) { + const command = text.substr(1); + const commandParameter = text.match(/^\/([a-z0-9._-]+) (.+)/im); + if (commandParameter) { + const db = database.active; + const [, name, params] = commandParameter; const commandsCollection = db.get('slash_commands'); try { - const command = await commandsCollection.find(name); - if (command.providesPreview) { - return this.setCommandPreview(command, name, params); + const commandRecord = await commandsCollection.find(name); + if (commandRecord.providesPreview) { + return this.setCommandPreview(commandRecord, name, params); } } catch (e) { - console.log('Slash command not found'); + // do nothing } } - } - - if (!isTextEmpty) { - try { - const { start, end } = this.selection; - const cursor = Math.max(start, end); - const lastNativeText = this.text; - // matches if text either starts with '/' or have (@,#,:) then it groups whatever comes next of mention type - let regexp = /(#|@|:|^\/)([a-z0-9._-]+)$/im; - - // if sharing, track #|@|: - if (sharing) { - regexp = /(#|@|:)([a-z0-9._-]+)$/im; - } - - const result = lastNativeText.substr(0, cursor).match(regexp); - if (!result) { - if (!sharing) { - const slash = lastNativeText.match(/^\/$/); // matches only '/' in input - if (slash) { - return this.identifyMentionKeyword('', MENTIONS_TRACKING_TYPE_COMMANDS); - } - } - return this.stopTrackingMention(); - } - const [, lastChar, name] = result; - this.identifyMentionKeyword(name, lastChar); - } catch (e) { - log(e); - } + return this.identifyMentionKeyword(command, MENTIONS_TRACKING_TYPE_COMMANDS); + } else if (channelMention) { + return this.identifyMentionKeyword(result, MENTIONS_TRACKING_TYPE_ROOMS); + } else if (userMention) { + return this.identifyMentionKeyword(result, MENTIONS_TRACKING_TYPE_USERS); + } else if (emojiMention) { + return this.identifyMentionKeyword(result, MENTIONS_TRACKING_TYPE_EMOJIS); } else { - this.stopTrackingMention(); + return this.stopTrackingMention(); } }, 100) @@ -483,10 +474,10 @@ class MessageBox extends Component { getFixedMentions = (keyword) => { let result = []; if ('all'.indexOf(keyword) !== -1) { - result = [{ id: -1, username: 'all' }]; + result = [{ rid: -1, username: 'all' }]; } if ('here'.indexOf(keyword) !== -1) { - result = [{ id: -2, username: 'here' }, ...result]; + result = [{ rid: -2, username: 'here' }, ...result]; } return result; } @@ -504,17 +495,17 @@ class MessageBox extends Component { getEmojis = debounce(async(keyword) => { const db = database.active; - if (keyword) { - const customEmojisCollection = db.get('custom_emojis'); - const likeString = sanitizeLikeString(keyword); - let customEmojis = await customEmojisCollection.query( - Q.where('name', Q.like(`${ likeString }%`)) - ).fetch(); - customEmojis = customEmojis.slice(0, MENTIONS_COUNT_TO_DISPLAY); - const filteredEmojis = emojis.filter(emoji => emoji.indexOf(keyword) !== -1).slice(0, MENTIONS_COUNT_TO_DISPLAY); - const mergedEmojis = [...customEmojis, ...filteredEmojis].slice(0, MENTIONS_COUNT_TO_DISPLAY); - this.setState({ mentions: mergedEmojis || [] }); + const customEmojisCollection = db.get('custom_emojis'); + const likeString = sanitizeLikeString(keyword); + const whereClause = []; + if (likeString) { + whereClause.push(Q.where('name', Q.like(`${ likeString }%`))); } + let customEmojis = await customEmojisCollection.query(...whereClause).fetch(); + customEmojis = customEmojis.slice(0, MENTIONS_COUNT_TO_DISPLAY); + const filteredEmojis = emojis.filter(emoji => emoji.indexOf(keyword) !== -1).slice(0, MENTIONS_COUNT_TO_DISPLAY); + const mergedEmojis = [...customEmojis, ...filteredEmojis].slice(0, MENTIONS_COUNT_TO_DISPLAY); + this.setState({ mentions: mergedEmojis || [] }); }, 300) getSlashCommands = debounce(async(keyword) => { diff --git a/app/lib/rocketchat.js b/app/lib/rocketchat.js index 3cce8ed0..fc73e375 100644 --- a/app/lib/rocketchat.js +++ b/app/lib/rocketchat.js @@ -618,9 +618,6 @@ const RocketChat = { async localSearch({ text, filterUsers = true, filterRooms = true }) { const searchText = text.trim(); - if (searchText === '') { - return []; - } const db = database.active; const likeString = sanitizeLikeString(searchText); let data = await db.get('subscriptions').query( @@ -659,10 +656,6 @@ const RocketChat = { this.oldPromise('cancel'); } - if (searchText === '') { - return []; - } - const data = await this.localSearch({ text, filterUsers, filterRooms }); const usernames = data.map(sub => sub.name); diff --git a/app/utils/avatar.js b/app/utils/avatar.js index c0b4e2db..664c3ea4 100644 --- a/app/utils/avatar.js +++ b/app/utils/avatar.js @@ -3,7 +3,7 @@ import { compareServerVersion, methods } from '../lib/utils'; const formatUrl = (url, size, query) => `${ url }?format=png&size=${ size }${ query }`; export const avatarURL = ({ - type, text, size, user = {}, avatar, server, avatarETag, rid, blockUnauthenticatedAccess, serverVersion + type, text, size = 25, user = {}, avatar, server, avatarETag, rid, blockUnauthenticatedAccess, serverVersion }) => { let room; if (type === 'd') { diff --git a/app/views/RoomsListView/index.js b/app/views/RoomsListView/index.js index 4c12c4dd..19df2c3b 100644 --- a/app/views/RoomsListView/index.js +++ b/app/views/RoomsListView/index.js @@ -519,6 +519,7 @@ class RoomsListView extends React.Component { const { openSearchHeader } = this.props; this.internalSetState({ searching: true }, () => { openSearchHeader(); + this.search(''); this.setHeader(); }); }; diff --git a/e2e/helpers/app.js b/e2e/helpers/app.js index 8a07cb45..2706eb2f 100644 --- a/e2e/helpers/app.js +++ b/e2e/helpers/app.js @@ -101,6 +101,8 @@ async function searchRoom(room) { await expect(element(by.id('rooms-list-view-search-input'))).toExist(); await waitFor(element(by.id('rooms-list-view-search-input'))).toExist().withTimeout(5000); await element(by.id('rooms-list-view-search-input')).typeText(room); + await sleep(300); + await waitFor(element(by.id(`rooms-list-view-item-${ room }`))).toBeVisible().withTimeout(60000); } async function tryTapping(theElement, timeout, longtap = false){ diff --git a/e2e/tests/assorted/01-e2eencryption.spec.js b/e2e/tests/assorted/01-e2eencryption.spec.js index f3e4bcf9..dc85cb7b 100644 --- a/e2e/tests/assorted/01-e2eencryption.spec.js +++ b/e2e/tests/assorted/01-e2eencryption.spec.js @@ -22,7 +22,6 @@ const checkBanner = async() => { async function navigateToRoom(roomName) { await searchRoom(`${ roomName }`); - await waitFor(element(by.id(`rooms-list-view-item-${ roomName }`))).toExist().withTimeout(60000); await element(by.id(`rooms-list-view-item-${ roomName }`)).tap(); await waitFor(element(by.id('room-view'))).toBeVisible().withTimeout(5000); } diff --git a/e2e/tests/assorted/02-broadcast.spec.js b/e2e/tests/assorted/02-broadcast.spec.js index 97e5783c..a11f7171 100644 --- a/e2e/tests/assorted/02-broadcast.spec.js +++ b/e2e/tests/assorted/02-broadcast.spec.js @@ -61,7 +61,6 @@ describe('Broadcast room', () => { //await element(by.id('two-factor-send')).tap(); await searchRoom(`broadcast${ data.random }`); - await waitFor(element(by.id(`rooms-list-view-item-broadcast${ data.random }`))).toExist().withTimeout(60000); await element(by.id(`rooms-list-view-item-broadcast${ data.random }`)).tap(); await waitFor(element(by.id('room-view'))).toBeVisible().withTimeout(5000); await waitFor(element(by.id(`room-view-title-broadcast${ data.random }`))).toBeVisible().withTimeout(60000); diff --git a/e2e/tests/assorted/04-setting.spec.js b/e2e/tests/assorted/04-setting.spec.js index 419d377d..201ac620 100644 --- a/e2e/tests/assorted/04-setting.spec.js +++ b/e2e/tests/assorted/04-setting.spec.js @@ -83,7 +83,11 @@ describe('Settings screen', () => { await element(by.label('Clear').and(by.type('_UIAlertControllerActionView'))).tap(); await waitFor(element(by.id('rooms-list-view'))).toBeVisible().withTimeout(5000); // Database was cleared, so the room shouldn't be there anymore while it's fetched again from the server - await waitFor(element(by.id(`rooms-list-view-item-${ data.groups.private.name }`))).toNotExist().withTimeout(10000); + /** + * FIXME: rooms are fetched to quickly on docker and the test below fails + * We need to think on another way to test database being resetted + */ + // await waitFor(element(by.id(`rooms-list-view-item-${ data.groups.private.name }`))).toNotExist().withTimeout(10000); await waitFor(element(by.id(`rooms-list-view-item-${ data.groups.private.name }`))).toExist().withTimeout(10000); }) }); diff --git a/e2e/tests/assorted/05-joinpublicroom.spec.js b/e2e/tests/assorted/05-joinpublicroom.spec.js index 1cf9a7fd..8e97a893 100644 --- a/e2e/tests/assorted/05-joinpublicroom.spec.js +++ b/e2e/tests/assorted/05-joinpublicroom.spec.js @@ -9,7 +9,6 @@ const room = data.channels.detoxpublic.name; async function navigateToRoom() { await searchRoom(room); - await waitFor(element(by.id(`rooms-list-view-item-${ room }`))).toBeVisible().withTimeout(60000); await element(by.id(`rooms-list-view-item-${ room }`)).tap(); await waitFor(element(by.id('room-view'))).toBeVisible().withTimeout(5000); } diff --git a/e2e/tests/assorted/08-joinprotectedroom.spec.js b/e2e/tests/assorted/08-joinprotectedroom.spec.js index 965af00a..a07d541c 100644 --- a/e2e/tests/assorted/08-joinprotectedroom.spec.js +++ b/e2e/tests/assorted/08-joinprotectedroom.spec.js @@ -10,7 +10,6 @@ const joinCode = data.channels.detoxpublicprotected.joinCode async function navigateToRoom() { await searchRoom(room); - await waitFor(element(by.id(`rooms-list-view-item-${ room }`))).toBeVisible().withTimeout(60000); await element(by.id(`rooms-list-view-item-${ room }`)).tap(); await waitFor(element(by.id('room-view'))).toBeVisible().withTimeout(5000); } diff --git a/e2e/tests/onboarding/06-roomslist.spec.js b/e2e/tests/onboarding/06-roomslist.spec.js index e213b7f2..ea6dc06c 100644 --- a/e2e/tests/onboarding/06-roomslist.spec.js +++ b/e2e/tests/onboarding/06-roomslist.spec.js @@ -36,7 +36,6 @@ describe('Rooms list screen', () => { describe('Usage', () => { it('should search room and navigate', async() => { await searchRoom('rocket.cat'); - await waitFor(element(by.id('rooms-list-view-item-rocket.cat'))).toBeVisible().withTimeout(60000); await element(by.id('rooms-list-view-item-rocket.cat')).tap(); await waitFor(element(by.id('room-view'))).toBeVisible().withTimeout(10000); await waitFor(element(by.id('room-view-title-rocket.cat'))).toBeVisible().withTimeout(60000); diff --git a/e2e/tests/room/02-room.spec.js b/e2e/tests/room/02-room.spec.js index e5fb4292..da83c1eb 100644 --- a/e2e/tests/room/02-room.spec.js +++ b/e2e/tests/room/02-room.spec.js @@ -6,7 +6,6 @@ const { navigateToLogin, login, mockMessage, tapBack, sleep, searchRoom, starMes async function navigateToRoom(roomName) { await searchRoom(`${ roomName }`); - await waitFor(element(by.id(`rooms-list-view-item-${ roomName }`))).toExist().withTimeout(60000); await element(by.id(`rooms-list-view-item-${ roomName }`)).tap(); await waitFor(element(by.id('room-view'))).toBeVisible().withTimeout(5000); } @@ -103,6 +102,14 @@ describe('Room screen', () => { await element(by.id('messagebox-input')).clearText(); }); + it('should not show emoji autocomplete on semicolon in middle of a string', async() => { + await element(by.id('messagebox-input')).tap(); + // await element(by.id('messagebox-input')).replaceText(':'); + await element(by.id('messagebox-input')).typeText('name:is'); + await waitFor(element(by.id('messagebox-container'))).toNotExist().withTimeout(20000); + await element(by.id('messagebox-input')).clearText(); + }); + it('should show and tap on user autocomplete and send mention', async() => { const username = data.users.regular.username await element(by.id('messagebox-input')).tap(); @@ -117,6 +124,14 @@ describe('Room screen', () => { // await waitFor(element(by.label(`@${ data.user } ${ data.random }mention`)).atIndex(0)).toExist().withTimeout(60000); }); + it('should not show user autocomplete on @ in the middle of a string', async() => { + const username = data.users.regular.username + await element(by.id('messagebox-input')).tap(); + await element(by.id('messagebox-input')).typeText(`email@gmail`); + await waitFor(element(by.id('messagebox-container'))).toNotExist().withTimeout(4000); + await element(by.id('messagebox-input')).clearText(); + }); + it('should show and tap on room autocomplete', async() => { await element(by.id('messagebox-input')).tap(); await element(by.id('messagebox-input')).typeText('#general'); @@ -127,6 +142,12 @@ describe('Room screen', () => { await element(by.id('messagebox-input')).clearText(); }); + it('should not show room autocomplete on # in middle of a string', async() => { + await element(by.id('messagebox-input')).tap(); + await element(by.id('messagebox-input')).typeText('te#gen'); + await waitFor(element(by.id('messagebox-container'))).toNotExist().withTimeout(4000); + await element(by.id('messagebox-input')).clearText(); + }); it('should draft message', async () => { await element(by.id('messagebox-input')).atIndex(0).tap(); await element(by.id('messagebox-input')).atIndex(0).typeText(`${ data.random }draft`); diff --git a/e2e/tests/room/03-roomactions.spec.js b/e2e/tests/room/03-roomactions.spec.js index 494a8e39..c7945f65 100644 --- a/e2e/tests/room/03-roomactions.spec.js +++ b/e2e/tests/room/03-roomactions.spec.js @@ -13,7 +13,6 @@ async function navigateToRoomActions(type) { room = data.groups.private.name; } await searchRoom(room); - await waitFor(element(by.id(`rooms-list-view-item-${ room }`))).toExist().withTimeout(60000); await element(by.id(`rooms-list-view-item-${ room }`)).tap(); await waitFor(element(by.id('room-view'))).toExist().withTimeout(2000); await element(by.id('room-header')).tap(); diff --git a/e2e/tests/room/04-discussion.spec.js b/e2e/tests/room/04-discussion.spec.js index d1686840..0a0bc8e7 100644 --- a/e2e/tests/room/04-discussion.spec.js +++ b/e2e/tests/room/04-discussion.spec.js @@ -8,7 +8,6 @@ const channel = data.groups.private.name; const navigateToRoom = async() => { await searchRoom(channel); - await waitFor(element(by.id(`rooms-list-view-item-${ channel }`))).toExist().withTimeout(60000); await element(by.id(`rooms-list-view-item-${ channel }`)).tap(); await waitFor(element(by.id('room-view'))).toBeVisible().withTimeout(5000); } diff --git a/e2e/tests/room/05-threads.spec.js b/e2e/tests/room/05-threads.spec.js index 1b7c64b8..e7dec4c7 100644 --- a/e2e/tests/room/05-threads.spec.js +++ b/e2e/tests/room/05-threads.spec.js @@ -9,7 +9,6 @@ async function navigateToRoom(roomName) { await navigateToLogin(); await login(data.users.regular.username, data.users.regular.password); await searchRoom(`${ roomName }`); - await waitFor(element(by.id(`rooms-list-view-item-${ roomName }`))).toExist().withTimeout(60000); await element(by.id(`rooms-list-view-item-${ roomName }`)).tap(); await waitFor(element(by.id('room-view'))).toBeVisible().withTimeout(5000); } diff --git a/e2e/tests/room/07-markasunread.spec.js b/e2e/tests/room/07-markasunread.spec.js index 0247e039..9f425e8c 100644 --- a/e2e/tests/room/07-markasunread.spec.js +++ b/e2e/tests/room/07-markasunread.spec.js @@ -6,7 +6,6 @@ const { navigateToLogin, login, mockMessage, tapBack, searchRoom, logout } = req async function navigateToRoom(user) { await searchRoom(`${ user }`); - await waitFor(element(by.id(`rooms-list-view-item-${ user }`))).toExist().withTimeout(60000); await element(by.id(`rooms-list-view-item-${ user }`)).tap(); await waitFor(element(by.id('room-view'))).toBeVisible().withTimeout(5000); } diff --git a/e2e/tests/room/08-roominfo.spec.js b/e2e/tests/room/08-roominfo.spec.js index b172935c..3cd339ba 100644 --- a/e2e/tests/room/08-roominfo.spec.js +++ b/e2e/tests/room/08-roominfo.spec.js @@ -14,7 +14,6 @@ async function navigateToRoomInfo(type) { room = privateRoomName; } await searchRoom(room); - await waitFor(element(by.id(`rooms-list-view-item-${ room }`))).toExist().withTimeout(60000); await element(by.id(`rooms-list-view-item-${ room }`)).tap(); await waitFor(element(by.id('room-view'))).toExist().withTimeout(2000); await element(by.id('room-header')).tap();