Merge pull request datopian#140 from datopian/feature/get-profile-gra…

…ceful-fail

[dms] Catch bad getProfile call and return empty obj

fixtures/index.js

@@ -786,7 +786,8 @@ module.exports.initMocks = function() {
 
   nock('http://127.0.0.1:5000', {"encodedQueryParams":true})
     .persist()
-    .post('/api/3/action/organization_show', {"id":"test_org_00","include_users":false})
+    .get('/api/3/action/organization_show')
+    .query({"id":"test_org_00","include_users":false})
     .reply(200, {"help":"http://127.0.0.1:5000/api/3/action/help_show?name=organization_show","success":true,"result":{"display_name":"Test Organization","description":"Just another test organization.","image_display_url":"http://placekitten.com/g/200/100","package_count":5,"created":"2019-03-27T21:26:27.501417","name":"test_org_00","is_organization":true,"state":"active","extras":[],"image_url":"http://placekitten.com/g/200/100","groups":[],"type":"organization","title":"Test Organization","revision_id":"24612477-8155-497c-9e8d-5fef03f94c52","num_followers":0,"id":"2669d62a-f122-4256-9382-21c260ceef40","tags":[],"approval_status":"approved"}}, [ 'Server',
     'PasteWSGIServer/0.5 Python/2.7.12',
     'Date',
@@ -796,11 +797,10 @@ module.exports.initMocks = function() {
     'Content-Length',
     '675' ])
 
-
-  nock('http://127.0.0.1:5000', {"encodedQueryParams":true})
-    .persist()
-    .post('/api/3/action/organization_show', {"id":"not-found-slug","include_users":false})
-    .reply(404)
+nock('http:/127.0.0.1:5000')
+    .get('/api/3/action/organization_show')
+    .query({"id":"known-bad","include_users":false})
+    .replyWithError('Mock error');
 
 
   nock('http://127.0.0.1:5000', {"encodedQueryParams":true})

lib/dms.js

@@ -20,9 +20,7 @@ class DmsModel {
       body: JSON.stringify(params),
       headers: { 'Content-Type': 'application/json' }
     })
-    if (response.status !== 200) {
-      throw response
-    }
+
     response = await response.json()
     // Convert CKAN descriptor => data package
     response.result.results = response.result.results.map(pkg => {
@@ -99,22 +97,22 @@ class DmsModel {
   }
 
   async getProfile(owner) {
-    const action = 'organization_show'
-    let url = new URL(resolve(this.api, action))
-    const params = {
-      id: owner,
-      include_users: false
-    }
-    let response = await fetch(url, {
-      method: 'POST',
-      body: JSON.stringify(params),
-      headers: { 'Content-Type': 'application/json' }
-    })
-    if (response.status !== 200) {
-      throw response
+    try {
+      const action = 'organization_show'
+      let url = new URL(resolve(this.api, action))
+      url.search = `id=${owner}&include_users=false`
+      let response = await fetch(url, {
+        method: 'GET',
+      })
+      if (response.status !== 200) {
+        throw response
+      }
+      response = await response.json()
+      return response.result
+    } catch (e) {
+      console.warn('Failed to fetch profile', e)
+      return {}
     }
-    response = await response.json()
-    return response.result
   }
 
   async getCollections(params) {

routes/dms.js

@@ -30,6 +30,7 @@ module.exports = function () {
     try {
       datapackage = await Model.getPackage(req.params.name)
     } catch (err) {
+      /* istanbul ignore next */
       next(err)
       return
     }
@@ -45,6 +46,7 @@ module.exports = function () {
     try {
       datapackage = await Model.getPackage(req.params.name)
     } catch (err) {
+      /* istanbul ignore next */
       next(err)
       return
     }
@@ -108,6 +110,7 @@ module.exports = function () {
         currentPage
       })
     } catch (e) {
+      /* istanbul ignore next */
       next(e)
     }
   })
@@ -122,6 +125,7 @@ module.exports = function () {
         slug: 'collections'
       })
     } catch (e) {
+      /* istanbul ignore next */
       next(e)
     }
   })
@@ -156,6 +160,7 @@ module.exports = function () {
         currentPage
       })
     } catch (e) {
+      /* istanbul ignore next */
       next(e)
     }
   })
@@ -168,6 +173,7 @@ module.exports = function () {
         datapackage = await Model.getPackage(req.params.name)
       }
     } catch (err) {
+      /* istanbul ignore next */
       next(err)
       return
     }
@@ -202,6 +208,7 @@ module.exports = function () {
         dpId: JSON.stringify(datapackage).replace(/'/g, "'") // keep for backwards compat?
       })
     } catch (err) {
+      /* istanbul ignore next */
       next(err)
       return
     }
@@ -213,6 +220,7 @@ module.exports = function () {
     try {
       datapackage = await Model.getPackage(req.params.name)
     } catch (err) {
+      /* istanbul ignore next */
       next(err)
       return
     }
@@ -247,6 +255,7 @@ module.exports = function () {
         slug: 'organization'
       })
     } catch (err) {
+      /* istanbul ignore next */
       next(err)
     }
   })
@@ -257,6 +266,13 @@ module.exports = function () {
       // Get owner details
       const owner = req.params.owner
       const profile = await Model.getProfile(owner)
+      // if not a valid profile, send them on the way
+      if (!profile.created) { 
+        return res.status(404).render('404.html', {
+          message: `Page found: ${owner}`,
+          status: 404
+        })
+      }
       const created = new Date(profile.created)
       const joinYear = created.getUTCFullYear()
       const joinMonth = created.toLocaleString('en-us', { month: "long" })
@@ -291,6 +307,7 @@ module.exports = function () {
         currentPage
       })
     } catch(err) {
+      /* istanbul ignore next */
       next(err)
     }
   })

tests/lib/index.test.js

@@ -102,6 +102,17 @@ test('getProfile api works', async t => {
 })
 
 
+test('getProfile api returns empty object on fail', async t => {
+  t.plan(2)
+
+  const result = await DmsModel.getProfile('known-bad')
+  console.log('get bad profile', result)
+
+  // make sure result is an empty object
+  t.is(result.constructor, Object)
+  t.is(Object.entries(result).length, 0)
+})
+
 
 
 test('getCollections (list of collections) api works', async t => {

tests/routes/index.test.js

@@ -85,9 +85,10 @@ test('Theme defined route does NOT exists when THEME is not set', async t => {
   config.set('THEME', 'opendk')
   const app = require('../../index').makeApp()
   const res = await request(app)
-    .get('/absolutely-not-a-chance')
+    .get('/known-bad')
 
-  t.is(res.statusCode, 500)
+  t.is(res.statusCode, 404)
+  t.true(res.text.includes('<!-- placeholder for testing 404 page -->'))
 })
 
 
@@ -302,6 +303,7 @@ test('If a page is not found in neither CMS or DMS, it returns 404 page', async
   let res = await agent
     .get('/not-found-slug')
 
+  t.is(res.status, 404)
   t.true(res.text.includes('<!-- placeholder for testing 404 page -->'))
 
   res = await agent