From 6391474d1455558a5d55579724360f5c73e50f7e Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Sun, 4 Nov 2018 00:59:52 -0700 Subject: [PATCH] getUrl -> url consistency test improvements --- app/components/Auth.js | 2 ++ server/api/hooks.test.js | 2 +- server/api/team.js | 5 ++++- server/mailer.test.js | 2 ++ server/models/Collection.js | 9 +++++---- server/models/Collection.test.js | 4 ++-- server/models/Document.js | 13 +++++++------ server/presenters/collection.js | 2 +- server/presenters/document.js | 2 +- server/presenters/share.js | 2 +- server/presenters/slackAttachment.js | 2 +- server/routes.js | 2 +- server/services/slack/index.js | 2 +- server/utils/domains.js | 2 ++ server/utils/domains.test.js | 17 +++++++++++++++++ setupJest.js | 2 +- 16 files changed, 49 insertions(+), 21 deletions(-) create mode 100644 server/utils/domains.test.js diff --git a/app/components/Auth.js b/app/components/Auth.js index 762a556a1..7c8bc33f6 100644 --- a/app/components/Auth.js +++ b/app/components/Auth.js @@ -24,6 +24,8 @@ const Auth = observer(({ auth, children }: Props) => { return ; } + // 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 ; diff --git a/server/api/hooks.test.js b/server/api/hooks.test.js index 27b7a4151..117bc9bb1 100644 --- a/server/api/hooks.test.js +++ b/server/api/hooks.test.js @@ -38,7 +38,7 @@ describe('#hooks.unfurl', async () => { links: [ { domain: 'getoutline.com', - url: document.getUrl(), + url: document.url, }, ], }, diff --git a/server/api/team.js b/server/api/team.js index 6e4bf5d79..d3c7561cd 100644 --- a/server/api/team.js +++ b/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}`)) { diff --git a/server/mailer.test.js b/server/mailer.test.js index fe2023628..2fc77f97a 100644 --- a/server/mailer.test.js +++ b/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 = { diff --git a/server/models/Collection.js b/server/models/Collection.js index 795127006..d9bfaa8eb 100644 --- a/server/models/Collection.js +++ b/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, diff --git a/server/models/Collection.test.js b/server/models/Collection.test.js index 930ce991a..65d8e68ce 100644 --- a/server/models/Collection.test.js +++ b/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'); }); }); diff --git a/server/models/Document.js b/server/models/Document.js index 62cb5ae71..2cca85179 100644 --- a/server/models/Document.js +++ b/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: [], }; }; diff --git a/server/presenters/collection.js b/server/presenters/collection.js index 5b9af99da..f4e9cd95f 100644 --- a/server/presenters/collection.js +++ b/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', diff --git a/server/presenters/document.js b/server/presenters/document.js index e84d7704f..50388e657 100644 --- a/server/presenters/document.js +++ b/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, diff --git a/server/presenters/share.js b/server/presenters/share.js index 1de3825ca..8d9e5f7df 100644 --- a/server/presenters/share.js +++ b/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, diff --git a/server/presenters/slackAttachment.js b/server/presenters/slackAttachment.js index a8f842126..5c91a9bfe 100644 --- a/server/presenters/slackAttachment.js +++ b/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(), diff --git a/server/routes.js b/server/routes.js index fa8c6d0f1..aa347d6a3 100644 --- a/server/routes.js +++ b/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; } diff --git a/server/services/slack/index.js b/server/services/slack/index.js index 6903b9c9a..08b225244 100644 --- a/server/services/slack/index.js +++ b/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, }, ], diff --git a/server/utils/domains.js b/server/utils/domains.js index d43f988a1..13133abda 100644 --- a/server/utils/domains.js +++ b/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; } diff --git a/server/utils/domains.test.js b/server/utils/domains.test.js new file mode 100644 index 000000000..c0556f47e --- /dev/null +++ b/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'); + }); +}); diff --git a/setupJest.js b/setupJest.js index ac3a3fe1f..98896a9cf 100644 --- a/setupJest.js +++ b/setupJest.js @@ -10,4 +10,4 @@ const snap = children => { }; global.localStorage = localStorage; -global.snap = snap; +global.snap = snap; \ No newline at end of file