feat: add date math for filters #1342

Merged
konrad merged 88 commits from feature/date-math into main 2022-03-28 17:30:43 +00:00
7 changed files with 50 additions and 29 deletions
Showing only changes of commit 84f177c80e - Show all commits

View File

@ -139,8 +139,8 @@ watch(
emitChanged()

This is triggered three times!

Once for each of these lines:

from.value = fromDate
to.value = toDate

in the called function inputChanged of the watcher.
And then here again.

This is triggered three times! Once for each of these lines: ```js from.value = fromDate to.value = toDate ``` in the called function inputChanged of the watcher. And then here again.

mhh how can we fix this? Using a debounce to prevent it getting called three times if it changed all values at once? Or removing the watchers again?

It looks like the tasks are only loaded once, not three times. Maybe that's already enough?

mhh how can we fix this? Using a debounce to prevent it getting called three times if it changed all values at once? Or removing the watchers again? It looks like the tasks are only loaded once, not three times. Maybe that's already enough?
},
)
watch(() => from, inputChanged)
watch(() => to, inputChanged)
watch(() => from.value, inputChanged)
watch(() => to.value, inputChanged)
function setDateRange(range: string[] | null) {
if (range === null) {
@ -152,7 +152,6 @@ function setDateRange(range: string[] | null) {
from.value = range[0]
to.value = range[1]
}
const customRangeActive = computed<boolean>(() => {

View File

@ -0,0 +1,3 @@
export function getNextWeekDate(): Date {
return new Date((new Date()).getTime() + 7 * 24 * 60 * 60 * 1000)
}

View File

@ -1,4 +1,4 @@
export function parseDateOrString(rawValue: string, fallback: any): string | Date {
export function parseDateOrString(rawValue: string | undefined, fallback: any): string | Date {
konrad marked this conversation as resolved Outdated

Maybe we should just move this to the emits section of the date range component? To avoid callees requiring to import it every time?

Maybe we should just move this to the emits section of the date range component? To avoid callees requiring to import it every time?

Yes that makes sense!

Yes that makes sense!

Since we're now using route props where we need this function I think we'll have to leave it as is.

Since we're now using route props where we need this function I think we'll have to leave it as is.
if (typeof rawValue === 'undefined') {
return fallback
}

View File

@ -3,6 +3,8 @@ import {saveLastVisited} from '@/helpers/saveLastVisited'
import {store} from '@/store'
import {saveListView, getListView} from '@/helpers/saveListView'
import {parseDateOrString} from '@/helpers/time/parseDateOrString'
import {getNextWeekDate} from '@/helpers/time/getNextWeekDate'
import HomeComponent from '../views/Home.vue'
import NotFoundComponent from '../views/404.vue'
@ -249,6 +251,12 @@ const router = createRouter({
path: '/tasks/by/upcoming',
name: 'tasks.range',
component: UpcomingTasksComponent,
props: route => ({
dateFrom: parseDateOrString(route.query.from, new Date()),
dateTo: parseDateOrString(route.query.to, getNextWeekDate()),
showNulls: route.query.showNulls === 'true',
showOverdue: route.query.showOverdue === 'true',
})
},
{
path: '/lists/new/:namespaceId/',

View File

@ -50,7 +50,12 @@
/>
</div>
</div>
<ShowTasks class="mt-4" :show-all="true" v-if="hasLists" :key="showTasksKey"/>
<ShowTasks
v-if="hasLists"
class="mt-4"
:show-all="true"
:key="showTasksKey"
/>
</div>
</template>
@ -83,13 +88,14 @@ const userInfo = computed(() => store.state.auth.info)
const hasTasks = computed(() => store.state.hasTasks)
const defaultListId = computed(() => store.state.auth.defaultListId)
const defaultNamespaceId = computed(() => store.state.namespaces.namespaces?.[0]?.id || 0)
const hasLists = computed (() => store.state.namespaces.namespaces?.[0]?.lists.length > 0)
const hasLists = computed(() => store.state.namespaces.namespaces?.[0]?.lists.length > 0)
const loading = computed(() => store.state.loading && store.state.loadingModule === 'tasks')
const deletionScheduledAt = computed(() => parseDateOrNull(store.state.auth.info?.deletionScheduledAt))
// This is to reload the tasks list after adding a new task through the global task add.
// FIXME: Should use vuex (somehow?)
const showTasksKey = ref(0)
function updateTaskList() {
showTasksKey.value++
}

View File

@ -62,10 +62,6 @@ import {useI18n} from 'vue-i18n'
import {setTitle} from '@/helpers/setTitle'
import {DATE_RANGES} from '@/components/date/dateRanges'
function getNextWeekDate() {
return new Date((new Date()).getTime() + 7 * 24 * 60 * 60 * 1000)
}
const store = useStore()
const route = useRoute()
const router = useRouter()
@ -76,28 +72,35 @@ const showNothingToDo = ref<boolean>(false)
setTimeout(() => showNothingToDo.value = true, 100)
const props = defineProps({
showAll: Boolean,
})
// NOTE: You MUST provide either dateFrom and dateTo OR showAll for the component to actually show tasks.
konrad marked this conversation as resolved Outdated

Wouldn't it be better to make showAll a computed that gets autoset when dateFrom and dateTo doesn't contain a value?

Wouldn't it be better to make showAll a computed that gets autoset when dateFrom and dateTo doesn't contain a value?

Excellent idea. Changed it!

Excellent idea. Changed it!
const {
dateFrom,
dateTo,
showAll = false,
showNulls = false,
showOverdue = false,
} = defineProps<{
dateFrom?: Date | string,
dateTo?: Date | string,
showAll?: Boolean,
showNulls?: Boolean,
showOverdue?: Boolean,
konrad marked this conversation as resolved Outdated

get value from props to remove dependency on route. Removing dependency from router makes the components easier reusable nested inside another view (which is what we do).

get value from props to remove dependency on route. Removing dependency from router makes the components easier reusable nested inside another view (which is what we do).
}>()
const dateFrom = computed<Date | string>(() => parseDateOrString(route.query.from as string, new Date()))
const dateTo = computed<Date | string>(() => parseDateOrString(route.query.to as string, getNextWeekDate()))
const showNulls = computed(() => route.query.showNulls === 'true')
const showOverdue = computed(() => route.query.showOverdue === 'true')
const pageTitle = computed(() => {
let title = ''
// We need to define "key" because it is the first parameter in the array and we need the second
konrad marked this conversation as resolved Outdated

Readd route props

Readd route props

Changed it to route props.

However, we still have a dependency on the router: every time when seleting a date in ShowTasks it will push the change to the route. I don't know how to change that without massively overengeneering everything so I'd say we leave it at that.

Changed it to route props. However, we still have a dependency on the router: every time when seleting a date in ShowTasks it will push the change to the route. I don't know how to change that without massively overengeneering everything so I'd say we leave it at that.

I think it's fine for now. I thought a while about this but also don't have a better soltion for this at the moment (except the v-model version)

I think it's fine for now. I thought a while about this but also don't have a better soltion for this at the moment (except the v-model version)
// eslint-disable-next-line no-unused-vars
const predefinedRange = Object.entries(DATE_RANGES).find(([key, value]) => dateFrom.value === value[0] && dateTo.value === value[1])
const predefinedRange = Object.entries(DATE_RANGES).find(([key, value]) => dateFrom === value[0] && dateTo === value[1])
if (typeof predefinedRange !== 'undefined') {
title = t(`input.datepickerRange.ranges.${predefinedRange[0]}`)
konrad marked this conversation as resolved Outdated

Unsure: I think eslint doesn't complain here if you use underscore (_) to spread the unused variable.

Unsure: I think eslint doesn't complain here if you use underscore (`_`) to spread the unused variable.

Add ?.[0] so we return the key, which is what we actually want to use and adjust variable name.

Add `?.[0]` so we return the key, which is what we actually want to use and adjust variable name.

Unsure: I think eslint doesn't complain here if you use underscore (_) to spread the unused variable.

Nope, does not seem to have an effect.

> Unsure: I think eslint doesn't complain here if you use underscore (_) to spread the unused variable. Nope, does not seem to have an effect.

Add ?.[0] so we return the key, which is what we actually want to use and adjust variable name.

Done.

> Add ?.[0] so we return the key, which is what we actually want to use and adjust variable name. Done.

Nope, does not seem to have an effect.

Interesting!

> Nope, does not seem to have an effect. Interesting!
} else {
konrad marked this conversation as resolved Outdated

Remove title var and return directly:

if (typeof predefinedRange !== 'undefined') {
    return t(`input.datepickerRange.ranges.${predefinedRangeKey}`)
} else {
    return showAll
			? t('task.show.titleCurrent')
			: t('task.show.fromuntil', {
				from: formatDate(dateFrom, 'PPP'),
				until: formatDate(dateTo, 'PPP'),
			})
}
Remove title var and return directly: ```js if (typeof predefinedRange !== 'undefined') { return t(`input.datepickerRange.ranges.${predefinedRangeKey}`) } else { return showAll ? t('task.show.titleCurrent') : t('task.show.fromuntil', { from: formatDate(dateFrom, 'PPP'), until: formatDate(dateTo, 'PPP'), }) }

Done.

Done.
title = props.showAll
title = showAll
? t('task.show.titleCurrent')
: t('task.show.fromuntil', {
from: formatDate(dateFrom.value, 'PPP'),
until: formatDate(dateTo.value, 'PPP'),
from: formatDate(dateFrom, 'PPP'),
until: formatDate(dateTo, 'PPP'),
})
}
@ -140,8 +143,8 @@ function setDate(dates: dateStrings) {
query: {
from: dates.dateFrom ?? dateFrom,
to: dates.dateTo ?? dateTo,
showOverdue: showOverdue.value ? 'true' : 'false',
showNulls: showNulls.value ? 'true' : 'false',
showOverdue: showOverdue ? 'true' : 'false',
showNulls: showNulls ? 'true' : 'false',
},
dpschen marked this conversation as resolved Outdated

Mhh that is a bit annoying that we have to convert to strings here :/
But I also don't have a better idea for now.
Right now this might be still fine, but I remember similar usecases where the conversion from state object to the url representation was quite complex.

We might need to abstract this in the future, so let's keep that in the back of our head =)

Mhh that is a bit annoying that we have to convert to strings here :/ But I also don't have a better idea for now. Right now this might be still fine, but I remember similar usecases where the conversion from state object to the url representation was quite complex. We might need to abstract this in the future, so let's keep that in the back of our head =)
})
}
@ -181,10 +184,10 @@ async function loadPendingTasks(from: string, to: string) {
filter_value: ['false'],
filter_comparator: ['equals'],
filter_concat: 'and',
filter_include_nulls: showNulls.value,
filter_include_nulls: showNulls,
}
if (!props.showAll) {
if (!showAll) {
params.filter_by.push('due_date')
params.filter_value.push(to)
params.filter_comparator.push('less')
@ -192,7 +195,7 @@ async function loadPendingTasks(from: string, to: string) {
// NOTE: Ideally we could also show tasks with a start or end date in the specified range, but the api
// is not capable (yet) of combining multiple filters with 'and' and 'or'.
if (!showOverdue.value) {
if (!showOverdue) {
params.filter_by.push('due_date')
params.filter_value.push(from)
params.filter_comparator.push('greater')
@ -203,7 +206,7 @@ async function loadPendingTasks(from: string, to: string) {
}
// FIXME: this modification should happen in the store
function updateTasks(updatedTask) {
function updateTasks(updatedTask: TaskModel) {
for (const t in tasks.value) {
if (tasks.value[t].id === updatedTask.id) {
tasks.value[t] = updatedTask
konrad marked this conversation as resolved Outdated

Instead of creating the params already in snake_case:
Wait until the last moment (ideally abstracted away from a apiClient) until you convert params from camelCase to snake_case, needed by the api. This way we use consequently use camelCase in most areas of the frontend – which is what you usually do in JS.

Instead of creating the params already in snake_case: Wait until the last moment (ideally abstracted away from a apiClient) until you convert params from camelCase to snake_case, needed by the api. This way we use consequently use camelCase in most areas of the frontend – which is what you usually do in JS.

I agree. However, this whole filter thing is a mess right now, not only here. I'd like to fix this everywhere at once at some point, simplifying the filter handling in the process as well.

That being said, I've changed there here for now.

I agree. However, this whole filter thing is a mess right now, not only here. I'd like to fix this everywhere at once at some point, simplifying the filter handling in the process as well. That being said, I've changed there here for now.
@ -217,7 +220,7 @@ function updateTasks(updatedTask) {
}
}
watchEffect(() => loadPendingTasks(dateFrom.value as string, dateTo.value as string))
watchEffect(() => loadPendingTasks(dateFrom as string, dateTo as string))
watchEffect(() => setTitle(pageTitle.value))
</script>

View File

@ -1,7 +1,8 @@
/// <reference types="vitest" />
import { defineConfig } from 'vite'
import {defineConfig} from 'vite'
import vue from '@vitejs/plugin-vue'
import legacyFn from '@vitejs/plugin-legacy'
const {VitePWA} = require('vite-plugin-pwa')
const path = require('path')
const {visualizer} = require('rollup-plugin-visualizer')
@ -49,6 +50,7 @@ export default defineConfig({
},
},
},
reactivityTransform: true,
}),
legacy,
svgLoader({