Page MenuHomePhabricator
Differential D9451

[web/native/keyserver] Start using new url facts
ClosedPublic
Actions

Authored by michal on Oct 11 2023, 8:56 AM.
Tags
None
Referenced Files
F3332536: D9451.id31940.diff
Thu, Nov 21, 12:26 AM
Unknown Object (File)
Wed, Nov 20, 3:16 PM
Unknown Object (File)
Sun, Nov 17, 5:10 PM
Unknown Object (File)
Fri, Nov 8, 12:28 AM
Unknown Object (File)
Fri, Nov 8, 12:28 AM
Unknown Object (File)
Fri, Nov 8, 12:28 AM
Unknown Object (File)
Fri, Nov 8, 12:28 AM
Unknown Object (File)
Fri, Nov 8, 12:27 AM
View All Files
Subscribers
ashoat
tomek
will

Details

Reviewers
inka
kamil
atul
ashoat
Commits
rCOMM5c2682a20c45: [web/native/keyserver] Start using new url facts
Summary

Part of ENG-5153
Depends on D9398

Now we can start using the new /webapp/ and /keyserver urls in other places.
This also brings back the option of testing the web app with "staging" keyserver because it sets the urlPrefix to the one specified in the url facts (passed as a variable in html) (more context in D9145).

Test Plan
  • check that web and native migrations work
  • check that uploading a new image works
  • test that navigating to https://localhost:3000/comm/chat/thread/256%7C83932/ redirects to /webapp/
  • I run the keyserver docs according to the instructions in the nix_keyserver_deployment.md to check if both new web app and keyserver endpoints work:
    • The webapp successfully loaded under http://localhost:3000/webapp/
    • In the network inspector I could see a successful request to the keyserver (http://localhost:3000/keyserver/get_initial_redux_state)

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

michal created this revision.Oct 11 2023, 8:56 AM
Herald added subscribers: will, tomek, ashoat. · View Herald TranscriptOct 11 2023, 8:56 AM
michal added inline comments.Oct 11 2023, 9:04 AM
keyserver/icons/site.webmanifest
6–11 ↗(On Diff #31940)

They are now fetched relative to the site.webmanifest. So on dev they will be fetched from localhost:3000/webapp/android-chrome.png and on production from web.comm.app/android-chrome.png so there should be no difference on production.

keyserver/src/creators/report-creator.js
148–152 ↗(On Diff #31940)

I don't think it's worth migrating the commbot messages to /keyserver// squadcal.com.

keyserver/src/fetchers/upload-fetchers.js
107 ↗(On Diff #31940)

We could migrate existing urls (on clients) to use /keyserver// squadcal.com but:

  • old clients still need to work with older urls
  • we will be migrating all urls to blob schema anyway in the future

because of that I think it's better to just add a redirect for now (it's in the next diff).

keyserver/src/keyserver.js
242–249 ↗(On Diff #31940)

Added to handle old links, bookmarks etc. on dev. On production there should be no difference because webAppUrl and commAppUrl should both have web.comm.app so we don't need to add a redirect.

web/redux/persist.js
164–176 ↗(On Diff #31940)

This should be a no-op on production but we should still run it to enable existing "testing" web clients to connect to staging.

michal edited the summary of this revision. (Show Details)Oct 11 2023, 9:07 AM
michal published this revision for review.Oct 11 2023, 9:14 AM
Harbormaster completed remote builds in B23192: Diff 31940.Oct 11 2023, 9:15 AM
michal added a child revision: D9452: [keyserver] Separate webapp and keyserver endpoints.Oct 11 2023, 9:21 AM
kamil accepted this revision.Oct 16 2023, 3:49 AM
kamil added inline comments.
web/redux/persist.js
164–176 ↗(On Diff #31940)

I think this migrating URL from web.comm.app to squadcal, is that correct?

This revision is now accepted and ready to land.Oct 16 2023, 3:49 AM
michal added inline comments.Oct 16 2023, 3:51 AM
web/redux/persist.js
164–176 ↗(On Diff #31940)

Ah right I got confused from native. On web production this will change the url from web.comm.app to squadcal (or to some different url in case of "staging") and on web dev it will change it from localhost:3000/comm to localhost:3000/webapp.

michal updated this revision to Diff 32148.Oct 18 2023, 3:59 AM

Rebase

Harbormaster completed remote builds in B23331: Diff 32148.Oct 18 2023, 4:16 AM
michal mentioned this in D9396: [keyserver] Update CORS.Oct 20 2023, 8:56 AM
michal updated this revision to Diff 32274.Oct 20 2023, 9:06 AM

Doc changes

Harbormaster completed remote builds in B23406: Diff 32274.Oct 20 2023, 9:24 AM
ashoat requested changes to this revision.Oct 22 2023, 3:49 PM

Some docs issues

docs/linux_dev_environment.md
94–95 ↗(On Diff #32274)

I don't think these changes are sufficient. You'll also need this line for JSON keyserver endpoints:

ProxyPass /keyserver/ http://localhost:3000/keyserver/
docs/nix_keyserver_deployment.md
15–16 ↗(On Diff #32274)

This is broken for a couple reasons:

  1. The names of the environmental variables need to be updated to reflect your changes earlier
  2. You need a keyserver config now. It wasn't previously necessary but now it is

It might help to actually test it

This revision now requires changes to proceed.Oct 22 2023, 3:49 PM
michal updated this revision to Diff 32463.Oct 27 2023, 7:54 AM

Updated the docs issues, amended the test plan for docker keyserver deploy.

michal edited the test plan for this revision. (Show Details)Oct 27 2023, 7:55 AM
ashoat accepted this revision.Oct 27 2023, 8:00 AM
This revision is now accepted and ready to land.Oct 27 2023, 8:00 AM
Harbormaster failed remote builds in B23538: Diff 32463!Oct 27 2023, 8:47 AM
Harbormaster completed remote builds in B23538: Diff 32463.Oct 27 2023, 8:58 AM
Closed by commit rCOMM5c2682a20c45: [web/native/keyserver] Start using new url facts (authored by Michal Gniadek <michal.gniadek@swmansion.com>). · Explain WhyOct 31 2023, 2:55 AM
This revision was automatically updated to reflect the committed changes.
Michal Gniadek <michal.gniadek@swmansion.com> added a commit: rCOMM5c2682a20c45: [web/native/keyserver] Start using new url facts.

Revision Contents
Changeset List

PathSize
desktop/
src/
2 lines
docs/
12 lines
5 lines
5 lines
2 lines
keyserver/
icons/
4 lines
src/
creators/
4 lines
fetchers/
4 lines
28 lines
responders/
17 lines
utils/
36 lines
native/
redux/
19 lines
utils/
2 lines
web/
push-notif/
2 lines
redux/
7 lines
17 lines

Diff 32528

View Options

desktop/src/main.js

Loading...
View Options

docs/dev_environment.md

Loading...
View Options

docs/linux_dev_environment.md

Loading...
View Options

docs/nix_keyserver_deployment.md

Loading...
View Options

docs/nix_web_workflows.md

Loading...
View Options

keyserver/icons/site.webmanifest

Loading...
View Options

keyserver/src/creators/report-creator.js

Loading...
View Options

keyserver/src/fetchers/upload-fetchers.js

Loading...
View Options

keyserver/src/keyserver.js

Loading...
View Options

keyserver/src/responders/website-responders.js

Loading...
View Options

keyserver/src/utils/urls.js

Loading...
View Options

native/redux/persist.js

Loading...
View Options

native/utils/url-utils.js

Loading...
View Options

web/push-notif/service-worker.js

Loading...
View Options

web/redux/default-state.js

Loading...
View Options

web/redux/persist.js

Loading...