WIP: feat: add default reminder for tasks with a due date #2360
|
@ -36,35 +36,35 @@
|
|||
<tbody>
|
||||
<tr>
|
||||
<td><code>s</code></td>
|
||||
<td>{{ $t('input.datemathHelp.units.seconds') }}</td>
|
||||
<td>{{ $tc('time.seconds', 2) }}</td>
|
||||
</tr>
|
||||
<tr>
|
||||
<td><code>m</code></td>
|
||||
<td>{{ $t('input.datemathHelp.units.minutes') }}</td>
|
||||
<td>{{ $tc('time.minutes', 2) }}</td>
|
||||
</tr>
|
||||
<tr>
|
||||
<td><code>h</code></td>
|
||||
<td>{{ $t('input.datemathHelp.units.hours') }}</td>
|
||||
<td>{{ $tc('time.hours', 2) }}</td>
|
||||
</tr>
|
||||
<tr>
|
||||
<td><code>H</code></td>
|
||||
<td>{{ $t('input.datemathHelp.units.hours') }}</td>
|
||||
<td>{{ $tc('time.hours', 2) }}</td>
|
||||
</tr>
|
||||
<tr>
|
||||
<td><code>d</code></td>
|
||||
<td>{{ $t('input.datemathHelp.units.days') }}</td>
|
||||
<td>{{ $tc('time.days', 2) }}</td>
|
||||
</tr>
|
||||
<tr>
|
||||
<td><code>w</code></td>
|
||||
<td>{{ $t('input.datemathHelp.units.weeks') }}</td>
|
||||
<td>{{ $tc('time.weeks', 2) }}</td>
|
||||
</tr>
|
||||
<tr>
|
||||
<td><code>M</code></td>
|
||||
<td>{{ $t('input.datemathHelp.units.months') }}</td>
|
||||
<td>{{ $tc('time.months', 2) }}</td>
|
||||
</tr>
|
||||
<tr>
|
||||
<td><code>y</code></td>
|
||||
<td>{{ $t('input.datemathHelp.units.years') }}</td>
|
||||
<td>{{ $tc('time.years', 2) }}</td>
|
||||
</tr>
|
||||
</tbody>
|
||||
</table>
|
||||
|
|
|
@ -48,11 +48,11 @@
|
|||
@change="updateData"
|
||||
:disabled="disabled || undefined"
|
||||
>
|
||||
<option value="hours">{{ $t('task.repeat.hours') }}</option>
|
||||
<option value="days">{{ $t('task.repeat.days') }}</option>
|
||||
<option value="weeks">{{ $t('task.repeat.weeks') }}</option>
|
||||
<option value="months">{{ $t('task.repeat.months') }}</option>
|
||||
<option value="years">{{ $t('task.repeat.years') }}</option>
|
||||
<option value="hours">{{ $tc('time.hours', 2) }}</option>
|
||||
<option value="days">{{ $tc('time.days', 2) }}</option>
|
||||
<option value="weeks">{{ $tc('time.weeks', 2) }}</option>
|
||||
<option value="months">{{ $tc('time.months', 2) }}</option>
|
||||
<option value="years">{{ $tc('time.years', 2) }}</option>
|
||||
</select>
|
||||
</div>
|
||||
</div>
|
||||
|
|
|
@ -0,0 +1,63 @@
|
|||
import {describe, it, expect, vi, afterEach, beforeEach} from 'vitest'
|
||||
import {
|
||||
AMOUNTS_IN_SECONDS,
|
||||
getDefaultReminderSettings,
|
||||
getSavedReminderSettings,
|
||||
parseSavedReminderAmount,
|
||||
saveDefaultReminder,
|
||||
} from '@/helpers/defaultReminder'
|
||||
import * as exports from '@/helpers/defaultReminder'
|
||||
|
||||
describe('Default Reminder Save', () => {
|
||||
it('Should save a default reminder with minutes', () => {
|
||||
const spy = vi.spyOn(window.localStorage, 'setItem')
|
||||
saveDefaultReminder(true, 'minutes', 5)
|
||||
|
||||
expect(spy).toHaveBeenCalledWith('defaultReminder', '{"enabled":true,"amount":300}')
|
||||
})
|
||||
it('Should save a default reminder with hours', () => {
|
||||
const spy = vi.spyOn(window.localStorage, 'setItem')
|
||||
saveDefaultReminder(true, 'hours', 5)
|
||||
|
||||
expect(spy).toHaveBeenCalledWith('defaultReminder', '{"enabled":true,"amount":18000}')
|
||||
})
|
||||
it('Should save a default reminder with days', () => {
|
||||
const spy = vi.spyOn(window.localStorage, 'setItem')
|
||||
saveDefaultReminder(true, 'days', 5)
|
||||
|
||||
expect(spy).toHaveBeenCalledWith('defaultReminder', '{"enabled":true,"amount":432000}')
|
||||
})
|
||||
it('Should save a default reminder with months', () => {
|
||||
const spy = vi.spyOn(window.localStorage, 'setItem')
|
||||
saveDefaultReminder(true, 'months', 5)
|
||||
|
||||
expect(spy).toHaveBeenCalledWith('defaultReminder', '{"enabled":true,"amount":12960000}')
|
||||
})
|
||||
})
|
||||
|
||||
describe('Default Reminder Load', () => {
|
||||
it('Should parse minutes', () => {
|
||||
const settings = parseSavedReminderAmount(5 * AMOUNTS_IN_SECONDS.minutes)
|
||||
|
||||
expect(settings.amount).toBe(5)
|
||||
expect(settings.type).toBe('minutes')
|
||||
})
|
||||
it('Should parse hours', () => {
|
||||
const settings = parseSavedReminderAmount(5 * AMOUNTS_IN_SECONDS.hours)
|
||||
|
||||
expect(settings.amount).toBe(5)
|
||||
expect(settings.type).toBe('hours')
|
||||
})
|
||||
it('Should parse days', () => {
|
||||
const settings = parseSavedReminderAmount(5 * AMOUNTS_IN_SECONDS.days)
|
||||
|
||||
expect(settings.amount).toBe(5)
|
||||
expect(settings.type).toBe('days')
|
||||
})
|
||||
it('Should parse months', () => {
|
||||
const settings = parseSavedReminderAmount(5 * AMOUNTS_IN_SECONDS.months)
|
||||
|
||||
expect(settings.amount).toBe(5)
|
||||
expect(settings.type).toBe('months')
|
||||
})
|
||||
})
|
|
@ -0,0 +1,89 @@
|
|||
const DEFAULT_REMINDER_KEY = 'defaultReminder'
|
||||
|
||||
export const AMOUNTS_IN_SECONDS: {
|
||||
konrad marked this conversation as resolved
|
||||
[type in SavedReminderSettings['type']]: number
|
||||
} = {
|
||||
minutes: 60,
|
||||
hours: 60 * 60,
|
||||
days: 60 * 60 * 24,
|
||||
months: 60 * 60 * 24 * 30,
|
||||
} as const
|
||||
|
||||
interface DefaultReminderSettings {
|
||||
enabled: boolean,
|
||||
konrad marked this conversation as resolved
Outdated
dpschen
commented
Add Add `as const`
konrad
commented
Done. Done.
|
||||
amount: number,
|
||||
}
|
||||
|
||||
konrad marked this conversation as resolved
Outdated
dpschen
commented
We should save the factors in array in a mapping constant. This way the constant can be reused for the test. We should save the factors in array in a mapping constant. This way the constant can be reused for the test.
konrad
commented
Done. Done.
|
||||
interface SavedReminderSettings {
|
||||
enabled: boolean,
|
||||
amount: number,
|
||||
type: 'minutes' | 'hours' | 'days' | 'months',
|
||||
}
|
||||
|
||||
function calculateDefaultReminderSeconds(type: SavedReminderSettings['type'], amount: number): number {
|
||||
return amount * (AMOUNTS_IN_SECONDS[type] || 0)
|
||||
}
|
||||
|
||||
konrad marked this conversation as resolved
dpschen
commented
Maybe we should save the
`type` would better be as `keyof typeof AMOUNT_IN_SECONDS`.
Maybe we should save the `type` as new type, e.g.
```ts
type IReminderDurationType = 'minutes' | 'hours' | 'days' | 'months'
```
konrad
commented
I've changed it to use I've changed it to use `SavedReminderSettings['type']` instead.
|
||||
export function saveDefaultReminder(enabled: boolean, type: SavedReminderSettings['type'], amount: number) {
|
||||
const defaultReminderSeconds = calculateDefaultReminderSeconds(type, amount)
|
||||
localStorage.setItem(DEFAULT_REMINDER_KEY, JSON.stringify(<DefaultReminderSettings>{
|
||||
enabled,
|
||||
amount: defaultReminderSeconds,
|
||||
}))
|
||||
}
|
||||
|
||||
export function getDefaultReminderAmount(): number | null {
|
||||
const settings = getDefaultReminderSettings()
|
||||
|
||||
return settings?.enabled
|
||||
? settings.amount
|
||||
dpschen marked this conversation as resolved
dpschen
commented
picky: don't create one-use-variable picky: don't create one-use-variable
konrad
commented
But here I'm using it twice? But here I'm using it twice?
dpschen
commented
Beeing too picky is also stupid… you are right! Beeing too picky is also stupid… you are right!
|
||||
: null
|
||||
}
|
||||
|
||||
export function getDefaultReminderSettings(): DefaultReminderSettings | null {
|
||||
const s: string | null = window.localStorage.getItem(DEFAULT_REMINDER_KEY)
|
||||
if (s === null) {
|
||||
return null
|
||||
}
|
||||
|
||||
return JSON.parse(s)
|
||||
}
|
||||
|
||||
export function parseSavedReminderAmount(amountSeconds: number): SavedReminderSettings {
|
||||
const amountMinutes = amountSeconds / 60
|
||||
const settings: SavedReminderSettings = {
|
||||
dpschen
commented
Why don't we save the type as well? Wouldn't that make thinks simpler? Why don't we save the type as well? Wouldn't that make thinks simpler?
konrad
commented
I guess I just thought about having the seconds ready to do the calculation, because that's what we need at the end. I guess I just thought about having the seconds ready to do the calculation, because that's what we need at the end.
dpschen
commented
Wouldn't it be easier to keep the format until it's actually displayed? Wouldn't it be easier to keep the format until it's actually displayed?
konrad
commented
It might. It might.
|
||||
enabled: true, // We're assuming the caller to have checked this properly
|
||||
amount: amountMinutes,
|
||||
type: 'minutes',
|
||||
}
|
||||
|
||||
if ((amountMinutes / 60 / 24) % 30 === 0) {
|
||||
settings.amount = amountMinutes / 60 / 24 / 30
|
||||
settings.type = 'months'
|
||||
} else if ((amountMinutes / 60) % 24 === 0) {
|
||||
settings.amount = amountMinutes / 60 / 24
|
||||
settings.type = 'days'
|
||||
} else if (amountMinutes % 60 === 0) {
|
||||
settings.amount = amountMinutes / 60
|
||||
settings.type = 'hours'
|
||||
}
|
||||
|
||||
return settings
|
||||
}
|
||||
|
||||
export function getSavedReminderSettings(): SavedReminderSettings | null {
|
||||
const s = getDefaultReminderSettings()
|
||||
if (s === null) {
|
||||
return null
|
||||
}
|
||||
|
||||
if (!s.enabled) {
|
||||
return {
|
||||
enabled: false,
|
||||
type: 'minutes',
|
||||
amount: 0,
|
||||
}
|
||||
}
|
||||
|
||||
return parseSavedReminderAmount(s.amount)
|
||||
}
|
|
@ -87,7 +87,11 @@
|
|||
"language": "Language",
|
||||
"defaultList": "Default List",
|
||||
"timezone": "Time Zone",
|
||||
"overdueTasksRemindersTime": "Overdue tasks reminder email time"
|
||||
"overdueTasksRemindersTime": "Overdue tasks reminder email time",
|
||||
"defaultReminder": "Set a default task reminder",
|
||||
"defaultReminderHint": "If enabled, Vikunja will automatically create a reminder for a task if you set a due date and the task does not have any reminders yet.",
|
||||
"defaultReminderAmount": "Default task reminder amount",
|
||||
"defaultReminderAmountBefore": "before the due date of a task"
|
||||
},
|
||||
"totp": {
|
||||
"title": "Two Factor Authentication",
|
||||
|
@ -547,19 +551,16 @@
|
|||
"fromto": "{from} to {to}",
|
||||
"ranges": {
|
||||
"today": "Today",
|
||||
|
||||
"thisWeek": "This Week",
|
||||
"restOfThisWeek": "The Rest of This Week",
|
||||
"nextWeek": "Next Week",
|
||||
"next7Days": "Next 7 Days",
|
||||
"lastWeek": "Last Week",
|
||||
|
||||
"thisMonth": "This Month",
|
||||
"restOfThisMonth": "The Rest of This Month",
|
||||
"nextMonth": "Next Month",
|
||||
"next30Days": "Next 30 Days",
|
||||
"lastMonth": "Last Month",
|
||||
|
||||
"thisYear": "This Year",
|
||||
"restOfThisYear": "The Rest of This Year"
|
||||
}
|
||||
|
@ -576,15 +577,6 @@
|
|||
"roundDay": "Round down to the nearest day",
|
||||
"supportedUnits": "Supported time units are:",
|
||||
"someExamples": "Some examples of time expressions:",
|
||||
"units": {
|
||||
"seconds": "Seconds",
|
||||
"minutes": "Minutes",
|
||||
"hours": "Hours",
|
||||
"days": "Days",
|
||||
"weeks": "Weeks",
|
||||
"months": "Months",
|
||||
"years": "Years"
|
||||
},
|
||||
"examples": {
|
||||
"now": "Right now",
|
||||
"in24h": "In 24h",
|
||||
|
@ -596,6 +588,15 @@
|
|||
}
|
||||
}
|
||||
},
|
||||
"time": {
|
||||
konrad marked this conversation as resolved
Outdated
dpschen
commented
Use vue-i18n pluralisation feature. Use [vue-i18n pluralisation](https://vue-i18n.intlify.dev/guide/essentials/pluralization.html) feature.
Or didn't you use that to be more because of crowdin?
konrad
commented
I actually just forgot about that feature :) will change it. I actually just forgot about that feature :) will change it.
konrad
commented
Done. Done.
|
||||
"seconds": "Second | Seconds",
|
||||
"minutes": "Minute | Minutes",
|
||||
"hours": "Hour | Hours",
|
||||
"days": "Day | Days",
|
||||
"weeks": "Week | Weeks",
|
||||
"months": "Month | Months",
|
||||
"years": "Year | Years"
|
||||
},
|
||||
"task": {
|
||||
"task": "Task",
|
||||
"new": "Create a new task",
|
||||
|
|
|
@ -12,4 +12,6 @@ export interface IUserSettings extends IAbstract {
|
|||
weekStart: 0 | 1 | 2 | 3 | 4 | 5 | 6
|
||||
timezone: string
|
||||
language: string
|
||||
defaultReminder: boolean
|
||||
defaultReminderAmount: number // The amount of seconds a reminder should be set before a given due date
|
||||
}
|
|
@ -2,6 +2,7 @@ import AbstractModel from './abstractModel'
|
|||
|
||||
import type {IUserSettings} from '@/modelTypes/IUserSettings'
|
||||
import {getCurrentLanguage} from '@/i18n'
|
||||
import {getDefaultReminderAmount} from '@/helpers/defaultReminder'
|
||||
|
||||
export default class UserSettingsModel extends AbstractModel<IUserSettings> implements IUserSettings {
|
||||
name = ''
|
||||
|
@ -13,6 +14,8 @@ export default class UserSettingsModel extends AbstractModel<IUserSettings> impl
|
|||
weekStart = 0 as IUserSettings['weekStart']
|
||||
timezone = ''
|
||||
language = getCurrentLanguage()
|
||||
konrad marked this conversation as resolved
Outdated
dpschen
commented
se here se here `getDefaultReminderAmount`
konrad
commented
Done. Done.
|
||||
defaultReminder = false
|
||||
defaultReminderAmount = getDefaultReminderAmount() || 0
|
||||
|
||||
constructor(data: Partial<IUserSettings> = {}) {
|
||||
super()
|
||||
|
|
|
@ -6,6 +6,7 @@ import LabelService from './label'
|
|||
|
||||
import {formatISO} from 'date-fns'
|
||||
import {colorFromHex} from '@/helpers/color/colorFromHex'
|
||||
import {getDefaultReminderAmount} from '@/helpers/defaultReminder'
|
||||
|
||||
const parseDate = date => {
|
||||
if (date) {
|
||||
|
@ -68,6 +69,15 @@ export default class TaskService extends AbstractService<ITask> {
|
|||
})
|
||||
}
|
||||
|
||||
if (model.dueDate !== null && model.reminderDates.length === 0) {
|
||||
const defaultReminder = getDefaultReminderAmount()
|
||||
if (defaultReminder !== null) {
|
||||
const dueDate = +new Date(model.dueDate)
|
||||
const reminderDate = new Date(dueDate - (defaultReminder * 1000))
|
||||
model.reminderDates.push(formatISO(reminderDate))
|
||||
}
|
||||
}
|
||||
|
||||
dpschen
commented
Below seems to be the same calculation as in the new Below seems to be the same calculation as in the new `calculateDefaultReminderSeconds` function. We should probably combine these.
konrad
commented
Looks very similar, yes. Looks very similar, yes.
|
||||
// Make the repeating amount to seconds
|
||||
let repeatAfterSeconds = 0
|
||||
if (model.repeatAfter !== null && (model.repeatAfter.amount !== null || model.repeatAfter.amount !== 0)) {
|
||||
|
|
|
@ -56,6 +56,7 @@
|
|||
}
|
||||
}
|
||||
|
||||
.field.has-addons .select select,
|
||||
konrad marked this conversation as resolved
Outdated
dpschen
commented
Combine with selector below. Maybe better: use class for specific case in General Combine with selector below.
Maybe better: use class for specific case in General
konrad
commented
I've combined them. I've combined them.
|
||||
.field.has-addons .control .select select {
|
||||
height: 100%;
|
||||
}
|
||||
|
|
|
@ -698,6 +698,9 @@ export default defineComponent({
|
|||
|
||||
this.task = await this.$store.dispatch('tasks/update', task)
|
||||
|
||||
// Show new fields set from the api or a newly set default reminder
|
||||
this.$nextTick(() => this.setActiveFields())
|
||||
dpschen
commented
I don't understand why the nextTick is necessary here. I don't understand why the nextTick is necessary here.
konrad
commented
The comment is wrong but the tick is required to make sure the maybe newly set reminder would be shown in the task detail view as well. I'll clarify the comment. The comment is wrong but the tick is required to make sure the maybe newly set reminder would be shown in the task detail view as well. I'll clarify the comment.
|
||||
|
||||
if (!showNotification) {
|
||||
return
|
||||
}
|
||||
|
|
|
@ -18,6 +18,50 @@
|
|||
</label>
|
||||
<list-search v-model="defaultList"/>
|
||||
</div>
|
||||
<div class="field">
|
||||
<label class="checkbox">
|
||||
<input type="checkbox" v-model="defaultReminderEnabled"/>
|
||||
{{ $t('user.settings.general.defaultReminder') }}
|
||||
</label>
|
||||
<p class="is-size-7">
|
||||
{{ $t('user.settings.general.defaultReminderHint') }}
|
||||
</p>
|
||||
</div>
|
||||
<div class="field" v-if="defaultReminderEnabled">
|
||||
<label class="label" for="defaultReminderAmount">
|
||||
{{ $t('user.settings.general.defaultReminderAmount') }}
|
||||
</label>
|
||||
<div class="field has-addons is-align-items-center">
|
||||
<div class="control">
|
||||
<input
|
||||
@keyup.enter="updateSettings"
|
||||
class="input"
|
||||
id="defaultReminderAmount"
|
||||
type="number"
|
||||
min="0"
|
||||
v-model="defaultReminderAmount"/>
|
||||
</div>
|
||||
<div class="control select">
|
||||
<select v-model="defaultReminderAmountType">
|
||||
<option value="minutes">
|
||||
{{ $tc('time.minutes', defaultReminderAmount) }}
|
||||
konrad marked this conversation as resolved
Outdated
dpschen
commented
In case we don't use vue-i18n pluralisation feature we shouldn't use local messages that can't be precompiled (see: https://vue-i18n.intlify.dev/guide/advanced/optimization.html#optimization).
In case we don't use vue-i18n pluralisation feature we shouldn't use local messages that can't be precompiled (see: https://vue-i18n.intlify.dev/guide/advanced/optimization.html#optimization).
```ts
{{ defaultReminderAmount === 1 ? $t('time.minute') : $t('time.minutes') }}
```
|
||||
</option>
|
||||
<option value="hours">
|
||||
{{ $tc('time.hours', defaultReminderAmount) }}
|
||||
</option>
|
||||
<option value="days">
|
||||
{{ $tc('time.days', defaultReminderAmount) }}
|
||||
</option>
|
||||
<option value="months">
|
||||
{{ $tc('time.months', defaultReminderAmount) }}
|
||||
</option>
|
||||
</select>
|
||||
</div>
|
||||
<p class="pl-2">
|
||||
{{ $t('user.settings.general.defaultReminderAmountBefore') }}
|
||||
</p>
|
||||
</div>
|
||||
</div>
|
||||
<div class="field">
|
||||
<label class="checkbox">
|
||||
<input type="checkbox" v-model="settings.overdueTasksRemindersEnabled"/>
|
||||
|
@ -175,6 +219,7 @@ import {AuthenticatedHTTPFactory} from '@/http-common'
|
|||
import {useColorScheme} from '@/composables/useColorScheme'
|
||||
import {useTitle} from '@/composables/useTitle'
|
||||
import {objectIsEmpty} from '@/helpers/objectIsEmpty'
|
||||
import {getSavedReminderSettings, saveDefaultReminder} from '@/helpers/defaultReminder'
|
||||
|
||||
const {t} = useI18n({useScope: 'global'})
|
||||
useTitle(() => `${t('user.settings.general.title')} - ${t('user.settings.title')}`)
|
||||
|
@ -266,9 +311,16 @@ watch(
|
|||
async function updateSettings() {
|
||||
localStorage.setItem(playSoundWhenDoneKey, playSoundWhenDone.value ? 'true' : 'false')
|
||||
setQuickAddMagicMode(quickAddMagicMode.value)
|
||||
saveDefaultReminder(defaultReminderEnabled.value, defaultReminderAmountType.value, defaultReminderAmount.value)
|
||||
|
||||
await authStore.saveUserSettings({
|
||||
settings: {...settings.value},
|
||||
})
|
||||
}
|
||||
|
||||
const reminderSettings = getSavedReminderSettings()
|
||||
|
||||
const defaultReminderEnabled = ref<boolean>(reminderSettings?.enabled || false)
|
||||
const defaultReminderAmount = ref(reminderSettings?.amount || 1)
|
||||
const defaultReminderAmountType = ref(reminderSettings?.type || 'days')
|
||||
konrad marked this conversation as resolved
dpschen
commented
Shouldn't the fallback be the same as the default in the model? Shouldn't the fallback be the same as the default in the model?
konrad
commented
Actually not, because this is what gets shown in the input when setting this for the first time. I felt like putting a Actually not, because this is what gets shown in the input when setting this for the first time. I felt like putting a `0` here would feel weired.
|
||||
</script>
|
||||
|
|
|
@ -32,6 +32,7 @@ export default defineConfig({
|
|||
// https://vitest.dev/config/
|
||||
test: {
|
||||
environment: 'happy-dom',
|
||||
mockReset: true,
|
||||
},
|
||||
css: {
|
||||
preprocessorOptions: {
|
||||
|
|
Reuse type definition. E.g.
Done.