vikunja
/
frontend
Archived
6
39
Fork 49
Code Issues 1 Pull Requests 4 Projects Releases 26 Activity

feat: manage caldav tokens [addition] #1307

Merged
konrad merged 24 commits from dpschen/frontend:feature/caldav-tokens into main 2022-04-02 15:51:44 +00:00
Conversation 4 Commits 24 Files Changed 5 +138 -38
dpschen commented 2022-01-05 15:52:01 +00:00
Member
Copy Link

Based on #1186.
Didn't want to push to the original branch without sharing first.

I also do not currently know how I could test this.

TODO:

  • check casing CalDAVin whole request
Based on https://kolaente.dev/vikunja/frontend/pulls/1186. Didn't want to push to the original branch without sharing first. I also do not currently know how I could test this. ------------------ ## TODO: - [x] check casing `CalDAV`in whole request
dpschen added 21 commits 2022-01-05 15:52:02 +00:00
continuous-integration/drone/pr Build is failing Details
79dd9ac30e
chore: clarify token is required for non-local users
continuous-integration/drone/pr Build is failing Details
e2fc5e940d
chore: make server functions async
continuous-integration/drone/pr Build is failing Details
e4af8a950a
chore: extract getting all tokens into a composable
continuous-integration/drone/pr Build is failing Details
2405f958b1
fix: lint
continuous-integration/drone/pr Build is passing Details
5106be028c
chore: check for no results
continuous-integration/drone/pr Build is passing Details
2d71897581
chore: use function statements everywhere
continuous-integration/drone/pr Build is passing Details
7d43bbad4e
chore: fix CalDAV casing
continuous-integration/drone/pr Build is failing Details
1c9f2b53ec
chore: use h5
continuous-integration/drone/pr Build is failing Details
6063c03c2f
chore: rename to useTokens
continuous-integration/drone/pr Build is passing Details
e08c3a3594
chore: directly use newToken.value
continuous-integration/drone/pr Build is passing Details
ef69fd28b2
chore: use .then instead of await
frederick commented 2022-01-05 15:56:36 +00:00
Member
Copy Link

Hi dpschen!

Thank you for creating a PR!

I've deployed the changes of this PR on a preview environment under this URL: https://1307-featurecaldav-tokens--vikunja-frontend-preview.netlify.app

You can use this url to view the changes live and test them out.
You will need to manually connect this to an api running somehwere. The easiest to use is https://try.vikunja.io/.

Have a nice day!

Beep boop, I'm a bot.

Hi dpschen! Thank you for creating a PR! I've deployed the changes of this PR on a preview environment under this URL: https://1307-featurecaldav-tokens--vikunja-frontend-preview.netlify.app You can use this url to view the changes live and test them out. You will need to manually connect this to an api running somehwere. The easiest to use is https://try.vikunja.io/. Have a nice day! > Beep boop, I'm a bot.
dpschen reviewed 2022-01-05 16:28:52 +00:00
src/models/abstractModel.ts Outdated
dpschen commented 2022-01-05 16:09:45 +00:00
Author
Member

Using ts here improves the DX a lot.

Using ts here improves the DX a lot.
dpschen commented 2022-01-05 16:12:08 +00:00
Author
Member

I also started converting the abstractService to ts. But I had too many issues with it so removed it again.

What I currently have:

import axios, {Method} from 'axios'
import {objectToSnakeCase} from '@/helpers/case'
import {getToken} from '@/helpers/auth'
import AbstractModel from '@/models/abstractModel'

interface Paths {
	create : string
	get : string
	getAll : string
	update : string
	delete : string
}

export default class AbstractService {

	/////////////////////////////
	// Initial variable definitions
	///////////////////////////

	http
	loading = false
	uploadProgress = 0
	paths: Paths = {
		create: '',
		get: '',
		getAll: '',
		update: '',
		delete: '',
	}
	// This contains the total number of pages and the number of results for the current page
	totalPages = 0
	resultCount = 0

	/////////////
	// Service init
	///////////

	/**
	 * The abstract constructor.
	 * @param [paths] An object with all paths.
	 */
	constructor(paths : Paths) {
		this.http = axios.create({
			baseURL: window.API_URL,
			headers: { 'Content-Type': 'application/json' },
		})

		// Set the interceptors to process every request
		const self = this
		this.http.interceptors.request.use((config) => {
			switch (config.method) {
				case 'post':
					if (this.useUpdateInterceptor()) {
						config.data = self.beforeUpdate(config.data)
						config.data = objectToSnakeCase(config.data)
					}
					break
				case 'put':
					if (this.useCreateInterceptor()) {
						config.data = self.beforeCreate(config.data)
						config.data = objectToSnakeCase(config.data)
					}
					break
				case 'delete':
					if (this.useDeleteInterceptor()) {
						config.data = self.beforeDelete(config.data)
						config.data = objectToSnakeCase(config.data)
					}
					break
			}
			return config
		})

		// Set the default auth header if we have a token
		const token = getToken()
		if (token !== null) {
			this.http.defaults.headers.common['Authorization'] = `Bearer ${token}`
		}

		if (paths) {
			Object.assign(this.paths, paths)
		}
	}

	/**
	 * Whether or not to use the create interceptor which processes a request payload into json
	 */
	useCreateInterceptor(): boolean {
		return true
	}

	/**
	 * Whether or not to use the update interceptor which processes a request payload into json
	 */
	useUpdateInterceptor(): boolean {
		return true
	}

	/**
	 * Whether or not to use the delete interceptor which processes a request payload into json
	 */
	useDeleteInterceptor(): boolean {
		return true
	}

	/////////////////
	// Helper functions
	///////////////

	/**
	 * Returns an object with all route parameters and their values.
	 */
	getRouteReplacements(route : string, parameters = {}) {
		const replace$$1 = {}
		let pattern = this.getRouteParameterPattern()
		pattern = new RegExp(pattern instanceof RegExp ? pattern.source : pattern, 'g')

		for (let parameter; (parameter = pattern.exec(route)) !== null;) {
			replace$$1[parameter[0]] = parameters[parameter[1]]
		}

		return replace$$1
	}

	/**
	 * Holds the replacement pattern for url paths, can be overwritten by implementations.
	 */
	getRouteParameterPattern(): RegExp {
		return /{([^}]+)}/
	}

	/**
	 * Returns a fully-ready-ready-to-make-a-request-to route with replaced parameters.
	 */
	getReplacedRoute(path : string, pathparams : Object) : string {
		const replacements = this.getRouteReplacements(path, pathparams)
		return Object.entries(replacements).reduce(
			(result, [parameter, value]) => result.replace(parameter, value as string),
			path,
		)
	}

	/**
	 * setLoading is a method which sets the loading variable to true, after a timeout of 100ms.
	 * It has the timeout to prevent the loading indicator from showing for only a blink of an eye in the
	 * case the api returns a response in < 100ms.
	 * But because the timeout is created using setTimeout, it will still trigger even if the request is
	 * already finished, so we return a method to call in that case.
	 */
	setLoading(): Function {
		const timeout = setTimeout(() => {
			this.loading = true
		}, 100)
		return () => {
			clearTimeout(timeout)
			this.loading = false
		}
	}

	//////////////////
	// Default factories
	// It is possible to specify a factory for each type of request.
	// This makes it possible to have different models returned from different routes.
	// Specific factories for each request are completly optional, if these are not specified, the defautl factory is used.
	////////////////

	/**
	 * The modelFactory returns an model from an object.
	 * This one here is the default one, usually the service definitions for a model will override this.
	 */
	modelFactory(data : Object) {
		return new AbstractModel(data)
	}

	/**
	 * This is the model factory for get requests.
	 */
	modelGetFactory(data : Object) {
		return this.modelFactory(data)
	}

	/**
	 * This is the model factory for get all requests.
	 */
	modelGetAllFactory(data : Object) {
		return this.modelFactory(data)
	}

	/**
	 * This is the model factory for create requests.
	 */
	modelCreateFactory(data : Object) {
		return this.modelFactory(data)
	}

	/**
	 * This is the model factory for update requests.
	 */
	modelUpdateFactory(data : Object) {
		return this.modelFactory(data)
	}

	//////////////
	// Preprocessors
	////////////

	/**
	 * Default preprocessor for get requests
	 */
	beforeGet(model : AbstractModel) {
		return model
	}

	/**
	 * Default preprocessor for create requests
	 */
	beforeCreate(model : AbstractModel) {
		return model
	}

	/**
	 * Default preprocessor for update requests
	 */
	beforeUpdate(model : AbstractModel) {
		return model
	}

	/**
	 * Default preprocessor for delete requests
	 */
	beforeDelete(model : AbstractModel) {
		return model
	}

	///////////////
	// Global actions
	/////////////

	/**
	 * Performs a get request to the url specified before.
	 * @param model The model to use. The request path is built using the values from the model.
	 * @param params Optional query parameters
	 */
	get(model : AbstractModel, params = {}) {
		if (this.paths.get === '') {
			throw new Error('This model is not able to get data.')
		}

		return this.getM(this.paths.get, model, params)
	}

	/**
	 * This is a more abstract implementation which only does a get request.
	 * Services which need more flexibility can use this.
	 */
	async getM(url : string, model = new AbstractModel({}), params = {}) {
		const cancel = this.setLoading()

		model = this.beforeGet(model)
		const finalUrl = this.getReplacedRoute(url, model)

		try {
			const response = await this.http.get(finalUrl, {params})
			const result = this.modelGetFactory(response.data)
			result.maxRight = Number(response.headers['x-max-right'])
			return result
		} finally {
			cancel()
		}
	}

	async getBlobUrl(url : string, method = 'GET' as Method, data = {}) {
		const response = await this.http({
			url,
			method,
			responseType: 'blob',
			data,
		})
		return window.URL.createObjectURL(new Blob([response.data]))
	}

	/**
	 * Performs a get request to the url specified before.
	 * The difference between this and get() is this one is used to get a bunch of data (an array), not just a single object.
	 * @param model The model to use. The request path is built using the values from the model.
	 * @param params Optional query parameters
	 * @param page The page to get
	 */
	async getAll(model = new AbstractModel({}), params = {}, page = 1) {
		if (this.paths.getAll === '') {
			throw new Error('This model is not able to get data.')
		}

		params.page = page

		const cancel = this.setLoading()
		model = this.beforeGet(model)
		const finalUrl = this.getReplacedRoute(this.paths.getAll, model)

		try {
			const response = await this.http.get(finalUrl, {params: params})
			this.resultCount = Number(response.headers['x-pagination-result-count'])
			this.totalPages = Number(response.headers['x-pagination-total-pages'])

			if (response.data === null) {
				return []
			}

			if (Array.isArray(response.data)) {
				return response.data.map(entry => this.modelGetAllFactory(entry))
			}
			return this.modelGetAllFactory(response.data)
		} finally {
			cancel()
		}
	}

	/**
	 * Performs a put request to the url specified before
	 * @returns {Promise<any | never>}
	 */
	async create(model : AbstractModel) {
		if (this.paths.create === '') {
			throw new Error('This model is not able to create data.')
		}

		const cancel = this.setLoading()
		const finalUrl = this.getReplacedRoute(this.paths.create, model)

		try {
			const response = await this.http.put(finalUrl, model)
			const result = this.modelCreateFactory(response.data)
			if (typeof model.maxRight !== 'undefined') {
				result.maxRight = model.maxRight
			}
			return result
		} finally {
			cancel()
		}
	}

	/**
	 * An abstract implementation to send post requests.
	 * Services can use this to implement functions to do post requests other than using the update method.
	 */
	async post(url : string, model : AbstractModel) {
		const cancel = this.setLoading()

		try {
			const response = await this.http.post(url, model)
			const result = this.modelUpdateFactory(response.data)
			if (typeof model.maxRight !== 'undefined') {
				result.maxRight = model.maxRight
			}
			return result
		} finally {
			cancel()
		}
	}

	/**
	 * Performs a post request to the update url
	 */
	update(model : AbstractModel) {
		if (this.paths.update === '') {
			throw new Error('This model is not able to update data.')
		}

		const finalUrl = this.getReplacedRoute(this.paths.update, model)
		return this.post(finalUrl, model)
	}

	/**
	 * Performs a delete request to the update url
	 */
	async delete(model : AbstractModel) {
		if (this.paths.delete === '') {
			throw new Error('This model is not able to delete data.')
		}

		const cancel = this.setLoading()
		const finalUrl = this.getReplacedRoute(this.paths.delete, model)

		try {
			const {data} = await this.http.delete(finalUrl, model)
			return data
		} finally {
			cancel()
		}
	}

	/**
	 * Uploads a file to a url.
	 * @param url
	 * @param file
	 * @param fieldName The name of the field the file is uploaded to.
	 */
	uploadFile(url : string, file, fieldName : string) {
		return this.uploadBlob(url, new Blob([file]), fieldName, file.name)
	}

	/**
	 * Uploads a blob to a url.
	 */
	uploadBlob(url : string, blob, fieldName, filename : string) {
		const data = new FormData()
		data.append(fieldName, blob, filename)
		return this.uploadFormData(url, data)
	}

	/**
	 * Uploads a form data object.
	 */
	async uploadFormData(url : string, formData) {
		const cancel = this.setLoading()
		try {
			const response = await this.http.put(
				url,
				formData,
				{
					headers: {
						'Content-Type':
							'multipart/form-data; boundary=' + formData._boundary,
					},
					onUploadProgress: progressEvent => {
						this.uploadProgress = Math.round((progressEvent.loaded * 100) / progressEvent.total)
					},
				},
			)
			return this.modelCreateFactory(response.data)
		} finally {
			this.uploadProgress = 0
			cancel()
		}
	}
}
I also started converting the abstractService to ts. But I had too many issues with it so removed it again. What I currently have: ```ts import axios, {Method} from 'axios' import {objectToSnakeCase} from '@/helpers/case' import {getToken} from '@/helpers/auth' import AbstractModel from '@/models/abstractModel' interface Paths { create : string get : string getAll : string update : string delete : string } export default class AbstractService { ///////////////////////////// // Initial variable definitions /////////////////////////// http loading = false uploadProgress = 0 paths: Paths = { create: '', get: '', getAll: '', update: '', delete: '', } // This contains the total number of pages and the number of results for the current page totalPages = 0 resultCount = 0 ///////////// // Service init /////////// /** * The abstract constructor. * @param [paths] An object with all paths. */ constructor(paths : Paths) { this.http = axios.create({ baseURL: window.API_URL, headers: { 'Content-Type': 'application/json' }, }) // Set the interceptors to process every request const self = this this.http.interceptors.request.use((config) => { switch (config.method) { case 'post': if (this.useUpdateInterceptor()) { config.data = self.beforeUpdate(config.data) config.data = objectToSnakeCase(config.data) } break case 'put': if (this.useCreateInterceptor()) { config.data = self.beforeCreate(config.data) config.data = objectToSnakeCase(config.data) } break case 'delete': if (this.useDeleteInterceptor()) { config.data = self.beforeDelete(config.data) config.data = objectToSnakeCase(config.data) } break } return config }) // Set the default auth header if we have a token const token = getToken() if (token !== null) { this.http.defaults.headers.common['Authorization'] = `Bearer ${token}` } if (paths) { Object.assign(this.paths, paths) } } /** * Whether or not to use the create interceptor which processes a request payload into json */ useCreateInterceptor(): boolean { return true } /** * Whether or not to use the update interceptor which processes a request payload into json */ useUpdateInterceptor(): boolean { return true } /** * Whether or not to use the delete interceptor which processes a request payload into json */ useDeleteInterceptor(): boolean { return true } ///////////////// // Helper functions /////////////// /** * Returns an object with all route parameters and their values. */ getRouteReplacements(route : string, parameters = {}) { const replace$$1 = {} let pattern = this.getRouteParameterPattern() pattern = new RegExp(pattern instanceof RegExp ? pattern.source : pattern, 'g') for (let parameter; (parameter = pattern.exec(route)) !== null;) { replace$$1[parameter[0]] = parameters[parameter[1]] } return replace$$1 } /** * Holds the replacement pattern for url paths, can be overwritten by implementations. */ getRouteParameterPattern(): RegExp { return /{([^}]+)}/ } /** * Returns a fully-ready-ready-to-make-a-request-to route with replaced parameters. */ getReplacedRoute(path : string, pathparams : Object) : string { const replacements = this.getRouteReplacements(path, pathparams) return Object.entries(replacements).reduce( (result, [parameter, value]) => result.replace(parameter, value as string), path, ) } /** * setLoading is a method which sets the loading variable to true, after a timeout of 100ms. * It has the timeout to prevent the loading indicator from showing for only a blink of an eye in the * case the api returns a response in < 100ms. * But because the timeout is created using setTimeout, it will still trigger even if the request is * already finished, so we return a method to call in that case. */ setLoading(): Function { const timeout = setTimeout(() => { this.loading = true }, 100) return () => { clearTimeout(timeout) this.loading = false } } ////////////////// // Default factories // It is possible to specify a factory for each type of request. // This makes it possible to have different models returned from different routes. // Specific factories for each request are completly optional, if these are not specified, the defautl factory is used. //////////////// /** * The modelFactory returns an model from an object. * This one here is the default one, usually the service definitions for a model will override this. */ modelFactory(data : Object) { return new AbstractModel(data) } /** * This is the model factory for get requests. */ modelGetFactory(data : Object) { return this.modelFactory(data) } /** * This is the model factory for get all requests. */ modelGetAllFactory(data : Object) { return this.modelFactory(data) } /** * This is the model factory for create requests. */ modelCreateFactory(data : Object) { return this.modelFactory(data) } /** * This is the model factory for update requests. */ modelUpdateFactory(data : Object) { return this.modelFactory(data) } ////////////// // Preprocessors //////////// /** * Default preprocessor for get requests */ beforeGet(model : AbstractModel) { return model } /** * Default preprocessor for create requests */ beforeCreate(model : AbstractModel) { return model } /** * Default preprocessor for update requests */ beforeUpdate(model : AbstractModel) { return model } /** * Default preprocessor for delete requests */ beforeDelete(model : AbstractModel) { return model } /////////////// // Global actions ///////////// /** * Performs a get request to the url specified before. * @param model The model to use. The request path is built using the values from the model. * @param params Optional query parameters */ get(model : AbstractModel, params = {}) { if (this.paths.get === '') { throw new Error('This model is not able to get data.') } return this.getM(this.paths.get, model, params) } /** * This is a more abstract implementation which only does a get request. * Services which need more flexibility can use this. */ async getM(url : string, model = new AbstractModel({}), params = {}) { const cancel = this.setLoading() model = this.beforeGet(model) const finalUrl = this.getReplacedRoute(url, model) try { const response = await this.http.get(finalUrl, {params}) const result = this.modelGetFactory(response.data) result.maxRight = Number(response.headers['x-max-right']) return result } finally { cancel() } } async getBlobUrl(url : string, method = 'GET' as Method, data = {}) { const response = await this.http({ url, method, responseType: 'blob', data, }) return window.URL.createObjectURL(new Blob([response.data])) } /** * Performs a get request to the url specified before. * The difference between this and get() is this one is used to get a bunch of data (an array), not just a single object. * @param model The model to use. The request path is built using the values from the model. * @param params Optional query parameters * @param page The page to get */ async getAll(model = new AbstractModel({}), params = {}, page = 1) { if (this.paths.getAll === '') { throw new Error('This model is not able to get data.') } params.page = page const cancel = this.setLoading() model = this.beforeGet(model) const finalUrl = this.getReplacedRoute(this.paths.getAll, model) try { const response = await this.http.get(finalUrl, {params: params}) this.resultCount = Number(response.headers['x-pagination-result-count']) this.totalPages = Number(response.headers['x-pagination-total-pages']) if (response.data === null) { return [] } if (Array.isArray(response.data)) { return response.data.map(entry => this.modelGetAllFactory(entry)) } return this.modelGetAllFactory(response.data) } finally { cancel() } } /** * Performs a put request to the url specified before * @returns {Promise<any | never>} */ async create(model : AbstractModel) { if (this.paths.create === '') { throw new Error('This model is not able to create data.') } const cancel = this.setLoading() const finalUrl = this.getReplacedRoute(this.paths.create, model) try { const response = await this.http.put(finalUrl, model) const result = this.modelCreateFactory(response.data) if (typeof model.maxRight !== 'undefined') { result.maxRight = model.maxRight } return result } finally { cancel() } } /** * An abstract implementation to send post requests. * Services can use this to implement functions to do post requests other than using the update method. */ async post(url : string, model : AbstractModel) { const cancel = this.setLoading() try { const response = await this.http.post(url, model) const result = this.modelUpdateFactory(response.data) if (typeof model.maxRight !== 'undefined') { result.maxRight = model.maxRight } return result } finally { cancel() } } /** * Performs a post request to the update url */ update(model : AbstractModel) { if (this.paths.update === '') { throw new Error('This model is not able to update data.') } const finalUrl = this.getReplacedRoute(this.paths.update, model) return this.post(finalUrl, model) } /** * Performs a delete request to the update url */ async delete(model : AbstractModel) { if (this.paths.delete === '') { throw new Error('This model is not able to delete data.') } const cancel = this.setLoading() const finalUrl = this.getReplacedRoute(this.paths.delete, model) try { const {data} = await this.http.delete(finalUrl, model) return data } finally { cancel() } } /** * Uploads a file to a url. * @param url * @param file * @param fieldName The name of the field the file is uploaded to. */ uploadFile(url : string, file, fieldName : string) { return this.uploadBlob(url, new Blob([file]), fieldName, file.name) } /** * Uploads a blob to a url. */ uploadBlob(url : string, blob, fieldName, filename : string) { const data = new FormData() data.append(fieldName, blob, filename) return this.uploadFormData(url, data) } /** * Uploads a form data object. */ async uploadFormData(url : string, formData) { const cancel = this.setLoading() try { const response = await this.http.put( url, formData, { headers: { 'Content-Type': 'multipart/form-data; boundary=' + formData._boundary, }, onUploadProgress: progressEvent => { this.uploadProgress = Math.round((progressEvent.loaded * 100) / progressEvent.total) }, }, ) return this.modelCreateFactory(response.data) } finally { this.uploadProgress = 0 cancel() } } } ```
dpschen commented 2022-01-05 16:18:27 +00:00
Author
Member

There is a lot of stuff where I'm not sure how to do this correctly. To give an example:

  • When you create a model the type of the model depends on the service, but all of these classes extend AbstractModel.
  • Right now I simply added AbstractModel as type, but that obviously doesn't work.
There is a lot of stuff where I'm not sure how to do this correctly. To give an example: - When you create a model the type of the model depends on the service, but all of these classes extend AbstractModel. - Right now I simply added `AbstractModel` as type, but that obviously doesn't work.
dpschen commented 2022-02-13 21:12:29 +00:00
Author
Member

I had some progress with this, but was not able to succeed yet.
Since this has nothing to do with the pull request I'm resolving this.

I had some progress with this, but was not able to succeed yet. Since this has nothing to do with the pull request I'm resolving this.
dpschen marked this conversation as resolved
src/models/caldavToken.ts Outdated
@ -0,0 +1,14 @@
import AbstractModel from './abstractModel'
export default class CaldavTokenModel extends AbstractModel {
id = 0
dpschen commented 2022-01-05 16:21:57 +00:00
Author
Member

Since AbstractModel already defines a defaults function we don't have to implement one here.
For better ts support we can define fields

Since `AbstractModel` already defines a `defaults` function we don't have to implement one here. For better ts support we can define [fields](https://www.typescriptlang.org/docs/handbook/2/classes.html#fields)
dpschen marked this conversation as resolved
src/services/caldavToken.js Outdated
@ -0,0 +2,4 @@
import CaldavTokenModel from '../models/caldavToken'
import AbstractService from './abstractService'
export default class CaldavTokenService extends AbstractService {
dpschen commented 2022-01-05 16:09:04 +00:00
Author
Member

I think we should also use the correct casing here (file and class name). What do you think?

I think we should also use the correct casing here (file and class name). What do you think?
konrad commented 2022-01-08 12:55:58 +00:00
Owner

Sounds like a good idea.

Sounds like a good idea.
src/views/user/settings/Caldav.vue Outdated
@ -19,2 +59,3 @@
<p>
<a href="https://vikunja.io/docs/caldav/" rel="noreferrer noopener nofollow" target="_blank">
<BaseButton :href="CALDAV_DOCS" rel="noreferrer noopener nofollow" target="_blank">
dpschen commented 2022-01-05 16:05:20 +00:00
Author
Member

In case <BaseButton> is a link (not a router-link):

Do you think we always want rel="noreferrer noopener nofollow" to be set?
The current default rel for links is only noopener (because copied from my own code).

In case `<BaseButton>` is a link (**not** a `router-link`): Do you think we always want `rel="noreferrer noopener nofollow"` to be set? The current default `rel` for links is only `noopener` (because copied from my own code).
konrad commented 2022-01-08 12:56:44 +00:00
Owner

Yeah, that sounds like a good idea.

Yeah, that sounds like a good idea.
dpschen commented 2022-02-06 21:35:35 +00:00
Author
Member

Solved in #1491

Solved in https://kolaente.dev/vikunja/frontend/pulls/1491
dpschen marked this conversation as resolved
src/views/user/settings/Caldav.vue Outdated
@ -31,0 +79,4 @@
import CaldavTokenService from '@/services/caldavToken'
import CaldavTokenModel from '@/models/caldavToken'
const {t} = useI18n()
dpschen commented 2022-01-05 16:01:41 +00:00
Author
Member

I think we should always start the view components with the title composable.
Sometimes data fetching might need to be first of course.

I think we should always start the view components with the title composable. Sometimes data fetching might need to be first of course.
👍 1
dpschen marked this conversation as resolved
src/views/user/settings/Caldav.vue Outdated
@ -31,0 +82,4 @@
const {t} = useI18n()
useTitle(() => `${t('user.settings.caldav.title')} - ${t('user.settings.title')}`)
const service = shallowReactive(new CaldavTokenService())
dpschen commented 2022-01-05 15:57:59 +00:00
Author
Member

I think (not sure) that using some kind of reactive for stuff like this is a good pattern, because this way you can always extract parts in a composable if needed later. Often we want to do this after a component has grown in size because some stuff was added.

I think (not sure) that using some kind of reactive for stuff like this is a good pattern, because this way you can always extract parts in a composable if needed later. Often we want to do this after a component has grown in size because some stuff was added.
src/views/user/settings/Caldav.vue Outdated
@ -31,0 +85,4 @@
const service = shallowReactive(new CaldavTokenService())
const tokens = ref<CaldavTokenModel[]>([])
service.getAll().then((result: CaldavTokenModel[]) => {
dpschen commented 2022-01-05 15:56:21 +00:00
Author
Member

I decided to flatten this again. Sorry for this… I'm also still lerning the composition api =)
Originally I thought that this might be reused in other components.

I decided to flatten this again. Sorry for this… I'm also still lerning the composition api =) Originally I thought that this might be reused in other components.
src/views/user/settings/Caldav.vue Outdated
@ -55,0 +97,4 @@
async function deleteToken(token: CaldavTokenModel) {
const r = await service.delete(token)
tokens.value = tokens.value.filter(({id}) => id !== token.id)
dpschen commented 2022-01-05 16:00:23 +00:00
Author
Member

Not sure if this works aswell.
From vue's perspective filtering and reassigning should have almost the same speed.

But I was not sure if this makes sense, since I am still calling success even if tokens stays the same after filtering because the condition never succeded.

Not sure if this works aswell. From vue's perspective filtering and reassigning should have almost the same speed. But I was not sure if this makes sense, since I am still calling success even if tokens stays the same after filtering because the condition never succeded.
konrad commented 2022-02-20 20:38:55 +00:00
Owner

But wouldn't the condition only never succeed when the token which was deleted is not present in the list? That case should never happen. Because of this, I think showing the success message regardless is not wrong here.

But wouldn't the condition only never succeed when the token which was deleted is not present in the list? That case should never happen. Because of this, I think showing the success message regardless is not wrong here.
👍 1
dpschen marked this conversation as resolved
src/views/user/settings/Caldav.vue Outdated
@ -55,0 +103,4 @@
const store = useStore()
const username = computed(() => store.state.auth.info?.username)
const caldavUrl = computed(() => `${store.getters['config/apiBase']}/dav/principals/${username.value}/`)
dpschen commented 2022-01-05 16:00:44 +00:00
Author
Member

I reused the username computed here

I reused the `username` computed here
👍 1
dpschen marked this conversation as resolved
dpschen requested review from konrad 2022-01-05 20:04:03 +00:00
konrad was assigned by dpschen 2022-01-05 20:04:11 +00:00
dpschen changed title from WIP: feat: manage caldav tokens [addition] to feat: manage caldav tokens [addition] 2022-01-05 20:04:17 +00:00
konrad commented 2022-01-08 12:58:01 +00:00
Owner
Copy Link

Maybe merging #1186 first could be a good idea? Would make this one easier to review at least

Maybe merging #1186 first could be a good idea? Would make this one easier to review at least
dpschen commented 2022-01-09 10:09:00 +00:00
Author
Member
Copy Link

Maybe merging #1186 first could be a good idea? Would make this one easier to review at least

In theory this is the diff

> Maybe merging #1186 first could be a good idea? Would make this one easier to review at least In theory [this is the diff](https://kolaente.dev/vikunja/frontend/compare/feature/caldav-tokens...dpschen/frontend:feature/caldav-tokens)
dpschen force-pushed feature/caldav-tokens from e2e0a6576c to a221b24ccf 2022-02-08 10:33:58 +00:00 Compare
dpschen commented 2022-02-08 10:34:36 +00:00
Author
Member
Copy Link

Fix this: #1489 (comment)

Fix this: https://kolaente.dev/vikunja/frontend/pulls/1489#issuecomment-25512
dpschen reviewed 2022-02-13 20:31:19 +00:00
src/i18n/lang/en.json
@ -103,9 +103,16 @@
"disableSuccess": "Two factor authentication was sucessfully disabled."
},
"caldav": {
dpschen commented 2022-02-13 20:31:19 +00:00
Author
Member

Is it possible to change this translation id?

Is it possible to change this translation id?
konrad commented 2022-02-20 20:40:02 +00:00
Owner

Can you make sure every string is shown correctly after changing it? Maybe not in this PR?

Can you make sure every string is shown correctly after changing it? Maybe not in this PR?
👍 1
dpschen marked this conversation as resolved
dpschen force-pushed feature/caldav-tokens from e4919ca00d to d014174011 2022-02-13 21:03:45 +00:00 Compare
konrad was unassigned by dpschen 2022-02-13 21:16:45 +00:00
konrad was assigned by dpschen 2022-02-13 21:16:52 +00:00
konrad force-pushed feature/caldav-tokens from d014174011 to 8eba71d448 2022-04-02 15:28:49 +00:00 Compare
konrad force-pushed feature/caldav-tokens from 90e47905b1 to 58b0397cec 2022-04-02 15:38:51 +00:00 Compare
konrad approved these changes 2022-04-02 15:51:11 +00:00
konrad left a comment
Owner

Should be fine to merge now with the fixes.

Thanks!

Should be fine to merge now with the fixes. Thanks!
konrad merged commit 0b31cce567 into main 2022-04-02 15:51:44 +00:00
konrad deleted branch feature/caldav-tokens 2022-04-02 15:51:44 +00:00
This repo is archived. You cannot comment on pull requests.
Reviewers
No reviewers
konrad
Labels
Clear labels   
area/internal-code

   
changes requested

   
confirmed

   
dependencies

   
duplicate

This issue or pull request already exists

  
good first issue

   
help wanted

Need some help

  
hosting

   
invalid

Something is wrong

  
kind/bug

Something is not working

  
kind/feature

New feature

  
question

More information is needed

  
wontfix

This won't be fixed

No Label
area/internal-code
changes requested
confirmed
dependencies
duplicate
good first issue
help wanted
hosting
invalid
kind/bug
kind/feature
question
wontfix
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
konrad
3 Participants
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: vikunja/frontend#1307
No description provided.
Delete Branch "dpschen/frontend:feature/caldav-tokens"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?