WIP: feat: add default reminder for tasks with a due date #2360

Draft
konrad wants to merge 17 commits from feature/add-default-reminder into main
12 changed files with 252 additions and 27 deletions

View File

@ -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>

View File

@ -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>

View File

@ -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')
})
})

View File

@ -0,0 +1,89 @@
const DEFAULT_REMINDER_KEY = 'defaultReminder'
export const AMOUNTS_IN_SECONDS: {
konrad marked this conversation as resolved
Review

Reuse type definition. E.g.

{ [type in SavedReminderSettings['type']]: number }
Reuse type definition. E.g. ```ts { [type in SavedReminderSettings['type']]: number } ```
Review

Done.

Done.
[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,
amount: number,
}
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
Review

type would better be as keyof typeof AMOUNT_IN_SECONDS.

Maybe we should save the type as new type, e.g.

type IReminderDurationType = 'minutes' | 'hours' | 'days' | 'months'
`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' ```
Review

I've changed it to use SavedReminderSettings['type'] instead.

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
Review

picky: don't create one-use-variable

picky: don't create one-use-variable
Review

But here I'm using it twice?

But here I'm using it twice?
Review

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 = {
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)
}

View File

@ -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": {
"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",

View File

@ -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
}

View File

@ -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()
defaultReminder = false
defaultReminderAmount = getDefaultReminderAmount() || 0
constructor(data: Partial<IUserSettings> = {}) {
super()

View File

@ -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) {
@ -39,7 +40,7 @@ export default class TaskService extends AbstractService<ITask> {
}
processModel(updatedModel) {
const model = { ...updatedModel }
const model = {...updatedModel}
model.title = model.title?.trim()
@ -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))
}
}
// Make the repeating amount to seconds
let repeatAfterSeconds = 0
if (model.repeatAfter !== null && (model.repeatAfter.amount !== null || model.repeatAfter.amount !== 0)) {

View File

@ -56,6 +56,7 @@
}
}
.field.has-addons .select select,
.field.has-addons .control .select select {
height: 100%;
}

View File

@ -697,6 +697,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())
Review

I don't understand why the nextTick is necessary here.

I don't understand why the nextTick is necessary here.
Review

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

View File

@ -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) }}
</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
Review

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?
Review

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.

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>

View File

@ -32,6 +32,7 @@ export default defineConfig({
// https://vitest.dev/config/
test: {
environment: 'happy-dom',
mockReset: true,
},
css: {
preprocessorOptions: {