Skip to content

Commit

Permalink
Merge pull request #3214 from omnivore-app/fix/search
Browse files Browse the repository at this point in the history
fix some bugs with new search feature
  • Loading branch information
sywhb authored Dec 7, 2023
2 parents 7277b27 + 5f09165 commit fe85345
Show file tree
Hide file tree
Showing 4 changed files with 97 additions and 21 deletions.
19 changes: 16 additions & 3 deletions packages/api/src/resolvers/article/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -822,8 +822,12 @@ export const bulkActionResolver = authorized<
},
})

// the query size is limited to 255 characters
if (!query || query.length > 255) {
// the query size is limited to 4000 characters to allow for 100 items
if (!query || query.length > 4000) {
log.error('bulkActionResolver error', {
error: 'QueryTooLong',
query,
})
return { errorCodes: [BulkActionErrorCode.BadRequest] }
}

Expand All @@ -837,7 +841,16 @@ export const bulkActionResolver = authorized<
labels = await findLabelsByIds(labelIds, uid)
}

await updateLibraryItems(action, query, uid, labels, args)
await updateLibraryItems(
action,
{
query,
useFolders: query.includes('use:folders'),
},
uid,
labels,
args
)

return { success: true }
} catch (error) {
Expand Down
42 changes: 24 additions & 18 deletions packages/api/src/services/library_item.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,11 +80,11 @@ export interface SearchResultItem {
}

export enum SortBy {
SAVED = 'saved_at',
UPDATED = 'updated_at',
PUBLISHED = 'published_at',
READ = 'read_at',
WORDS_COUNT = 'word_count',
SAVED = 'saved',
UPDATED = 'updated',
PUBLISHED = 'published',
READ = 'read',
WORDS_COUNT = 'wordsCount',
}

export enum SortOrder {
Expand All @@ -102,6 +102,10 @@ interface Select {
alias?: string
}

const paramtersToObject = (parameters: ObjectLiteral[]) => {
return parameters.reduce((a, b) => ({ ...a, ...b }), {})
}

export const sortParamsToSort = (
sortParams: InputMaybe<SortParams> | undefined
) => {
Expand Down Expand Up @@ -181,15 +185,14 @@ export const buildQuery = (
return null
}

const param = 'implicit_field'
const alias = 'rank'
const param = `implicit_field_${parameters.length}`
const alias = `rank_${parameters.length}`
selects.push({
column: `ts_rank_cd(library_item.search_tsv, websearch_to_tsquery('english', :${param}))`,
alias,
})

// always sort by rank first
orders.unshift({ by: alias, order: SortOrder.DESCENDING })
orders.push({ by: alias, order: SortOrder.DESCENDING })

return escapeQueryWithParameters(
`websearch_to_tsquery('english', :${param}) @@ library_item.search_tsv`,
Expand Down Expand Up @@ -532,6 +535,7 @@ export const buildQuery = (
}
case 'use':
case 'mode':
case 'event':
// mode is ignored and used only by the frontend
return null
case 'readPosition':
Expand Down Expand Up @@ -688,7 +692,7 @@ export const searchLibraryItems = async (
// add where clause from query
queryBuilder
.andWhere(query)
.setParameters(parameters.reduce((a, b) => ({ ...a, ...b }), {}))
.setParameters(paramtersToObject(parameters))
}

const count = await queryBuilder.getCount()
Expand Down Expand Up @@ -1011,7 +1015,7 @@ export const countByCreatedAt = async (

export const updateLibraryItems = async (
action: BulkActionType,
query: string,
searchArgs: SearchArgs,
userId: string,
labels?: Label[],
args?: unknown
Expand Down Expand Up @@ -1065,19 +1069,21 @@ export const updateLibraryItems = async (
throw new Error('Invalid bulk action')
}

const searchQuery = parseSearchQuery(query)
if (!searchArgs.query) {
throw new Error('Search query is required')
}

const searchQuery = parseSearchQuery(searchArgs.query)
const parameters: ObjectLiteral[] = []
const query = buildQuery(searchQuery, parameters)

await authTrx(async (tx) => {
const queryBuilder = tx
.createQueryBuilder(LibraryItem, 'library_item')
.where('library_item.user_id = :userId', { userId })

const parameters: ObjectLiteral[] = []
const whereClause = buildQuery(searchQuery, parameters)
if (whereClause) {
queryBuilder
.andWhere(whereClause)
.setParameters(parameters.reduce((a, b) => ({ ...a, ...b }), {}))
if (query) {
queryBuilder.andWhere(query).setParameters(paramtersToObject(parameters))
}

if (addLabels) {
Expand Down
2 changes: 2 additions & 0 deletions packages/api/src/utils/search.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ export const parseSearchQuery = (query: string): LiqeQuery => {
.replace('in:library', 'no:subscription') // compatibility with old search
// wrap the value behind colon in quotes if it's not already
.replace(/(\w+):("([^"]+)"|([^")\s]+))/g, '$1:"$3$4"')
// remove any quotes that are in the array value for example: label:"test","test2" -> label:"test,test2"
.replace(/","/g, ',')

return parse(searchQuery)
}
55 changes: 55 additions & 0 deletions packages/api/test/resolvers/article.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1702,6 +1702,61 @@ describe('Article API', () => {
expect(res.body.data.search.edges[0].node.id).to.eq(items[0].id)
})
})

context('when label:test1,test2 is in the query', () => {
let items: LibraryItem[] = []
let label1: Label
let label2: Label

before(async () => {
keyword = 'label:test1,test2'
// Create some test items
label1 = await createLabel('test1', '', user.id)
label2 = await createLabel('test2', '', user.id)
items = await createLibraryItems(
[
{
user,
title: 'test title 1',
readableContent: '<p>test 1</p>',
slug: 'test slug 1',
originalUrl: `${url}/test1`,
},
{
user,
title: 'test title 2',
readableContent: '<p>test 2</p>',
slug: 'test slug 2',
originalUrl: `${url}/test2`,
},
{
user,
title: 'test title 3',
readableContent: '<p>test 3</p>',
slug: 'test slug 3',
originalUrl: `${url}/test3`,
},
],
user.id
)
await saveLabelsInLibraryItem([label1], items[0].id, user.id)
await saveLabelsInLibraryItem([label2], items[1].id, user.id)
})

after(async () => {
await deleteLabels({ id: label1.id }, user.id)
await deleteLabels({ id: label2.id }, user.id)
await deleteLibraryItems(items, user.id)
})

it('returns items with label test1 or test2', async () => {
const res = await graphqlRequest(query, authToken).expect(200)

expect(res.body.data.search.pageInfo.totalCount).to.eq(2)
expect(res.body.data.search.edges[0].node.id).to.eq(items[1].id)
expect(res.body.data.search.edges[1].node.id).to.eq(items[0].id)
})
})
})

describe('TypeaheadSearch API', () => {
Expand Down

1 comment on commit fe85345

@vercel
Copy link

@vercel vercel bot commented on fe85345 Dec 7, 2023

Choose a reason for hiding this comment

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

Please sign in to comment.