Use relative paths for API

[adamlerer]

Hi @kestasjk . With the 0.3 release, the beta UI isn't working in prod, due to using env-specified paths for the backend that weren't set to webdiplomacy.net when we compiled.

We're discussing on our end what's the best way to do this in the future. For the moment can you see if switching back to relative paths fixes the issue?

cc @utiq

[kestasjk]

Hi @adamlerer ,

Is it definitely not working at the moment? It seems to be working here after I added the undefined folder workaround.

I thought perhaps it was somehow detecting when I went into Web Developer tools to view the page, since it was working fine for me long after we committed but then suddenly stopped. In the error logs there were only three IPs I could see that have this issue:

FYI users can use www.webdiplomacy.net or webdiplomacy.net , and could have authentication errors if it has the domain encoded in.

Just want to be a bit clearer on what's going wrong before applying this. I would have thought that we would get heaps of error reports and that the error logs would be full if the issue is that all requests were going to the wrong place. Was there a reason for switching to using the environment variable or was that just for dev purposes?

Rgds,
Chris

[utiq]

Hello guys. I wouldn't suggest to go back to relative path because is going to break everything on dev environment. Also, it's not a good practice to use relative paths in production.

The fix is easy, it is just to add the production env variables when the app is built. Let me do that in the morning. Which branch should I use? This is happening in the master branch?

[adamlerer]

What is the "undefined folder workaround"? It has indeed started working for me when I tried it now. If it's working lets leave it alone until we can discuss the tradeoffs.

The source of the issue is that I ran npm run build without setting REACT_APP_WD_BASE_URL=https://webdiplomacy.net in my .env.X file, so this var was undefined and it was trying to access the backend through the path undefined/api.php instead of https://webdiplomacy.net/api.php.

We've gone back and forth between the env var and relative paths... I prefer relative paths because if you use env vars, then they need to be configured differently for dev (localhost) vs prod (https://webdiplomacy.net) vs other people hosting the site, and these hardcoded paths get checked in to the hardcoded js files in /beta and can't really be tested until you run in prod where you can hit that backend.

However @utiq and others have told me that relative urls are bad practice, which is probably true, but I haven't figured out why. P.S. @utiq I tried the dev environment with the relative paths and it seemed to work for me?

[utiq]

@kestasjk I cannot create a PR, can you add me as a collaborator so I can push the changes?

[kestasjk]

Right or wrong, to be honest the convention is that paths are relative and it’ll create issues where users that are logged in on www.WebDiplomacy.net<http://www.WebDiplomacy.net> won’t work if the static url doesn’t have the www. And they need to authenticate against https://webdiplomacy.net/api.php . There is a STATICSRV define() that you could use to inject a static site URL if you need to have a static URL locally for dev purposes, which is usually an empty string. The undefined folder workaround was that commit I did that added “beta/undefined/api.php” and “beta/undefined/Ajax.php” That does seem to be working now, Im sure we would be getting lots of complaints if it wasn’t working across the board but Im only seeing positive feedback at the moment. So I would say the way to go would be to go relative for the moment. Could we go back to doing the first commit to the staging branch, since now people are using the new UI much more heavily? BR, Kestas From: Adam Lerer ***@***.***> Sent: Saturday, 9 July 2022 11:25 AM To: kestasjk/webDiplomacy ***@***.***> Cc: kestasjk ***@***.***>; Mention ***@***.***> Subject: Re: [kestasjk/webDiplomacy] Use relative paths for API (PR #811) What is the "undefined folder workaround"? It has indeed started working for me when I tried it now. If it's working lets leave it alone until we can discuss the tradeoffs. The source of the issue is that I ran npm run build without setting REACT_APP_WD_BASE_URL=https://webdiplomacy.net in my .env.X file, so this var was undefined and it was trying to access the backend through the path undefined/api.php instead of https://webdiplomacy.net/api.php. We've gone back and forth between the env var and relative paths... I prefer relative paths because if you use env vars, then they need to be configured differently for dev (localhost) vs prod (https://webdiplomacy.net) vs other people hosting the site, and these hardcoded paths get checked in to the hardcoded js files in /beta and can't really be tested until you run in prod where you can hit that backend. However @utiq<https://github.com/utiq> and others have told me that relative urls are bad practice, which is probably true, but I haven't figured out why. P.S. @utiq<https://github.com/utiq> I tried the dev environment with the relative paths and it seemed to work for me? — Reply to this email directly, view it on GitHub<#811 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AAH2GTL7HJFQG5SJGLCFSD3VTDWJBANCNFSM53CNELRA>. You are receiving this because you were mentioned.Message ID: ***@***.******@***.***>>

[kestasjk]

Hi Cesar, The official site sync live with GitHub, so we require a pull request that we then merge in so we can stage changes. Otherwise you would basically be pushing code straight onto the live site, and we want to have a bit of oversight. We could give you push access to the staging branch, but for the official live site we need a pull request. The same process as Adam so he should be able to help you out. Rgds, Kestas Regards,

…

________________________________ From: Cesar Gutierrez ***@***.***> Sent: Saturday, July 9, 2022 11:52:44 PM To: kestasjk/webDiplomacy ***@***.***> Cc: kestasjk ***@***.***>; Mention ***@***.***> Subject: Re: [kestasjk/webDiplomacy] Use relative paths for API (PR #811) @kestasjk<https://github.com/kestasjk> I cannot create a PR, can you add me as a collaborator so I can push the changes? — Reply to this email directly, view it on GitHub<#811 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AAH2GTJLFANWCN5GWQUGXP3VTGN4ZANCNFSM53CNELRA>. You are receiving this because you were mentioned.Message ID: ***@***.***>

[utiq]

@kestasjk I wasn't trying to push code directly, I always create PRs. The problem is that I wasn't able to create remote branches in the repo, so I wasn't able to create a PR. How can we solve that?

[adamlerer]

Discussed with @utiq and we concluded that relative paths are the better option for this setup.

I've switched the PR to merge to staging for testing.

[kestasjk]

Hi Cesar, Hmm, I can have a dig around to see if I can give you the ability to add branches if need be, but Adam/Bryan will target an existing branch when they do a PR. I think they set up branches locally, and then they do a PR against staging / master when they’re ready to commit their changes to the official repo. If you ask Adam / Bryan how they are doing it hopefully they’ll be able to get you set up the same way. Regards,

…

________________________________ From: Cesar Gutierrez ***@***.***> Sent: Monday, July 11, 2022 9:50:15 PM To: kestasjk/webDiplomacy ***@***.***> Cc: kestasjk ***@***.***>; Mention ***@***.***> Subject: Re: [kestasjk/webDiplomacy] Use relative paths for API (PR #811) @kestasjk<https://github.com/kestasjk> I wasn't trying to push code directly, I always create PRs. The problem is that I wasn't able to create remote branches in the repo, so I wasn't able to create a PR. How can we solve that? — Reply to this email directly, view it on GitHub<#811 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AAH2GTO4PVXMSD7P5U74XKLVTQRBPANCNFSM53CNELRA>. You are receiving this because you were mentioned.Message ID: ***@***.***>