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

fix: api config domain name contains the current domain instead of the provided one #1581

Merged
konrad merged 1 commits from fix/api-url-error into main 2022-02-20 22:08:00 +00:00
Conversation 1 Commits 1 Files Changed 1 +6 -4
1 changed files with 6 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
src/components/misc/api-config.vue
View File

@ -41,7 +41,7 @@
<script setup lang="ts"> <script setup lang="ts">
import {ref, computed, watch} from 'vue' import {ref, computed, watch} from 'vue'
import { useI18n } from 'vue-i18n' import {useI18n} from 'vue-i18n'
import {parseURL} from 'ufo' import {parseURL} from 'ufo'
import {checkAndSetApiUrl} from '@/helpers/checkAndSetApiUrl' import {checkAndSetApiUrl} from '@/helpers/checkAndSetApiUrl'
@ -61,18 +61,20 @@ const emit = defineEmits(['foundApi'])
const apiUrl = ref(window.API_URL) const apiUrl = ref(window.API_URL)
const configureApi = ref(apiUrl.value === '') const configureApi = ref(apiUrl.value === '')
const apiDomain = computed(() => parseURL(apiUrl.value).host || parseURL(window.location.href).host) // Because we're only using this to parse the hostname, it should be fine to just prefix with http://
konrad marked this conversation as resolved Outdated
dpschen commented 2022-02-20 21:11:09 +00:00
Outdated
Review
Copy Link

How about:

const apiDomain = computed(() => parseURL(apiUrl.value, 'http://').host || parseURL(window.location.href).host)

Shouldn't this work aswell? See my test of the function.

How about: ```js const apiDomain = computed(() => parseURL(apiUrl.value, 'http://').host || parseURL(window.location.href).host) ``` Shouldn't this work aswell? See my test of the function.
konrad commented 2022-02-20 21:38:32 +00:00
Outdated
Review
Copy Link

Ah yes, that makes things a lot simpler. Changed!

Ah yes, that makes things a lot simpler. Changed!
// regardless of whether the url is actually reachable under http.
const apiDomain = computed(() => parseURL(apiUrl.value, 'http://').host || parseURL(window.location.href).host)
watch(() => props.configureOpen, (value) => { watch(() => props.configureOpen, (value) => {
configureApi.value = value configureApi.value = value
}, { immediate: true }) }, {immediate: true})
const {t} = useI18n() const {t} = useI18n()
konrad marked this conversation as resolved Outdated
dpschen commented 2022-02-20 09:23:02 +00:00
Outdated
Review
Copy Link

Can you give an example for this case?

Can you give an example for this case?
konrad commented 2022-02-20 16:24:00 +00:00
Outdated
Review
Copy Link

If you pass in example.com to ufo, it will return undefined because it expects the url to start with a protocol. Since we want to get the host, we need to make sure there is a protocol before we can try to parse it.

If you pass in `example.com` to ufo, it will return `undefined` because it expects the url to start with a protocol. Since we want to get the host, we need to make sure there is a protocol before we can try to parse it.
dpschen commented 2022-02-20 17:39:06 +00:00
Outdated
Review
Copy Link

That's weird, because there documentation says that's possible: https://github.com/unjs/ufo#parseurl (see second example).

That's weird, because there documentation says that's possible: https://github.com/unjs/ufo#parseurl (see second example).
dpschen commented 2022-02-20 17:40:53 +00:00
Outdated
Review
Copy Link

… just saw that the host is part of the pathname in that example.

… just saw that the host is part of the pathname in that example.
dpschen commented 2022-02-20 17:49:13 +00:00
Outdated
Review
Copy Link

Okay I think it's still usable. We have to use the second parameter:

parseURL('foo.com/foo?test=123#token', 'http://')
{
  protocol: 'http:',
  auth: '',
  host: 'foo.com',
  pathname: '/foo',
  search: '?test=123',
  hash: '#token'
}

parseURL('https://foo.com/foo?test=123#token', 'http://')
{
  protocol: 'http:',
  auth: '',
  host: 'foo.com',
  pathname: '/foo',
  search: '?test=123',
  hash: '#token'
}
Okay I think it's still usable. We have to use the second parameter: ``` parseURL('foo.com/foo?test=123#token', 'http://') { protocol: 'http:', auth: '', host: 'foo.com', pathname: '/foo', search: '?test=123', hash: '#token' } parseURL('https://foo.com/foo?test=123#token', 'http://') { protocol: 'http:', auth: '', host: 'foo.com', pathname: '/foo', search: '?test=123', hash: '#token' } ```
👍 1
konrad commented 2022-02-20 19:15:40 +00:00
Outdated
Review
Copy Link

Done!

Done!
const errorMsg = ref('') const errorMsg = ref('')
const successMsg = ref('') const successMsg = ref('')
async function setApiUrl() { async function setApiUrl() {
if (apiUrl.value === '') { if (apiUrl.value === '') {
// Don't try to check and set an empty url // Don't try to check and set an empty url