getUrl -> url consistency

test improvements

app/components/Auth.js

@@ -24,6 +24,8 @@ const Auth = observer(({ auth, children }: Props) => {
       return <LoadingIndicator />;
     }
 
+    // Check the current origin against the teams url, if they differ we need to
+    // redirect to the canonical subdomain for this team
     if (window.location.origin !== team.url) {
       window.location.href = `${team.url}${window.location.pathname}`;
       return <LoadingIndicator />;

server/api/hooks.test.js

@@ -38,7 +38,7 @@ describe('#hooks.unfurl', async () => {
           links: [
             {
               domain: 'getoutline.com',
-              url: document.getUrl(),
+              url: document.url,
             },
           ],
         },

server/api/team.js

@@ -19,7 +19,10 @@ router.post('team.update', auth(), async ctx => {
   const team = await Team.findById(user.teamId);
   authorize(user, 'update', team);
 
-  team.subdomain = subdomain === '' ? null : subdomain;
+  if (process.env.SUBDOMAINS_ENABLED) {
+    team.subdomain = subdomain === '' ? null : subdomain;
+  }
+
   if (name) team.name = name;
   if (sharing !== undefined) team.sharing = sharing;
   if (avatarUrl && avatarUrl.startsWith(`${endpoint}/uploads/${user.id}`)) {

server/mailer.test.js

@@ -6,7 +6,9 @@ describe('Mailer', () => {
   let sendMailOutput;
 
   beforeEach(() => {
+    process.env.URL = 'http://localhost:3000';
     process.env.SMTP_FROM_EMAIL = 'hello@example.com';
+    jest.resetModules();
 
     fakeMailer = new Mailer();
     fakeMailer.transporter = {

server/models/Collection.js

@@ -70,6 +70,11 @@ const Collection = sequelize.define(
         await collection.save();
       },
     },
+    getterMethods: {
+      url() {
+        return `/collections/${this.id}`;
+      },
+    },
   }
 );
 
@@ -120,10 +125,6 @@ Collection.addHook('afterUpdate', model =>
 
 // Instance methods
 
-Collection.prototype.getUrl = function() {
-  return `/collections/${this.id}`;
-};
-
 Collection.prototype.addDocumentToStructure = async function(
   document,
   index,

server/models/Collection.test.js

@@ -6,10 +6,10 @@ import uuid from 'uuid';
 beforeEach(flushdb);
 beforeEach(jest.resetAllMocks);
 
-describe('#getUrl', () => {
+describe('#url', () => {
   test('should return correct url for the collection', () => {
     const collection = new Collection({ id: '1234' });
-    expect(collection.getUrl()).toBe('/collections/1234');
+    expect(collection.url).toBe('/collections/1234');
   });
 });
 

server/models/Document.js

@@ -104,6 +104,12 @@ const Document = sequelize.define(
       afterCreate: createRevision,
       afterUpdate: createRevision,
     },
+    getterMethods: {
+      url: function() {
+        const slugifiedTitle = slugify(this.title);
+        return `/doc/${slugifiedTitle}-${this.urlId}`;
+      },
+    },
   }
 );
 
@@ -312,18 +318,13 @@ Document.prototype.getSummary = function() {
   return lines.length >= 1 ? lines[1] : '';
 };
 
-Document.prototype.getUrl = function() {
-  const slugifiedTitle = slugify(this.title);
-  return `/doc/${slugifiedTitle}-${this.urlId}`;
-};
-
 Document.prototype.toJSON = function() {
   // Warning: only use for new documents as order of children is
   // handled in the collection's documentStructure
   return {
     id: this.id,
     title: this.title,
-    url: this.getUrl(),
+    url: this.url,
     children: [],
   };
 };

server/presenters/collection.js

@@ -23,7 +23,7 @@ async function present(ctx: Object, collection: Collection) {
 
   const data = {
     id: collection.id,
-    url: collection.getUrl(),
+    url: collection.url,
     name: collection.name,
     description: collection.description,
     color: collection.color || '#4E5C6E',

server/presenters/document.js

@@ -25,7 +25,7 @@ async function present(ctx: Object, document: Document, options: ?Options) {
 
   const data = {
     id: document.id,
-    url: document.getUrl(),
+    url: document.url,
     urlId: document.urlId,
     title: document.title,
     text: document.text,

server/presenters/share.js

@@ -6,7 +6,7 @@ function present(ctx: Object, share: Share) {
   return {
     id: share.id,
     documentTitle: share.document.title,
-    documentUrl: share.document.getUrl(),
+    documentUrl: share.document.url,
     url: `${process.env.URL}/share/${share.id}`,
     createdBy: presentUser(ctx, share.user),
     createdAt: share.createdAt,

server/presenters/slackAttachment.js

@@ -11,7 +11,7 @@ function present(document: Document, context?: string) {
   return {
     color: document.collection.color,
     title: document.title,
-    title_link: `${process.env.URL}${document.getUrl()}`,
+    title_link: `${process.env.URL}${document.url}`,
     footer: document.collection.name,
     text,
     ts: document.getTimestamp(),

server/routes.js

@@ -92,7 +92,7 @@ router.get('/', async ctx => {
       );
     }
 
-    ctx.redirect(process.env.URL);
+    ctx.redirect(`${process.env.URL}?notice=invalid-auth`);
     return;
   }
 

server/services/slack/index.js

@@ -48,7 +48,7 @@ class Slack {
           {
             color: collection.color,
             title: collection.name,
-            title_link: `${process.env.URL}${collection.getUrl()}`,
+            title_link: `${process.env.URL}${collection.url}`,
             text: collection.description,
           },
         ],

server/utils/domains.js

@@ -3,6 +3,8 @@ import parseDomain from 'parse-domain';
 
 export function stripSubdomain(hostname: string) {
   const parsed = parseDomain(hostname);
+  if (!parsed) return hostname;
+
   if (parsed.tld) return `${parsed.domain}.${parsed.tld}`;
   return parsed.domain;
 }

server/utils/domains.test.js

@@ -0,0 +1,17 @@
+/* eslint-disable flowtype/require-valid-file-annotation */
+import { stripSubdomain } from './domains';
+
+describe('#stripSubdomain', () => {
+  test('to work with localhost', () => {
+    expect(stripSubdomain('localhost')).toBe('localhost');
+  });
+  test('to return domains without a subdomain', () => {
+    expect(stripSubdomain('example')).toBe('example');
+    expect(stripSubdomain('example.com')).toBe('example.com');
+    expect(stripSubdomain('example.org:3000')).toBe('example.org');
+  });
+  test('to remove subdomains', () => {
+    expect(stripSubdomain('test.example.com')).toBe('example.com');
+    expect(stripSubdomain('test.example.com:3000')).toBe('example.com');
+  });
+});