feat: manage caldav tokens #1186
|
@ -103,9 +103,16 @@
|
|||
"disableSuccess": "Two factor authentication was sucessfully disabled."
|
||||
},
|
||||
"caldav": {
|
||||
"title": "Caldav",
|
||||
"howTo": "You can connect Vikunja to caldav clients to view and manage all tasks from different clients. Enter this url into your client:",
|
||||
"more": "More information about caldav in Vikunja"
|
||||
"title": "CalDAV",
|
||||
konrad marked this conversation as resolved
Outdated
|
||||
"howTo": "You can connect Vikunja to CalDAV clients to view and manage all tasks from different clients. Enter this url into your client:",
|
||||
"more": "More information about CalDAV in Vikunja",
|
||||
"tokens": "CalDAV Tokens",
|
||||
"tokensHowTo": "You can use a CalDAV token to use instead of a password to log in the above endpoint.",
|
||||
"createToken": "Create a token",
|
||||
"tokenCreated": "Here is your token: {token}",
|
||||
"wontSeeItAgain": "Write it down, you won't be able to see it again.",
|
||||
"mustUseToken": "You need to create a CalDAV token if you want to use CalDAV with a third party client. Use the token as the password.",
|
||||
"usernameIs": "Your username is: {0}"
|
||||
},
|
||||
"avatar": {
|
||||
"title": "Avatar",
|
||||
|
@ -484,7 +491,10 @@
|
|||
"showMenu": "Show the menu",
|
||||
"hideMenu": "Hide the menu",
|
||||
"forExample": "For example:",
|
||||
"welcomeBack": "Welcome Back!"
|
||||
"welcomeBack": "Welcome Back!",
|
||||
"id": "ID",
|
||||
"created": "Created at",
|
||||
"actions": "Actions"
|
||||
},
|
||||
"input": {
|
||||
"resetColor": "Reset Color",
|
||||
|
|
|
@ -0,0 +1,15 @@
|
|||
import AbstractModel from './abstractModel'
|
||||
|
||||
export default class CaldavTokenModel extends AbstractModel {
|
||||
constructor(data) {
|
||||
super(data)
|
||||
this.created = new Date(this.created)
|
||||
}
|
||||
|
||||
defaults() {
|
||||
konrad marked this conversation as resolved
Outdated
dpschen
commented
Why does the defaults function exist instead of using field declarations (maybe even private)? Asking because if it would be:
typing would be better. But maybe I miss something. Why does the defaults function exist instead of using [field declarations](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Classes#field_declarations) (maybe even private)?
Asking because if it would be:
```ts
export default class CaldavTokenModel extends AbstractModel {
id: 0;
created: null;
constructor(data) {
super(data)
this.created = new Date(this.created)
}
}
```
typing would be better. But maybe I miss something.
konrad
commented
I think it only exists because of my lack of js knowledge back a few years ago when I created the models and services :) It should be cleaned up at some point, but for all models at once (which is kinda out of scope for this PR). I'm pretty sure there's a better way to do this with typescript so that we have proper type hints everywhere. I think it only exists because of my lack of js knowledge back a few years ago when I created the models and services :) It should be cleaned up at some point, but for all models at once (which is kinda out of scope for this PR).
I'm pretty sure there's a better way to do this with typescript so that we have proper type hints everywhere.
dpschen
commented
I think field declarations didn't exist then. Agree with the rest of the stuff =) I think field declarations didn't exist then.
Why I was asking: I'm trying to fix all ts errors that I have while working on a segment. And if creating these props as field declarations helps ts I guess it would be a manageable effort.
Agree with the rest of the stuff =)
|
||||
return {
|
||||
id: 0,
|
||||
created: null,
|
||||
}
|
||||
}
|
||||
}
|
|
@ -0,0 +1,25 @@
|
|||
import {formatISO} from 'date-fns'
|
||||
import CaldavTokenModel from '../models/caldavToken'
|
||||
import AbstractService from './abstractService'
|
||||
|
||||
export default class CaldavTokenService extends AbstractService {
|
||||
constructor() {
|
||||
super({
|
||||
getAll: '/user/settings/token/caldav',
|
||||
create: '/user/settings/token/caldav',
|
||||
delete: '/user/settings/token/caldav/{id}',
|
||||
})
|
||||
}
|
||||
|
||||
processModel(model) {
|
||||
return {
|
||||
konrad marked this conversation as resolved
Outdated
dpschen
commented
Return a new model instead of modifying the existing:
Return a new model instead of modifying the existing:
```ts
return {
...model,
created: formatISO(new Date(model.created)),
}
```
konrad
commented
Done. Done.
|
||||
...model,
|
||||
created: formatISO(new Date(model.created)),
|
||||
}
|
||||
}
|
||||
|
||||
modelFactory(data) {
|
||||
return new CaldavTokenModel(data)
|
||||
}
|
||||
}
|
||||
|
|
@ -16,40 +16,106 @@
|
|||
/>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
<h5 class="mt-5 mb-4 has-text-weight-bold">
|
||||
konrad marked this conversation as resolved
Outdated
dpschen
commented
This seems to be a headline? => use This seems to be a headline? => use `<hX>`
|
||||
{{ $t('user.settings.caldav.tokens') }}
|
||||
</h5>
|
||||
|
||||
<p>
|
||||
<a href="https://vikunja.io/docs/caldav/" rel="noreferrer noopener nofollow" target="_blank">
|
||||
{{ isLocalUser ? $t('user.settings.caldav.tokensHowTo') : $t('user.settings.caldav.mustUseToken') }}
|
||||
<template v-if="!isLocalUser">
|
||||
<br/>
|
||||
<i18n-t keypath="user.settings.caldav.usernameIs">
|
||||
<strong>{{ username }}</strong>
|
||||
</i18n-t>
|
||||
</template>
|
||||
</p>
|
||||
|
||||
<table class="table" v-if="tokens.length > 0">
|
||||
<tr>
|
||||
<th>{{ $t('misc.id') }}</th>
|
||||
<th>{{ $t('misc.created') }}</th>
|
||||
<th class="has-text-right">{{ $t('misc.actions') }}</th>
|
||||
</tr>
|
||||
<tr v-for="tk in tokens" :key="tk.id">
|
||||
<td>{{ tk.id }}</td>
|
||||
<td>{{ formatDateShort(tk.created) }}</td>
|
||||
<td class="has-text-right">
|
||||
<x-button type="secondary" @click="deleteToken(tk)">
|
||||
{{ $t('misc.delete') }}
|
||||
</x-button>
|
||||
</td>
|
||||
</tr>
|
||||
</table>
|
||||
|
||||
<Message v-if="newToken" class="mb-4">
|
||||
{{ $t('user.settings.caldav.tokenCreated', {token: newToken.token}) }}<br/>
|
||||
{{ $t('user.settings.caldav.wontSeeItAgain') }}
|
||||
</Message>
|
||||
|
||||
<x-button icon="plus" class="mb-4" @click="createToken" :loading="service.loading">
|
||||
{{ $t('user.settings.caldav.createToken') }}
|
||||
</x-button>
|
||||
|
||||
<p>
|
||||
<BaseButton :href="CALDAV_DOCS" rel="noreferrer noopener nofollow" target="_blank">
|
||||
{{ $t('user.settings.caldav.more') }}
|
||||
</a>
|
||||
</BaseButton>
|
||||
</p>
|
||||
</card>
|
||||
</template>
|
||||
|
||||
<script>
|
||||
<script lang="ts" setup>
|
||||
konrad marked this conversation as resolved
Outdated
dpschen
commented
use ts use ts
|
||||
import copy from 'copy-to-clipboard'
|
||||
import {mapState} from 'vuex'
|
||||
import {CALDAV_DOCS} from '@/urls'
|
||||
import {computed, ref} from 'vue'
|
||||
import {useI18n} from 'vue-i18n'
|
||||
import {useStore} from 'vuex'
|
||||
|
||||
export default {
|
||||
name: 'user-settings-caldav',
|
||||
data() {
|
||||
return {
|
||||
caldavDocsUrl: CALDAV_DOCS,
|
||||
}
|
||||
},
|
||||
mounted() {
|
||||
this.setTitle(`${this.$t('user.settings.caldav.title')} - ${this.$t('user.settings.title')}`)
|
||||
},
|
||||
computed: {
|
||||
caldavUrl() {
|
||||
return `${this.$store.getters['config/apiBase']}/dav/principals/${this.userInfo.username}/`
|
||||
},
|
||||
...mapState('config', ['caldavEnabled']),
|
||||
...mapState({
|
||||
userInfo: state => state.auth.info,
|
||||
}),
|
||||
},
|
||||
methods: {
|
||||
copy,
|
||||
},
|
||||
import {CALDAV_DOCS} from '@/urls'
|
||||
import {useTitle} from '@/composables/useTitle'
|
||||
import {success} from '@/message'
|
||||
import BaseButton from '@/components/base/BaseButton.vue'
|
||||
konrad marked this conversation as resolved
Outdated
dpschen
commented
use use `@`
|
||||
import Message from '@/components/misc/message.vue'
|
||||
import CaldavTokenService from '@/services/caldavToken'
|
||||
import CaldavTokenModel from '@/models/caldavToken'
|
||||
|
||||
dpschen marked this conversation as resolved
Outdated
dpschen
commented
=> isolated abstraction - Move service in `useTokens` function.
- Make service a `shallowReactive`. => Will be destroyed when component is unmounted.
- return a new computed `serviceIsLoading` from `useTokens` that computes the isLoading property.
=> isolated abstraction
konrad
commented
Wouldn't I need to do this for all other functions as well? ( Wouldn't I need to do this for all other functions as well? (`deleteToken` and `createToken`) How would I handle loading state from all of them? I'd like to avoid having a bunch of loading variables for each of these functions.
dpschen
commented
Solved differently in #1307/ Solved differently in https://kolaente.dev/vikunja/frontend/pulls/1307/
|
||||
const service = new CaldavTokenService()
|
||||
|
||||
dpschen marked this conversation as resolved
Outdated
dpschen
commented
We should call this function then Unsure: I think
Trying to learn: Not sure what the go-to way for ts is in such a case. We should call this function then `useTokens`.
----
Unsure:
I think `: ref<CaldavTokenModel[]>` is now unnecessary since the type of the return was already defined by the line:
```ts
const tokens = ref<CaldavTokenModel[]>([])
```
Trying to learn: Not sure what the go-to way for ts is in such a case.
konrad
commented
Renamed the function. I'm not quite sure about the other one, since the Renamed the function.
I'm not quite sure about the other one, since the `const token` now does not have a type declaration, only the function does. I think that should be fine?
dpschen
commented
I cannot follow you: I cannot follow you:
In [this line](https://kolaente.dev/vikunja/frontend/src/commit/ce479b40769d4a116694b39a4d552e561a84b8d9/src/views/user/settings/Caldav.vue#L85) the type of the returned value is defined.
dpschen
commented
Regardless this is not important anymore, since the code changed. Regardless this is not important anymore, since the code changed.
|
||||
function useTokens(): ref<CaldavTokenModel[]> {
|
||||
const tokens = ref<CaldavTokenModel[]>([])
|
||||
dpschen marked this conversation as resolved
Outdated
dpschen
commented
A composable should usually not return a promise. See https://antfu.me/posts/async-with-composition-api Better: create a new function inside the composable that does the async action and call it without await.
EDIT: A composable should *usually* not return a promise. See https://antfu.me/posts/async-with-composition-api
(only exception as far as I know seems to be when you use the `<suspense>` component)
Better: create a new function inside the composable that does the async action and call it without await.
Something like:
```ts
function init() {
tokens.value = await service.getAll()
}
// and then call it either directly
init()
// or return it so the user of the composable can call it when he wants (probably not what you want here).
return {
init,
[...]
}
```
**EDIT:**
There is also: [useAsyncState](https://vueuse.org/core/useasyncstate/)
konrad
commented
But afaik I need an Using the example you sent gives me an error about that. But afaik I need an `async` function when using `await`? Or use `.then()`?
Using the example you sent gives me an error about that.
konrad
commented
Okay so this does not seem to work at all right now. Okay so this does not seem to work at all right now.
konrad
commented
I think I got it working in I think I got it working in ef69fd28b299ad965a9f68e467cee5d166df9e1d but I'm not sure if that's a good way to do it. I think I'm returning the ref before it has data in it, that only gets populated once the callback in `.then` is done and puts it in `tokens.value`. Not sure if that's a good way to do it?
dpschen
commented
Continuing this is #1307/ Continuing this is https://kolaente.dev/vikunja/frontend/pulls/1307/
|
||||
service.getAll()
|
||||
.then((t: CaldavTokenModel[]) => {
|
||||
tokens.value = t
|
||||
})
|
||||
return tokens
|
||||
konrad marked this conversation as resolved
Outdated
dpschen
commented
Abstract with new Abstract with new `useToken` composable (can be local) that returns the tokens ref.
Then the getAll call can also be async (an async init function that is called without await).
konrad
commented
How do I properly type hint this? I tried this:
but that does not seem to work. How do I properly type hint this? I tried this:
```ts
async function useToken(): Promise<ref<CaldavTokenModel[]>> {
const tokens = ref<CaldavTokenModel[]>([])
tokens.value = await service.getAll()
return tokens
}
```
but that does not seem to work.
dpschen
commented
What errors do you get? I guess not all props are defined (because I got those kind of errors all the time with the models). For this the field declarations might help What errors do you get?
I guess not all props are defined (because I got those kind of errors all the time with the models).
For this the [field declarations](https://kolaente.dev/vikunja/frontend/pulls/1186#issuecomment-21746) might help
konrad
commented
Seems like
did the trick. Seems like
```js
async function useToken(): ref<CaldavTokenModel[]> {
```
did the trick.
dpschen
commented
Not sure what problem you had here, but I realize now that it might have also been because of the usage of await in a composable. > but that does not seem to work.
Not sure what problem you had here, but I realize now that it might have also been because of the usage of await in a composable.
|
||||
}
|
||||
|
||||
const tokens = useTokens()
|
||||
|
||||
const store = useStore()
|
||||
const {t} = useI18n()
|
||||
|
||||
useTitle(() => `${t('user.settings.caldav.title')} - ${t('user.settings.title')}`)
|
||||
konrad marked this conversation as resolved
Outdated
dpschen
commented
`createToken` should be an async function (also defined in `useToken` composable).
konrad
commented
Done. Done.
dpschen
commented
Use => Makes it more clear that these are the "methods" of vue and makes them stand out more against computeds and refs. Use `function` statement.
=> Makes it more clear that these are the "methods" of vue and makes them stand out more against computeds and refs.
|
||||
|
||||
const caldavUrl = computed(() => `${store.getters['config/apiBase']}/dav/principals/${store.state.auth.info.username}/`)
|
||||
const caldavEnabled = computed(() => store.state.config.caldavEnabled)
|
||||
const isLocalUser = computed(() => store.state.auth.info?.isLocalUser)
|
||||
const username = computed(() => store.state.auth.info?.username)
|
||||
|
||||
konrad marked this conversation as resolved
Outdated
dpschen
commented
Would there be any negative side effect if you would assign to
If yes (picky): I prefere to give api returns a name that makes clear what they contain, e.g. Would there be any negative side effect if you would assign to `newToken.value` directly?
```ts
newToken.value = await service.create({})
tokens.value.push(newToken.value)
```
If yes (picky): I prefere to give api returns a name that makes clear what they contain, e.g. `token`.
konrad
commented
That seems to work fine, I've changed it accordingly. That seems to work fine, I've changed it accordingly.
|
||||
const newToken = ref(null)
|
||||
|
||||
konrad marked this conversation as resolved
Outdated
dpschen
commented
Should be async function Should be async function
konrad
commented
Done. Done.
dpschen
commented
Picky: Use function statement. Picky: Use function statement.
|
||||
async function createToken() {
|
||||
newToken.value = await service.create({})
|
||||
tokens.value.push(newToken.value)
|
||||
}
|
||||
dpschen marked this conversation as resolved
Outdated
dpschen
commented
Use filter Use filter
konrad
commented
Done. Done.
dpschen
commented
Now it's With something like:
It might work =) On the other hand: Now it's `findIndex` but this seems to have other problems, because of the `@ts-ignore`.
With something like:
```js
const filteredTokenEntries = Objects.entries(token.value).filter([...])
token.value = Object.fromEntries(filteredTokenEntries)
```
It might work =)
-----
On the other hand:
Maybe with that the original solution was nicer / smarter.
I somehow thought tokens is an array and not an object… that's why I proposed using `filter` in the first place.
konrad
commented
Isn't > I somehow thought tokens is an array and not an object… that's why I proposed using filter in the first place.
`tokens` is an array, not an object. But I need the index of the entry I want to feed into `splice` to remove the entry. AFAIK I can't use `find` because that would only give me the object in the array, not the index?
Isn't `findIndex` pretty much the same as a `for(const i in ..)` with a `break`?
dpschen
commented
It is if you add a condition for the case of It is if you add a condition for the case of `-1` (which you did now)
|
||||
|
||||
konrad marked this conversation as resolved
Outdated
dpschen
commented
Why the Why the `@ts-ignore`? => Add comment if you need it
konrad
commented
I just took another look and think I figured out how to fix the types on the I just took another look and think I figured out how to fix the types on the `useToken` function so the `@ts-ignore` isn't required anymore. Should be fine now.
dpschen
commented
Move Move `success` after all state changes
konrad
commented
Done. Done.
|
||||
async function deleteToken(token: CaldavTokenModel) {
|
||||
konrad marked this conversation as resolved
Outdated
dpschen
commented
I think you need to check for I think you need to check for `-1`.
|
||||
const r = await service.delete(token)
|
||||
konrad marked this conversation as resolved
Outdated
dpschen
commented
Why the Why the `@ts-ignore`? => Add comment if you need it
|
||||
const i = tokens.value.findIndex(v => v.id === token.id)
|
||||
if (i === -1) {
|
||||
return
|
||||
}
|
||||
tokens.value.splice(i, 1)
|
||||
success(r)
|
||||
}
|
||||
</script>
|
||||
|
|
casing