From 61fe9dbb1e201286c1616311c94474b29739a7b7 Mon Sep 17 00:00:00 2001 From: Reinaldo Neto <47038980+reinaldonetof@users.noreply.github.com> Date: Mon, 29 May 2023 12:03:24 -0300 Subject: [PATCH] feat: case insensitive for non-ASCII text on main search (#3309) * Added slug as dependecy and created a slugified String * add the slug and slugifyLikeString * using unsafeSql instead of the slug * need to fix the like on the watermelon side and need the slug anyway * watermelondb patch to change the like to use the upper or toUpperCase * Updated config.yml * Updated config.yml * implemented the sanitized fname and fix the discussion icon at search * add the search for non-latin alphabets * fix the searchRoom function * change the library of slug and added the unit tests * optional sanitizedFname * add some comment * remove @types/slug * remove watermelondb patch package * latin test, tweak at comment and tweak e2e test * minor tweak e2e * change typeText to replaceText at searchRoom * regexp to test the characters * add typeText on searchRoom * e2e search room replace and type * to fix the replace text for iOS and type non-ASCII on Android * minor tweak * minor tweak * enable artifact * disable artifacts * increase sleep time and change from toExist to toBeVisible * fix android flaky test --- app/definitions/ISubscription.ts | 1 + app/lib/database/model/Subscription.js | 2 + app/lib/database/model/migrations.js | 9 +++ app/lib/database/schema/app.js | 3 +- app/lib/database/utils.test.ts | 31 ++++++++++ app/lib/database/utils.ts | 12 ++++ .../helpers/mergeSubscriptionsRooms.ts | 3 +- app/lib/methods/search.ts | 17 +++++- e2e/helpers/app.ts | 12 ++-- e2e/tests/room/01-createroom.spec.ts | 60 ++++++++++++++++++- e2e/tests/room/02-room.spec.ts | 19 ++++-- package.json | 1 + yarn.lock | 7 +++ 13 files changed, 163 insertions(+), 14 deletions(-) diff --git a/app/definitions/ISubscription.ts b/app/definitions/ISubscription.ts index 10dce8f19..f8f7749f9 100644 --- a/app/definitions/ISubscription.ts +++ b/app/definitions/ISubscription.ts @@ -46,6 +46,7 @@ export interface ISubscription { ls: Date; name: string; fname?: string; + sanitizedFname?: string; rid: string; // the same as id open: boolean; alert: boolean; diff --git a/app/lib/database/model/Subscription.js b/app/lib/database/model/Subscription.js index fc873bec6..a79d5a59d 100644 --- a/app/lib/database/model/Subscription.js +++ b/app/lib/database/model/Subscription.js @@ -29,6 +29,8 @@ export default class Subscription extends Model { @field('fname') fname; + @field('sanitized_fname') sanitizedFname; + @field('rid') rid; @field('open') open; diff --git a/app/lib/database/model/migrations.js b/app/lib/database/model/migrations.js index 8f23b7f2c..71fc4a759 100644 --- a/app/lib/database/model/migrations.js +++ b/app/lib/database/model/migrations.js @@ -266,6 +266,15 @@ export default schemaMigrations({ columns: [{ name: 'users_count', type: 'string', isOptional: true }] }) ] + }, + { + toVersion: 22, + steps: [ + addColumns({ + table: 'subscriptions', + columns: [{ name: 'sanitized_fname', type: 'string', isOptional: true }] + }) + ] } ] }); diff --git a/app/lib/database/schema/app.js b/app/lib/database/schema/app.js index 90ef03aa2..5e408ade0 100644 --- a/app/lib/database/schema/app.js +++ b/app/lib/database/schema/app.js @@ -1,7 +1,7 @@ import { appSchema, tableSchema } from '@nozbe/watermelondb'; export default appSchema({ - version: 21, + version: 22, tables: [ tableSchema({ name: 'subscriptions', @@ -13,6 +13,7 @@ export default appSchema({ { name: 'ls', type: 'number' }, { name: 'name', type: 'string', isIndexed: true }, { name: 'fname', type: 'string' }, + { name: 'sanitized_fname', type: 'string', isOptional: true }, { name: 'rid', type: 'string', isIndexed: true }, { name: 'open', type: 'boolean' }, { name: 'alert', type: 'boolean' }, diff --git a/app/lib/database/utils.test.ts b/app/lib/database/utils.test.ts index 54ecbab6a..3c914112c 100644 --- a/app/lib/database/utils.test.ts +++ b/app/lib/database/utils.test.ts @@ -39,3 +39,34 @@ describe('sanitizer', () => { expect(utils.sanitizer(content)).toBe(content); }); }); + +describe('slugifyLikeString', () => { + test('render empty', () => { + expect(utils.slugifyLikeString('')).toBe(''); + expect(utils.slugifyLikeString(undefined)).toBe(''); + }); + test('slugify the latin alphabet', () => { + expect(utils.slugifyLikeString('test123')).toBe('test123'); + expect(utils.slugifyLikeString('TEST123')).toBe('test123'); + }); + test('slugify the russian alphabet', () => { + const textToSlugify = 'ПРОВЕРКА'; + const textSlugified = 'proverka'; + expect(utils.slugifyLikeString(textToSlugify)).toBe(textSlugified); + }); + test('slugify the arabic alphabet', () => { + const textToSlugify = 'اختبار123'; + const textSlugified = 'khtbr123'; + expect(utils.slugifyLikeString(textToSlugify)).toBe(textSlugified); + }); + test('slugify the chinese trad alphabet', () => { + const textToSlugify = '測試123'; + const textSlugified = 'ce-shi-123'; + expect(utils.slugifyLikeString(textToSlugify)).toBe(textSlugified); + }); + test('slugify the japanese alphabet', () => { + const textToSlugify = 'テスト123'; + const textSlugified = 'tesuto123'; + expect(utils.slugifyLikeString(textToSlugify)).toBe(textSlugified); + }); +}); diff --git a/app/lib/database/utils.ts b/app/lib/database/utils.ts index a5b9d8e8d..8bc2df194 100644 --- a/app/lib/database/utils.ts +++ b/app/lib/database/utils.ts @@ -1,7 +1,19 @@ import XRegExp from 'xregexp'; +import { slugify } from 'transliteration'; // Matches letters from any alphabet and numbers const likeStringRegex = XRegExp('[^\\p{L}\\p{Nd}]', 'g'); export const sanitizeLikeString = (str?: string): string | undefined => str?.replace(likeStringRegex, '_'); +// Will change any non-latin character to return a lower latin character string +// Example: +// slugifyLikeString('測試123') => 'ce-shi-123' +// slugifyLikeString('テスト123') => 'tesuto123' +export const slugifyLikeString = (str?: string) => { + if (!str) return ''; + str?.replace(likeStringRegex, '_'); + const slugified = slugify(str); + return slugified; +}; + export const sanitizer = (r: object): object => r; diff --git a/app/lib/methods/helpers/mergeSubscriptionsRooms.ts b/app/lib/methods/helpers/mergeSubscriptionsRooms.ts index 90b5445b2..fec41dfd3 100644 --- a/app/lib/methods/helpers/mergeSubscriptionsRooms.ts +++ b/app/lib/methods/helpers/mergeSubscriptionsRooms.ts @@ -1,5 +1,6 @@ import EJSON from 'ejson'; +import { slugifyLikeString } from '../../database/utils'; import { Encryption } from '../../encryption'; import { store as reduxStore } from '../../store/auxStore'; import findSubscriptionsRooms from './findSubscriptionsRooms'; @@ -101,7 +102,7 @@ export const merge = ( mergedSubscription.blocker = !!mergedSubscription.blocker; mergedSubscription.blocked = !!mergedSubscription.blocked; mergedSubscription.hideMentionStatus = !!mergedSubscription.hideMentionStatus; - + mergedSubscription.sanitizedFname = slugifyLikeString(mergedSubscription.fname || mergedSubscription.name); return mergedSubscription; }; diff --git a/app/lib/methods/search.ts b/app/lib/methods/search.ts index 8bb2dd3fa..d4bc8c2f6 100644 --- a/app/lib/methods/search.ts +++ b/app/lib/methods/search.ts @@ -1,6 +1,6 @@ import { Q } from '@nozbe/watermelondb'; -import { sanitizeLikeString } from '../database/utils'; +import { sanitizeLikeString, slugifyLikeString } from '../database/utils'; import database from '../database/index'; import { store as reduxStore } from '../store/auxStore'; import { spotlight } from '../services/restApi'; @@ -15,10 +15,20 @@ export const localSearchSubscription = async ({ text = '', filterUsers = true, f const searchText = text.trim(); const db = database.active; const likeString = sanitizeLikeString(searchText); + const slugifiedString = slugifyLikeString(searchText); let subscriptions = await db .get('subscriptions') .query( - Q.or(Q.where('name', Q.like(`%${likeString}%`)), Q.where('fname', Q.like(`%${likeString}%`))), + Q.or( + // `sanitized_fname` is an optional column, so it's going to start null and it's going to get filled over time + Q.where('sanitized_fname', Q.like(`%${slugifiedString}%`)), + // TODO: Remove the conditionals below at some point. It is merged at 4.39 + // the param 'name' is slugified by the server when the slugify setting is enable, just for channels and teams + Q.where('name', Q.like(`%${slugifiedString}%`)), + // Still need the below conditionals because at the first moment the the sanitized_fname won't be filled + Q.where('name', Q.like(`%${likeString}%`)), + Q.where('fname', Q.like(`%${likeString}%`)) + ), Q.sortBy('room_updated_at', Q.desc) ) .fetch(); @@ -39,7 +49,8 @@ export const localSearchSubscription = async ({ text = '', filterUsers = true, f encrypted: item.encrypted, lastMessage: item.lastMessage, status: item.status, - teamMain: item.teamMain + teamMain: item.teamMain, + prid: item.prid })) as ISearchLocal[]; return search; diff --git a/e2e/helpers/app.ts b/e2e/helpers/app.ts index c9c82a9c3..4748d7b64 100644 --- a/e2e/helpers/app.ts +++ b/e2e/helpers/app.ts @@ -118,14 +118,18 @@ async function tapBack() { await sleep(300); // Wait for animation to finish } -async function searchRoom(room: string) { +async function searchRoom(room: string, roomTestID?: string) { await waitFor(element(by.id('rooms-list-view'))) .toBeVisible() .withTimeout(30000); await tapAndWaitFor(element(by.id('rooms-list-view-search')), element(by.id('rooms-list-view-search-input')), 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}`))) + // to fix the replace text for iOS and type non-ASCII on Android + const roomFirstSlice = room.slice(0, room.length - 2); + await element(by.id('rooms-list-view-search-input')).replaceText(roomFirstSlice); + await sleep(500); + await element(by.id('rooms-list-view-search-input')).replaceText(room); + await sleep(500); + await waitFor(element(by.id(roomTestID || `rooms-list-view-item-${room}`))) .toBeVisible() .withTimeout(60000); } diff --git a/e2e/tests/room/01-createroom.spec.ts b/e2e/tests/room/01-createroom.spec.ts index eb61aed6d..7a7ac99d0 100644 --- a/e2e/tests/room/01-createroom.spec.ts +++ b/e2e/tests/room/01-createroom.spec.ts @@ -1,6 +1,6 @@ import { device, waitFor, element, by, expect } from 'detox'; -import { tapBack, navigateToLogin, login, platformTypes, TTextMatcher, tapAndWaitFor } from '../../helpers/app'; +import { tapBack, navigateToLogin, login, platformTypes, TTextMatcher, tapAndWaitFor, searchRoom } from '../../helpers/app'; import { createRandomUser } from '../../helpers/data_setup'; import random from '../../helpers/random'; @@ -238,6 +238,9 @@ describe('Create room screen', () => { await waitFor(element(by.id('create-channel-view'))) .toExist() .withTimeout(10000); + await waitFor(element(by.id('create-channel-name'))) + .toBeVisible() + .withTimeout(2000); await element(by.id('create-channel-name')).replaceText(room); await element(by.id('create-channel-name')).tapReturnKey(); await waitFor(element(by.id('create-channel-submit'))) @@ -261,6 +264,61 @@ describe('Create room screen', () => { .withTimeout(60000); await expect(element(by.id(`rooms-list-view-item-${room}`))).toExist(); }); + + it('should create a room with non-latin alphabet and do a case insensitive search for it', async () => { + const randomValue = random(); + const roomName = `ПРОВЕРКА${randomValue}`; + const roomNameLower = roomName.toLowerCase(); + + await waitFor(element(by.id('rooms-list-view'))) + .toExist() + .withTimeout(10000); + await element(by.id('rooms-list-view-create-channel')).tap(); + await waitFor(element(by.id('new-message-view'))) + .toBeVisible() + .withTimeout(5000); + await waitFor(element(by.id('new-message-view-create-channel'))) + .toBeVisible() + .withTimeout(2000); + await element(by.id('new-message-view-create-channel')).tap(); + await waitFor(element(by.id('select-users-view'))) + .toExist() + .withTimeout(5000); + await element(by.id('selected-users-view-submit')).tap(); + await waitFor(element(by.id('create-channel-view'))) + .toExist() + .withTimeout(10000); + await waitFor(element(by.id('create-channel-name'))) + .toBeVisible() + .withTimeout(2000); + await element(by.id('create-channel-name')).replaceText(roomName); + await element(by.id('create-channel-name')).tapReturnKey(); + await waitFor(element(by.id('create-channel-submit'))) + .toExist() + .withTimeout(2000); + await element(by.id('create-channel-submit')).tap(); + await waitFor(element(by.id('room-view'))) + .toExist() + .withTimeout(60000); + await expect(element(by.id('room-view'))).toExist(); + await waitFor(element(by.id(`room-view-title-${roomName}`))) + .toExist() + .withTimeout(60000); + await expect(element(by.id(`room-view-title-${roomName}`))).toExist(); + await tapBack(); + await waitFor(element(by.id('rooms-list-view'))) + .toExist() + .withTimeout(2000); + await waitFor(element(by.id(`rooms-list-view-item-${roomName}`))) + .toExist() + .withTimeout(60000); + await expect(element(by.id(`rooms-list-view-item-${roomName}`))).toExist(); + await searchRoom(roomNameLower, `rooms-list-view-item-${roomName}`); + 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/02-room.spec.ts b/e2e/tests/room/02-room.spec.ts index 8b19a4916..60d4265c2 100644 --- a/e2e/tests/room/02-room.spec.ts +++ b/e2e/tests/room/02-room.spec.ts @@ -490,11 +490,11 @@ describe('Room screen', () => { const { name: replyRoom } = await createRandomRoom(replyUser, 'c'); const originalMessage = 'Message to reply in DM'; const replyMessage = 'replied in dm'; + await sendMessage(replyUser, replyRoom, originalMessage); await waitFor(element(by.id('rooms-list-view'))) .toBeVisible() .withTimeout(2000); await navigateToRoom(replyRoom); - await sendMessage(replyUser, replyRoom, originalMessage); await waitFor(element(by[textMatcher](originalMessage)).atIndex(0)) .toBeVisible() .withTimeout(10000); @@ -502,16 +502,27 @@ describe('Room screen', () => { await waitFor(element(by.id('room-view-join-button'))) .not.toBeVisible() .withTimeout(10000); - await element(by[textMatcher](originalMessage)).atIndex(0).tap(); await element(by[textMatcher](originalMessage)).atIndex(0).longPress(); - await sleep(300); // wait for animation + await sleep(600); // wait for animation await waitFor(element(by.id('action-sheet'))) .toExist() .withTimeout(2000); - await waitFor(element(by[textMatcher]('Reply in Direct Message')).atIndex(0)) + await sleep(600); // wait for animation + // Fix android flaky test. Close the action sheet, then re-open again + await element(by.id('action-sheet-handle')).swipe('down', 'fast', 0.5); + await sleep(1000); // wait for animation + await element(by[textMatcher](originalMessage)).atIndex(0).longPress(); + await sleep(600); // wait for animation + await waitFor(element(by.id('action-sheet'))) .toExist() + .withTimeout(2000); + await sleep(600); // wait for animation + await waitFor(element(by[textMatcher]('Reply in Direct Message')).atIndex(0)) + .toBeVisible() .withTimeout(6000); + await sleep(600); // wait for animation await element(by[textMatcher]('Reply in Direct Message')).atIndex(0).tap(); + await sleep(600); // wait for animation await waitFor(element(by.id(`room-view-title-${replyUser.username}`))) .toExist() .withTimeout(6000); diff --git a/package.json b/package.json index 98b98b777..4ebbaa063 100644 --- a/package.json +++ b/package.json @@ -143,6 +143,7 @@ "rn-fetch-blob": "^0.12.0", "rn-root-view": "RocketChat/rn-root-view", "semver": "^7.3.8", + "transliteration": "^2.3.5", "ua-parser-js": "^1.0.32", "uri-js": "^4.4.1", "url-parse": "1.5.10", diff --git a/yarn.lock b/yarn.lock index a61f6de87..bf5eb782b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -19640,6 +19640,13 @@ transformation-matrix@^2.8.0: resolved "https://registry.yarnpkg.com/transformation-matrix/-/transformation-matrix-2.12.0.tgz#cb826a23aa5d675d18940215ccb7613b8587830f" integrity sha512-BbzXM7el7rNwIr1s87m8tcffH5qgY+HYROLn3BStRU9Y6vYTL37YZKadfNPEvGbP813iA1h8qflo4pa2TomkyQ== +transliteration@^2.3.5: + version "2.3.5" + resolved "https://registry.yarnpkg.com/transliteration/-/transliteration-2.3.5.tgz#8f92309575f69e4a8a525dab4ff705ebcf961c45" + integrity sha512-HAGI4Lq4Q9dZ3Utu2phaWgtm3vB6PkLUFqWAScg/UW+1eZ/Tg6Exo4oC0/3VUol/w4BlefLhUUSVBr/9/ZGQOw== + dependencies: + yargs "^17.5.1" + traverse@~0.6.6: version "0.6.6" resolved "https://registry.yarnpkg.com/traverse/-/traverse-0.6.6.tgz#cbdf560fd7b9af632502fed40f918c157ea97137"