Use github login info to associate pages/comments with a user (#467)

swang · maxbeatty · commit 02295f79a6ff · 2018-10-08T21:37:46.000-07:00
diff --git a/server/lib/db/migrations/20180311_add_github_to_comments.js b/server/lib/db/migrations/20180311_add_github_to_comments.js
@@ -0,0 +1,13 @@
+module.exports = {
+  up: function (genericQuery) {
+    return genericQuery(`
+      ALTER TABLE comments ADD COLUMN authorGitHub varchar(39) NOT NULL AFTER author, ADD INDEX gh_username (authorGitHub);
+    `);
+  },
+
+  down: function (genericQuery) {
+    return genericQuery(`
+      ALTER TABLE comments DROP COLUMN authorGithub, DROP INDEX gh_username;
+    `);
+  }
+};
diff --git a/server/lib/db/migrations/20180311_add_github_to_pages.js b/server/lib/db/migrations/20180311_add_github_to_pages.js
@@ -0,0 +1,13 @@
+module.exports = {
+  up: function (genericQuery) {
+    return genericQuery(`
+      ALTER TABLE pages ADD COLUMN authorGitHub varchar(39) NOT NULL AFTER author, ADD INDEX gh_username (authorGitHub);
+    `);
+  },
+
+  down: function (genericQuery) {
+    return genericQuery(`
+      ALTER TABLE pages DROP COLUMN authorGithub, DROP INDEX gh_username;
+    `);
+  }
+};
diff --git a/server/lib/defaults.js b/server/lib/defaults.js
@@ -37,6 +37,7 @@ exports.testPageContext = {
   genError: null,
   slugPattern: regex.slug,
   author: '',
+  authorGitHub: '',
   authorEmail: '',
   authorURL: '',
   title: '',
diff --git a/server/lib/regex.js b/server/lib/regex.js
@@ -85,5 +85,6 @@ module.exports = {
     // resource path
     '(?:/\\S*)?' + '$',
   slug: '[A-Za-z0-9](?:-?[A-Za-z0-9])*',
-  script: '(<script[^>]*?>)([\\s\\S]*?)(</script>)'
+  script: '(<script[^>]*?>)([\\s\\S]*?)(</script>)',
+  github: '(?=^.{0,39}$)[A-Za-z0-9]+(-?[A-Za-z0-9])+$'
 };
diff --git a/server/lib/schema.js b/server/lib/schema.js
@@ -6,13 +6,15 @@ const mediumText = Joi.string().allow('').max(defaults.mediumTextLength);
 const author = Joi.string().min(1);
 const authorEmail = Joi.string().email();
 const authorURL = Joi.string().allow('').regex(new RegExp(regex.url, 'i'), 'url');
+const authorGitHub = Joi.string().allow('').max(39).regex(new RegExp(regex.github, 'i'));
 
 exports.mediumText = mediumText;
 
 exports.testPage = Joi.object().keys({
   author: author.allow(''),
   authorEmail: authorEmail.allow(''),
   authorURL,
+  authorGitHub,
   title: Joi.string().required().trim().min(1).max(255),
   slug: Joi.string().required().trim().min(1).max(55).regex(new RegExp(regex.slug), 'slug'),
   visible: Joi.string().default('n').valid('y', 'n'),
@@ -32,5 +34,6 @@ exports.comment = Joi.object().keys({
   author: author.required(),
   authorEmail: authorEmail.required(),
   authorURL: authorURL.required(),
+  authorGitHub,
   message: Joi.string().min(1).max(defaults.mediumTextLength)
 });
diff --git a/server/repositories/pages.js b/server/repositories/pages.js
@@ -54,16 +54,18 @@ exports.register = function (server, options, next) {
     // WHERE
     //   author LIKE '%{{author}}%'
     //   OR author LIKE '{{author}}'
+    //   OR authorGitHub = '{{author}}'
     //   AND updated IN (SELECT MAX(updated) FROM pages WHERE visible = 'y' GROUP BY slug)
     //   AND visible = 'y'
     // ORDER BY updated DESC
 
     var wcAuthor = '%' + author + '%';
     var a = author.trim().replace('-', '%');
+    var gh = author.trim();
 
     return db.genericQuery(
-      "SELECT id AS pID, slug AS url, revision, title, published, updated, author, (SELECT COUNT(*) FROM pages WHERE slug = url AND visible = 'y') AS revisionCount, (SELECT COUNT(*) FROM tests WHERE pageID = pID) AS testCount FROM pages WHERE author LIKE ? OR author LIKE ? AND updated IN (SELECT MAX(updated) FROM pages WHERE visible = 'y' GROUP BY slug) AND visible = 'y' ORDER BY updated DESC",
-      [wcAuthor, a]
+      "SELECT id AS pID, slug AS url, revision, title, published, updated, author, authorGitHub, (SELECT COUNT(*) FROM pages WHERE slug = url AND visible = 'y') AS revisionCount, (SELECT COUNT(*) FROM tests WHERE pageID = pID) AS testCount FROM pages WHERE (author LIKE ? OR author LIKE ? OR authorGitHub = ?) AND updated IN (SELECT MAX(updated) FROM pages WHERE visible = 'y' GROUP BY slug) AND visible = 'y' ORDER BY updated DESC",
+      [wcAuthor, a, gh]
     );
   });
 
@@ -119,7 +121,7 @@ exports.register = function (server, options, next) {
     server.log(['debug'], `${name}::findBySlug: ${JSON.stringify(arguments)}`);
 
     return db.genericQuery(
-      'SELECT published, updated, author, authorEmail, authorURL, revision, visible, title FROM pages WHERE slug = ? ORDER BY published ASC',
+      'SELECT published, updated, author, authorGitHub, authorEmail, authorURL, revision, visible, title FROM pages WHERE slug = ? ORDER BY published ASC',
       [slug]
     );
   });
@@ -128,7 +130,7 @@ exports.register = function (server, options, next) {
     server.log(['debug'], `${name}::findBySlug: ${JSON.stringify(arguments)}`);
 
     return db.genericQuery(
-      'SELECT published, updated, author, authorEmail, revision, visible, title FROM pages WHERE slug = ? AND visible = ? ORDER BY published ASC',
+      'SELECT published, updated, author, authorGitHub, authorEmail, revision, visible, title FROM pages WHERE slug = ? AND visible = ? ORDER BY published ASC',
       [slug, 'y']
     );
   });
diff --git a/server/services/comments.js b/server/services/comments.js
@@ -10,6 +10,7 @@ exports.register = function (server, options, next) {
       author: payload.author,
       authorEmail: payload.authorEmail,
       authorURL: payload.authorURL,
+      authorGitHub: payload.authorGitHub,
       content: payload.message,
       ip,
       published: new Date()
diff --git a/server/web/auth/github.js b/server/web/auth/github.js
@@ -13,13 +13,13 @@ exports.register = function (server, options, next) {
   });
 
   // attach credentials to view context to let people know they are logged in
-  // attach authorSlug to view context to construct "My Tests" link in footer
+  // attach authorGitHub to view context to construct "My Tests" link in footer
   server.ext('onPreResponse', function (request, reply) {
     const response = request.response;
     if (response.variety === 'view') {
       response.source.context = response.source.context || {};
       response.source.context.credentials = request.auth.isAuthenticated ? request.auth.credentials : null;
-      response.source.context.authorSlug = request.yar.get('authorSlug');
+      response.source.context.authorGitHub = request.yar.get('authorGitHub');
     }
     return reply.continue();
   });
diff --git a/server/web/browse/index.js b/server/web/browse/index.js
@@ -90,19 +90,19 @@ exports.register = function (server, options, next) {
 
   server.route({
     method: 'GET',
-    path: '/browse/{authorSlug}',
+    path: '/browse/{authorGitHub}',
     handler: function (request, reply) {
-      pagesRepo.getLatestVisibleForAuthor(request.params.authorSlug)
+      pagesRepo.getLatestVisibleForAuthor(request.params.authorGitHub)
         .then(function (rows) {
           if (rows.length === 0) {
             reply(Boom.notFound('The author was not found'));
           } else {
             reply.view('browse/author', {
-              headTitle: 'Test cases by ' + request.params.authorSlug,
+              headTitle: 'Test cases by ' + request.params.authorGitHub,
               showAtom: {
-                slug: 'browse/' + request.params.authorSlug
+                slug: 'browse/' + request.params.authorGitHub
               },
-              author: request.params.authorSlug,
+              author: request.params.authorGitHub,
               pages: rows
             });
           }
@@ -115,14 +115,14 @@ exports.register = function (server, options, next) {
 
   server.route({
     method: 'GET',
-    path: '/browse/{authorSlug}.atom',
+    path: '/browse/{authorGitHub}.atom',
     handler: function (request, reply) {
-      pagesRepo.getLatestVisibleForAuthor(request.params.authorSlug)
+      pagesRepo.getLatestVisibleForAuthor(request.params.authorGitHub)
         .then(function (rows) {
           var updated = getUpdatedDate(rows);
 
           reply.view('browse/author-atom', {
-            author: request.params.authorSlug,
+            author: request.params.authorGitHub,
             update: updated.toISOString,
             pages: rows
           }, {
diff --git a/server/web/edit/index.js b/server/web/edit/index.js
@@ -78,6 +78,8 @@ exports.register = function (server, options, next) {
         }).code(400);
       };
 
+      request.payload.authorGitHub = request.auth.credentials.username;
+
       Joi.validate(request.payload, schema.testPage, function (err, pageWithTests) {
         let errObj = {};
         if (err) {
@@ -127,8 +129,7 @@ exports.register = function (server, options, next) {
                 return pagesService.edit(pageWithTests, update, prevPage.maxRev, prevPage.id);
               })
               .then(resultingRevision => {
-                request.yar.set('authorSlug', pageWithTests.author.replace(' ', '-').replace(/[^a-zA-Z0-9 -]/, ''));
-
+                request.yar.set('authorGitHub', pageWithTests.authorGitHub.replace(/[^a-zA-Z0-9-]/, ''));
                 const r = resultingRevision > 1 ? `/${resultingRevision}` : '';
 
                 reply.redirect(`/${request.params.testSlug}${r}`);
diff --git a/server/web/home/index.js b/server/web/home/index.js
@@ -49,6 +49,8 @@ exports.register = function (server, options, next) {
         }, errObj)).code(400);
       };
 
+      request.payload.authorGitHub = request.auth.credentials.username;
+
       Joi.validate(request.payload, schema.testPage, function (er, pageWithTests) {
         let errObj = {};
         if (er) {
@@ -107,7 +109,7 @@ exports.register = function (server, options, next) {
                       const own = request.yar.get('own') || {};
                       own[resultPageId] = true;
                       request.yar.set('own', own);
-                      request.yar.set('authorSlug', payload.author.replace(' ', '-').replace(/[^a-zA-Z0-9 -]/, ''));
+                      request.yar.set('authorGitHub', payload.authorGitHub.replace(/[^a-zA-Z0-9-]/, ''));
                       reply.redirect('/' + payload.slug);
                     });
                 }
diff --git a/server/web/test/index.js b/server/web/test/index.js
@@ -125,6 +125,8 @@ exports.register = function (server, options, next) {
 
       getTestPage(request)
         .then((model) => {
+          request.payload.authorGitHub = request.auth.credentials.username;
+
           const result = Joi.validate(request.payload, schema.comment, {abortEarly: false});
           if (result.error) {
             let errObj = {};
diff --git a/templates/partials/footer.hbs b/templates/partials/footer.hbs
@@ -16,9 +16,9 @@
             <a href="/">Add</a>
           </li>
 
-          {{#if authorSlug}}
+          {{#if authorGitHub}}
             <li>
-              <a href="/browse/{{authorSlug}}">My tests</a>
+              <a href="/browse/{{authorGitHub}}">My tests</a>
             </li>
           {{/if}}
 
diff --git a/test/unit/server/lib/regex.js b/test/unit/server/lib/regex.js
@@ -77,4 +77,34 @@ lab.experiment('Regex Lib', function () {
       done();
     });
   });
+
+  lab.experiment('github', function () {
+    lab.test('is a string', function (done) {
+      Code.expect(regex.github).to.be.a.string();
+
+      done();
+    });
+
+    lab.test('matches valid github name', function (done) {
+      var re = new RegExp(regex.github);
+
+      Code.expect(re.test('agithubname5000')).to.be.true();
+      Code.expect(re.test('agit-hubname5000')).to.be.true();
+      Code.expect(re.test('a-github-name500')).to.be.true();
+      Code.expect(re.test('a'.repeat(39))).to.be.true();
+
+      done();
+    });
+
+    lab.test('doesn\'t allow dashes at beginning or end of github name, nor allows for more than one consecutive dash', function (done) {
+      var re = new RegExp(regex.github);
+
+      Code.expect(re.test('-agithubname5000')).to.be.false();
+      Code.expect(re.test('agithubname5000-')).to.be.false();
+      Code.expect(re.test('agit--hubname500')).to.be.false();
+      Code.expect(re.test('a'.repeat(40))).to.be.false();
+
+      done();
+    });
+  });
 });
diff --git a/test/unit/server/lib/schema.js b/test/unit/server/lib/schema.js
@@ -31,6 +31,7 @@ lab.experiment('Schema Library', function () {
           author: 'Pitcher Man',
           authorEmail: 'kool-aid@kraft.com',
           authorURL: 'http://kool-aid.com',
+          authorGitHub: 'koolaidman',
           title: 'oh',
           slug: 'wee',
           info: '',
diff --git a/test/unit/server/repositories/pages.js b/test/unit/server/repositories/pages.js
@@ -274,7 +274,7 @@ lab.experiment('Pages Repository', function () {
         .then(function (p) {
           Code.expect(p).to.be.array();
           Code.expect(genericQueryStub.calledWithExactly(
-            'SELECT published, updated, author, authorEmail, authorURL, revision, visible, title FROM pages WHERE slug = ? ORDER BY published ASC',
+            'SELECT published, updated, author, authorGitHub, authorEmail, authorURL, revision, visible, title FROM pages WHERE slug = ? ORDER BY published ASC',
             [slug]
           )).to.be.true();
 
@@ -293,7 +293,7 @@ lab.experiment('Pages Repository', function () {
         .then(p => {
           Code.expect(p).to.be.array();
           Code.expect(genericQueryStub.calledWithExactly(
-            'SELECT published, updated, author, authorEmail, revision, visible, title FROM pages WHERE slug = ? AND visible = ? ORDER BY published ASC',
+            'SELECT published, updated, author, authorGitHub, authorEmail, revision, visible, title FROM pages WHERE slug = ? AND visible = ? ORDER BY published ASC',
             [slug, 'y']
           )).to.be.true();
 
diff --git a/test/unit/server/web/edit/index.js b/test/unit/server/web/edit/index.js
@@ -379,7 +379,7 @@ lab.experiment('POST', function () {
     request = {
       method: 'POST',
       url: '/wee/1/edit',
-      credentials: {'test': 'profile'},
+      credentials: {'username': 'koolaidman', 'test': 'profile'},
       payload: {
         author: 'Pitcher Man',
         authorEmail: 'kool-aid@kraft.com',
diff --git a/test/unit/server/web/home/index.js b/test/unit/server/web/home/index.js
@@ -117,7 +117,7 @@ lab.experiment('home', function () {
       request = {
         method: 'POST',
         url: '/',
-        credentials: {'test': 'profile'},
+        credentials: {'username': 'koolaidman', 'test': 'profile'},
         payload: {
           author: 'Pitcher Man',
           authorEmail: 'kool-aid@kraft.com',
diff --git a/test/unit/server/web/test/index.js b/test/unit/server/web/test/index.js
@@ -560,7 +560,7 @@ lab.experiment('create comment', () => {
 
   lab.experiment('when authorized', () => {
     lab.beforeEach(done => {
-      request.credentials = {'test': 'profile'};
+      request.credentials = {'username': 'somerandomaccount', 'test': 'profile'};
       done();
     });
 
