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

konrad · 2022-02-19T22:09:37Z

konrad commented

2022-02-19 22:09:37 +00:00

No description provided.

konrad requested review from dpschen 2022-02-19 22:09:47 +00:00

frederick commented

2022-02-19 22:14:16 +00:00

Hi konrad!

Thank you for creating a PR!

I've deployed the changes of this PR on a preview environment under this URL: https://1581-fix-api-url-error--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 konrad! Thank you for creating a PR! I've deployed the changes of this PR on a preview environment under this URL: https://1581-fix-api-url-error--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-02-20 09:23:02 +00:00

src/components/misc/api-config.vue Outdated

						
				@ -65,0 +70,4 @@

					if (!apiUrl.value.startsWith('http')) {

						// Because we're only using this to parse the hostname, it should be fine to just prefix with http:// 

						// regardless of whether the url is actually reachable under http.

dpschen commented

2022-02-20 09:23:02 +00:00

Can you give an example for this case?

konrad commented

2022-02-20 16:24:00 +00:00

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

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

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

dpschen commented

2022-02-20 17:49:13 +00:00

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

Done!

konrad marked this conversation as resolved

konrad force-pushed fix/api-url-error from b6dafdc6ef to f7e7dce259

2022-02-20 19:15:31 +00:00

Compare

dpschen reviewed 2022-02-20 21:11:09 +00:00

src/components/misc/api-config.vue Outdated

						
				@ -61,18 +61,33 @@ const emit = defineEmits(['foundApi'])

				const apiUrl = ref(window.API_URL)

				const configureApi = ref(apiUrl.value === '')

				const apiDomain = computed(() => parseURL(apiUrl.value).host || parseURL(window.location.href).host)

dpschen commented

2022-02-20 21:11:09 +00:00

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

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

konrad marked this conversation as resolved

konrad force-pushed fix/api-url-error from f7e7dce259 to ae1fe20f3e

2022-02-20 21:38:17 +00:00

Compare

dpschen approved these changes 2022-02-20 21:54:51 +00:00

dpschen force-pushed fix/api-url-error from ae1fe20f3e to 661cc45235

2022-02-20 21:55:56 +00:00

Compare

konrad merged commit bdb53ec8ee into main