diff --git a/fixtures/index.js b/fixtures/index.js index f2d3ab2e..5ec21c4b 100644 --- a/fixtures/index.js +++ b/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}) diff --git a/lib/dms.js b/lib/dms.js index 0dba73e0..558f1b7b 100644 --- a/lib/dms.js +++ b/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) { diff --git a/routes/dms.js b/routes/dms.js index 301a7b8b..2154e255 100644 --- a/routes/dms.js +++ b/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) } }) diff --git a/tests/lib/index.test.js b/tests/lib/index.test.js index 51abab8f..bda8fcf7 100644 --- a/tests/lib/index.test.js +++ b/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 => { diff --git a/tests/routes/index.test.js b/tests/routes/index.test.js index 19a64cd8..53e86457 100644 --- a/tests/routes/index.test.js +++ b/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('')) }) @@ -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('')) res = await agent