feat: improve user assignments via quick add magic #3348

Merged
konrad merged 18 commits from feature/better-user-search-assignees into main 2023-06-05 15:03:16 +00:00
8 changed files with 66 additions and 32 deletions

View File

@ -48,7 +48,6 @@ const displayName = computed(() => getDisplayName(props.user))
<style lang="scss" scoped>
.user {
margin: .5rem;
display: flex;
justify-items: center;

View File

@ -20,7 +20,8 @@
:user="n.notification.doer"
:show-username="false"
:avatar-size="16"
v-if="n.notification.doer"/>
v-if="n.notification.doer"
/>
<div class="detail">
<div>
<span class="has-text-weight-bold mr-1" v-if="n.notification.doer">

View File

@ -12,12 +12,15 @@
>
<template #tag="{item: user}">
<span class="assignee">
<user :avatar-size="32" :show-username="false" :user="user"/>
<user :avatar-size="32" :show-username="false" :user="user" class="m-2"/>
<BaseButton @click="removeAssignee(user)" class="remove-assignee" v-if="!disabled">
<icon icon="times"/>
</BaseButton>
</span>
</template>
<template #searchResult="{option: user}">
<user :avatar-size="24" :show-username="true" :user="user"/>
</template>
</Multiselect>
</template>
@ -104,11 +107,6 @@ async function removeAssignee(user: IUser) {
}
async function findUser(query: string) {
if (query === '') {
foundUsers.value = []
return
}
const response = await projectUserService.getAll({projectId: props.projectId}, {s: query}) as IUser[]
// Filter the results to not include users who are already assigned

View File

@ -56,6 +56,7 @@
:key="task.id + 'assignee' + a.id + i"
:show-username="false"
:user="a"
class="m-2"
/>
<!-- FIXME: use popup -->

View File

@ -76,8 +76,8 @@
"savedSuccess": "The settings were successfully updated.",
"emailReminders": "Send me reminders for tasks via Email",
"overdueReminders": "Send me a summary of my undone overdue tasks every day",
"discoverableByName": "Let other users find me when they search for my name",
"discoverableByEmail": "Let other users find me when they search for my full email",
"discoverableByName": "Allow other users to add me as a member to teams or projects when they search for my name",
"discoverableByEmail": "Allow other users to add me as a member to teams or projects when they search for my full email",
"playSoundWhenDone": "Play a sound when marking tasks as done",
"weekStart": "Week starts on",
"weekStartSunday": "Sunday",

View File

@ -691,6 +691,14 @@ describe('Parse Task Text', () => {
expect(result.assignees).toHaveLength(1)
expect(result.assignees[0]).toBe('today')
})
it('should recognize an email address', () => {
const text = 'Lorem Ipsum @email@example.com'
const result = parseTaskText(text)
expect(result.text).toBe('Lorem Ipsum @email@example.com')
expect(result.assignees).toHaveLength(1)
expect(result.assignees[0]).toBe('email@example.com')
})
})
describe('Recurring Dates', () => {

View File

@ -109,7 +109,9 @@ const getItemsFromPrefix = (text: string, prefix: string): string[] => {
return
}
p = p.replace(prefix, '')
if (p.startsWith(prefix)) {
p = p.substring(1)
}
let itemText
if (p.charAt(0) === '\'') {
@ -120,8 +122,8 @@ const getItemsFromPrefix = (text: string, prefix: string): string[] => {
// Only until the next space
itemText = p.split(' ')[0]
}
if(itemText !== '') {
if (itemText !== '') {
items.push(itemText)
}
})
@ -278,13 +280,16 @@ const getRepeats = (text: string): repeatParsedResult => {
export const cleanupItemText = (text: string, items: string[], prefix: string): string => {
items.forEach(l => {
if (l === '') {
return
}
text = text
.replace(`${prefix}'${l}' `, '')
.replace(`${prefix}'${l}'`, '')
.replace(`${prefix}"${l}" `, '')
.replace(`${prefix}"${l}"`, '')
.replace(`${prefix}${l} `, '')
.replace(`${prefix}${l}`, '')
.replace(new RegExp(`\\${prefix}'${l}' `, 'ig'), '')
.replace(new RegExp(`\\${prefix}'${l}'`, 'ig'), '')
.replace(new RegExp(`\\${prefix}"${l}" `, 'ig'), '')
.replace(new RegExp(`\\${prefix}"${l}"`, 'ig'), '')
.replace(new RegExp(`\\${prefix}${l} `, 'ig'), '')
.replace(new RegExp(`\\${prefix}${l}`, 'ig'), '')
})
return text
}

View File

@ -5,7 +5,6 @@ import router from '@/router'
import TaskService from '@/services/task'
import TaskAssigneeService from '@/services/taskAssignee'
import LabelTaskService from '@/services/labelTask'
import UserService from '@/services/user'
import {playPop} from '@/helpers/playPop'
import {getQuickAddMagicMode} from '@/helpers/quickAddMagicMode'
@ -29,12 +28,21 @@ import {useProjectStore} from '@/stores/projects'
import {useAttachmentStore} from '@/stores/attachments'
import {useKanbanStore} from '@/stores/kanban'
import {useBaseStore} from '@/stores/base'
import ProjectUserService from '@/services/projectUsers'
interface MatchedAssignee extends IUser {
match: string,
}
// IDEA: maybe use a small fuzzy search here to prevent errors
dpschen marked this conversation as resolved
Review
For a simple implementation copy over: https://github.com/hiddentao/fast-levenshtein/blob/3.0.0/levenshtein.js
Review

Why copy it instead of pulling it in as a dependency?

We don't do any fancy matching in the api (other than lowercase contains), which means the frontend can't find things with fuzzy matching which the api did not return in the first place. I feel like we should fuzzy matching in the api first and then add it in the frontend as well.

Why copy it instead of pulling it in as a dependency? We don't do any fancy matching in the api (other than lowercase contains), which means the frontend can't find things with fuzzy matching which the api did not return in the first place. I feel like we should fuzzy matching in the api first and then add it in the frontend as well.
Review

The module is a slim wrapper around fastest-levenshtein with some sensible defaults and using Intl.Collator. The wrapper is a bit dated. So I would only copy the function as a helper and import fastest-levenshtein directly. Fuzzy matching in the api makes sense.

Doing it in the frontend as well for the locally available data still makes sense, because fuzzy finding increases the chance that the wanted result is further at the top in the list of found results.

This is a really cheap method with a big impact. For the backend I would probably try to use something better directly.

The module is a slim wrapper around [fastest-levenshtein](https://github.com/ka-weihe/fastest-levenshtein) with some sensible defaults and using [Intl.Collator](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/Collator). The wrapper is a bit dated. So I would only copy the function as a helper and import fastest-levenshtein directly. Fuzzy matching in the api makes sense. Doing it in the frontend as well for the locally available data still makes sense, because fuzzy finding increases the chance that the wanted result is further at the top in the list of found results. This is a really cheap method with a big impact. For the backend I would probably try to use something better directly.
Review

But if the frontend and api both do fuzzy matching, what's the advantage of doing it in the frontend as well? Wouldn't that give use the same results? (right now neither does, but it should be done in the api anyway)

But if the frontend and api both do fuzzy matching, what's the advantage of doing it in the frontend as well? Wouldn't that give use the same results? (right now neither does, but it should be done in the api anyway)
Review

We have a few points in the code where we used strict matching for searching in local (aka downloaded to frontend) data. I thought this was one of those places. Since adding levenshtein seems simple I thought we would have a quick-win here. If we are offline first this also enables usage when offline without server request. To get up-to-date data an api-request is obviously unavoidable. So I'm unsure what our general strategy here is, I guess?

We have a few points in the code where we used strict matching for searching in local (aka downloaded to frontend) data. I thought this was one of those places. Since adding levenshtein seems simple I thought we would have a quick-win here. If we are offline first this also enables usage when offline without server request. To get up-to-date data an api-request is obviously unavoidable. So I'm unsure what our general strategy here is, I guess?
Review

Sure, we could add it but I'm not sure how useful that would be in the current state. The offline argument sticks though… but then we could do that when we have offline storage. Better not complicate things more than we must right now to solve this.

Sure, we could add it but I'm not sure how useful that would be in the current state. The offline argument sticks though… but then we could do that when we have offline storage. Better not complicate things more than we must right now to solve this.
function findPropertyByValue(object, key, value) {
return Object.values(object).find(
(l) => l[key]?.toLowerCase() === value.toLowerCase(),
)
function findPropertyByValue(object, key, value, fuzzy = false) {
return Object.values(object).find(l => {
if (fuzzy) {
return l[key]?.toLowerCase().includes(value.toLowerCase())
}
return l[key]?.toLowerCase() === value.toLowerCase()
})
}
// Check if the user exists in the search results
@ -42,9 +50,19 @@ function validateUser(
users: IUser[],
query: IUser['username'] | IUser['name'] | IUser['email'],
) {
return findPropertyByValue(users, 'username', query) ||
if (users.length === 1) {
return (
findPropertyByValue(users, 'username', query, true) ||
findPropertyByValue(users, 'name', query, true) ||
findPropertyByValue(users, 'email', query, true)
)
}
return (
findPropertyByValue(users, 'username', query) ||
findPropertyByValue(users, 'name', query) ||
findPropertyByValue(users, 'email', query)
)
}
// Check if the label exists
@ -63,14 +81,18 @@ async function addLabelToTask(task: ITask, label: ILabel) {
return response
}
async function findAssignees(parsedTaskAssignees: string[]): Promise<IUser[]> {
async function findAssignees(parsedTaskAssignees: string[], projectId: number): Promise<MatchedAssignee[]> {
if (parsedTaskAssignees.length <= 0) {
return []
}
const userService = new UserService()
const userService = new ProjectUserService()
const assignees = parsedTaskAssignees.map(async a => {
const users = await userService.getAll({}, {s: a})
const users = (await userService.getAll({projectId}, {s: a}))
.map(u => ({
...u,
match: a,
}))
return validateUser(users, a)
})
@ -388,15 +410,15 @@ export const useTaskStore = defineStore('task', () => {
cancel()
throw new Error('NO_PROJECT')
}
const assignees = await findAssignees(parsedTask.assignees)
const assignees = await findAssignees(parsedTask.assignees, foundProjectId)
// Only clean up those assignees from the task title which actually exist
let cleanedTitle = parsedTask.text
if (assignees.length > 0) {
const assigneePrefix = PREFIXES[quickAddMagicMode]?.assignee
if (assigneePrefix) {
cleanedTitle = cleanupItemText(cleanedTitle, assignees.map(a => a.username), assigneePrefix)
cleanedTitle = cleanupItemText(cleanedTitle, assignees.map(a => a.match), assigneePrefix)
}
}