From c6265335af3625eb2a200c7ecb9395e67bd6e069 Mon Sep 17 00:00:00 2001
From: Paul Fitzpatrick <paulfitz@alum.mit.edu>
Date: Thu, 27 May 2021 16:08:18 -0400
Subject: [PATCH] (core) make SELF_HYPERLINK urls cleaner

Summary:
This cleans up a few things about SELF_HYPERLINK urls:

 * Use `urlId` rather than `docId`.
 * Correctly merge personal org subdomain.
 * In dev environment, use clearer port number.

Test Plan: updated test

Reviewers: alexmojaki, dsagal

Reviewed By: dsagal

Differential Revision: https://phab.getgrist.com/D2841
---
 app/gen-server/lib/HomeDBManager.ts | 25 +++++++++++++++++--------
 app/server/devServerMain.ts         | 10 ++++++++--
 app/server/lib/FlexServer.ts        |  2 +-
 3 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/app/gen-server/lib/HomeDBManager.ts b/app/gen-server/lib/HomeDBManager.ts
index bd15be31..236082a3 100644
--- a/app/gen-server/lib/HomeDBManager.ts
+++ b/app/gen-server/lib/HomeDBManager.ts
@@ -2277,15 +2277,24 @@ export class HomeDBManager extends EventEmitter {
   public normalizeOrgDomain(orgId: number, domain: string|null,
                             ownerId: number|undefined, mergePersonalOrgs: boolean = true,
                             suppressDomain: boolean = false): string {
-    if (!domain) {
-      if (ownerId) {
-        // This is an org with no domain set, and an owner set.
-        domain = mergePersonalOrgs ? this.mergedOrgDomain() : `docs-${this._idPrefix}${ownerId}`;
-      } else {
-        // This is an org with no domain or owner set.
-        domain = `o-${this._idPrefix}${orgId}`;
+    if (ownerId) {
+      // An org with an ownerId set is a personal org.  Historically, those orgs
+      // have a subdomain like docs-NN where NN is the user ID.
+      const personalDomain = `docs-${this._idPrefix}${ownerId}`;
+      // In most cases now we pool all personal orgs as a single virtual org.
+      // So when mergePersonalOrgs is on, and the subdomain is either not set
+      // (as it is in the database for personal orgs) or set to something
+      // like docs-NN (as it is in the API), normalization should just return the
+      // single merged org ("docs" or "docs-s").
+      if (mergePersonalOrgs && (!domain || domain === personalDomain)) {
+        domain = this.mergedOrgDomain();
       }
-    } else if (suppressDomain)  {
+      if (!domain) {
+        domain = personalDomain;
+      }
+    } else if (suppressDomain || !domain) {
+      // If no subdomain is set, or custom subdomains or forbidden, return something
+      // uninspiring but unique, like o-NN where NN is the org ID.
       domain = `o-${this._idPrefix}${orgId}`;
     }
     return domain;
diff --git a/app/server/devServerMain.ts b/app/server/devServerMain.ts
index 419f0d32..7f12ae7f 100644
--- a/app/server/devServerMain.ts
+++ b/app/server/devServerMain.ts
@@ -88,9 +88,16 @@ export async function main() {
     return;
   }
 
+  // The home server and web server(s) are effectively identical in Grist deployments
+  // now, but remain distinct in some test setups.
   const homeServerPort = getPort("HOME_PORT", 9000);
+  const webServerPort = getPort("PORT", 8080);
   if (!process.env.APP_HOME_URL) {
-    process.env.APP_HOME_URL = `http://localhost:${homeServerPort}`;
+    // All servers need to know a "main" URL for Grist.  This is generally
+    // that of the web server.  In some test setups, the web server port is left
+    // at 0 to be auto-allocated, but for those tests it suffices to use the home
+    // server port.
+    process.env.APP_HOME_URL = `http://localhost:${webServerPort || homeServerPort}`;
   }
 
   // Bring up the static resource server
@@ -107,7 +114,6 @@ export async function main() {
 
   // If a distinct webServerPort is specified, we listen also on that port, though serving
   // exactly the same content.  This is handy for testing CORS issues.
-  const webServerPort = getPort("PORT", 8080);
   if (webServerPort !== 0 && webServerPort !== homeServerPort) {
     await home.startCopy('webServer', webServerPort);
   }
diff --git a/app/server/lib/FlexServer.ts b/app/server/lib/FlexServer.ts
index 8a852175..6f555bbf 100644
--- a/app/server/lib/FlexServer.ts
+++ b/app/server/lib/FlexServer.ts
@@ -1147,7 +1147,7 @@ export class FlexServer implements GristServer {
       state.ws = resource.id;
     } else {
       org = resource.workspace.org;
-      state.doc = resource.id;
+      state.doc = resource.urlId || resource.id;
       state.slug = getSlugIfNeeded(resource);
     }
     state.org = this.dbManager.normalizeOrgDomain(org.id, org.domain, org.ownerId);