Skip to content

Commit b8fe760

Browse files
maxbeattymathiasbynens
maxbeatty
authored and
mathiasbynens
committed
Allow removing of code snippet (#407)
1 parent ddef607 commit b8fe760

File tree

10 files changed

+274
-95
lines changed

10 files changed

+274
-95
lines changed

‎server/lib/defaults.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ const regex = require('./regex');
55
const MEDIUM_TEXT_LENGTH = 16777215;
66
exports.mediumTextLength = MEDIUM_TEXT_LENGTH;
77

8+
exports.deleteMe = 'GENERATED_DEFAULT_DELETE_ME';
9+
810
exports.errors = {
911
title: 'You must enter a title for this test case.',
1012
slug: 'The slug can only contain alphanumeric characters and hyphens.',

‎server/lib/schema.js‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,8 @@ exports.testPage = Joi.object().keys({
2121
setup: mediumText,
2222
teardown: mediumText,
2323
test: Joi.array().required().min(2).items(Joi.object().required().keys({
24-
title: Joi.string().required().trim().allow('').empty('').max(255).when('code', {
25-
is: '',
26-
then: Joi.string().length(0), // must be blank too so can delete
27-
otherwise: Joi.string().min(1)
28-
}),
29-
code: Joi.string().required().trim().allow('').max(defaults.mediumTextLength),
24+
title: Joi.string().trim().allow('').empty('').default(defaults.deleteMe).max(255),
25+
code: Joi.string().trim().allow('').empty('').default(defaults.deleteMe).max(defaults.mediumTextLength),
3026
defer: Joi.string().default('n').valid('y', 'n'),
3127
testID: Joi.number().integer().optional() // only present when editing
3228
}))

‎server/repositories/tests.js‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
const defaults = require('../lib/defaults');
2+
13
const name = 'repositories/tests';
24
const table = 'tests';
35

@@ -26,10 +28,12 @@ exports.register = function (server, options, next) {
2628
let queries = [];
2729

2830
tests.forEach(test => {
29-
// setting existing test title and code to blank indicates it should be deleted ...
30-
if (!test.title && !test.code) {
31+
// setting existing test title and code to blank indicates it should be deleted
32+
// Joi converts this to the default ...
33+
if (test.title === defaults.deleteMe && test.code === defaults.deleteMe) {
3134
// ... if it's theirs to delete ...
3235
if (isOwn && test.testID) {
36+
server.log('info', `deleting test ${test.testID} from page ${pageID}`);
3337
queries.push(db.genericQuery(`DELETE FROM ?? WHERE pageID = ${pageID} AND testID = ${test.testID}`, [table]));
3438
}
3539
// ... otherwise skip over the test

‎server/web/edit/index.hbs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,11 @@ This edit will create a new revision.
149149
</fieldset>
150150
{{/each}}
151151
</fieldset>
152+
153+
<p>
154+
<em>NB: leave <code>Title</code> and <code>Code</code> blank to remove code snippet.</em>
155+
</p>
156+
152157
<div class="buttons">
153158
<input type="submit" class="submit" value="Save test case" title="Save and view test case">
154159
</div>

‎server/web/edit/index.js‎

Lines changed: 43 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -79,51 +79,61 @@ exports.register = function (server, options, next) {
7979
};
8080

8181
Joi.validate(request.payload, schema.testPage, function (err, pageWithTests) {
82+
let errObj = {};
8283
if (err) {
83-
let errObj = {};
84+
server.log(['error'], err);
8485
try {
85-
const valErr = err.details[0];
86-
switch (valErr.path) {
87-
case 'title':
88-
errObj.titleError = defaults.errors.title;
89-
break;
90-
default:
91-
const idx = valErr.path.split('.')[1];
92-
switch (valErr.context.key) {
93-
case 'title':
94-
request.payload.test[idx].codeTitleError = defaults.errors.codeTitle;
95-
break;
96-
case 'code':
97-
request.payload.test[idx].codeError = defaults.errors.code;
98-
break;
99-
default:
100-
throw err;
101-
}
86+
if (err.details[0].path === 'title') {
87+
errObj.titleError = defaults.errors.title;
88+
} else {
89+
throw err;
10290
}
10391
} catch (ex) {
10492
server.log(['error'], ex);
10593
errObj.genError = defaults.errors.general;
10694
}
10795
errResp(errObj);
10896
} else {
109-
let isOwn = false;
97+
// additional validation that's not possible or insanely complex w/ Joi
98+
let wasAdditionalValidationError = false;
99+
pageWithTests.test.forEach((t, idx) => {
100+
const missingTitle = t.title === defaults.deleteMe;
101+
const missingCode = t.code === defaults.deleteMe;
110102

111-
pagesService.getBySlug(request.params.testSlug, request.params.rev)
112-
.then(values => {
113-
const prevPage = values[0];
114-
const own = request.yar.get('own') || {};
115-
isOwn = own[prevPage.id];
116-
const isAdmin = request.yar.get('admin');
117-
let update = !!(isAdmin || isOwn);
118-
return pagesService.edit(pageWithTests, update, prevPage.maxRev, prevPage.id);
119-
})
120-
.then(resultingRevision => {
121-
request.yar.set('authorSlug', pageWithTests.author.replace(' ', '-').replace(/[^a-zA-Z0-9 -]/, ''));
103+
if (missingTitle && !missingCode) {
104+
wasAdditionalValidationError = true;
105+
request.payload.test[idx].codeTitleError = defaults.errors.codeTitle;
106+
}
107+
108+
if (missingCode && !missingTitle) {
109+
wasAdditionalValidationError = true;
110+
request.payload.test[idx].codeError = defaults.errors.code;
111+
}
112+
});
122113

123-
const r = resultingRevision > 1 ? `/${resultingRevision}` : '';
114+
if (wasAdditionalValidationError) {
115+
errResp(errObj);
116+
} else {
117+
let isOwn = false;
118+
119+
pagesService.getBySlug(request.params.testSlug, request.params.rev)
120+
.then(values => {
121+
const prevPage = values[0];
122+
const own = request.yar.get('own') || {};
123+
isOwn = own[prevPage.id];
124+
const isAdmin = request.yar.get('admin');
125+
let update = !!(isAdmin || isOwn);
126+
server.log('debug', `isAdmin: ${isAdmin} isOwn: ${isOwn} update: ${update}`);
127+
return pagesService.edit(pageWithTests, update, prevPage.maxRev, prevPage.id);
128+
})
129+
.then(resultingRevision => {
130+
request.yar.set('authorSlug', pageWithTests.author.replace(' ', '-').replace(/[^a-zA-Z0-9 -]/, ''));
124131

125-
reply.redirect(`/${request.params.testSlug}${r}`);
126-
}).catch(errResp);
132+
const r = resultingRevision > 1 ? `/${resultingRevision}` : '';
133+
134+
reply.redirect(`/${request.params.testSlug}${r}`);
135+
}).catch(errResp);
136+
}
127137
}
128138
});
129139
}

‎server/web/home/index.js‎

Lines changed: 42 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,8 @@ exports.register = function (server, options, next) {
5050
};
5151

5252
Joi.validate(request.payload, schema.testPage, function (er, pageWithTests) {
53+
let errObj = {};
5354
if (er) {
54-
var errObj = {};
5555
// `abortEarly` option defaults to `true` so can rely on 0 index
5656
// but just in case...
5757
try {
@@ -65,44 +65,55 @@ exports.register = function (server, options, next) {
6565
errObj.slugError = defaults.errors.slug;
6666
break;
6767
default:
68-
const idx = valErr.path.split('.')[1];
69-
switch (valErr.context.key) {
70-
case 'title':
71-
request.payload.test[idx].codeTitleError = defaults.errors.codeTitle;
72-
break;
73-
case 'code':
74-
request.payload.test[idx].codeError = defaults.errors.code;
75-
break;
76-
default:
77-
throw new Error('unknown validation error');
78-
}
68+
throw new Error('unknown validation error');
7969
}
8070
} catch (ex) {
8171
errObj.genError = defaults.errors.general;
8272
}
8373
errResp(errObj);
8474
} else {
85-
// Joi defaults any properties not present in `request.payload` so use `payload` from here on out
86-
var payload = pageWithTests;
75+
// additional validation that's not possible or insanely complex w/ Joi
76+
let wasAdditionalValidationError = false;
77+
pageWithTests.test.forEach((t, idx) => {
78+
const missingTitle = t.title === defaults.deleteMe;
79+
const missingCode = t.code === defaults.deleteMe;
80+
81+
if (missingTitle && !missingCode) {
82+
wasAdditionalValidationError = true;
83+
request.payload.test[idx].codeTitleError = defaults.errors.codeTitle;
84+
}
85+
86+
if (missingCode && !missingTitle) {
87+
wasAdditionalValidationError = true;
88+
request.payload.test[idx].codeError = defaults.errors.code;
89+
}
90+
});
91+
92+
if (wasAdditionalValidationError) {
93+
errResp(errObj);
94+
} else {
95+
// Joi defaults any properties not present in `request.payload` so use `payload` from here on out
96+
var payload = pageWithTests;
8797

88-
pagesService.checkIfSlugAvailable(server, payload.slug)
89-
.then(isAvail => {
90-
if (!isAvail) {
91-
errResp({
92-
slugError: defaults.errors.slugDupe
93-
});
94-
} else {
95-
return pagesService.create(payload)
96-
.then(resultPageId => {
97-
const own = request.yar.get('own') || {};
98-
own[resultPageId] = true;
99-
request.yar.set('own', own);
100-
request.yar.set('authorSlug', payload.author.replace(' ', '-').replace(/[^a-zA-Z0-9 -]/, ''));
101-
reply.redirect('/' + payload.slug);
98+
pagesService.checkIfSlugAvailable(server, payload.slug)
99+
.then(isAvail => {
100+
if (!isAvail) {
101+
errResp({
102+
slugError: defaults.errors.slugDupe
102103
});
103-
}
104-
})
105-
.catch(errResp);
104+
} else {
105+
return pagesService.create(payload)
106+
.then(resultPageId => {
107+
const own = request.yar.get('own') || {};
108+
own[resultPageId] = true;
109+
request.yar.set('own', own);
110+
request.yar.set('authorSlug', payload.author.replace(' ', '-').replace(/[^a-zA-Z0-9 -]/, ''));
111+
reply.redirect('/' + payload.slug);
112+
});
113+
}
114+
})
115+
.catch(errResp);
116+
}
106117
}
107118
});
108119
}

‎test/unit/server/lib/schema.js‎

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
var Lab = require('lab');
22
var Code = require('code');
3+
const Joi = require('joi');
34
const schema = require('../../../../server/lib/schema');
45

56
var lab = exports.lab = Lab.script();
@@ -21,6 +22,119 @@ lab.experiment('Schema Library', function () {
2122

2223
done();
2324
});
25+
26+
lab.experiment('test array', () => {
27+
let value;
28+
29+
lab.beforeEach((done) => {
30+
value = {
31+
author: 'Pitcher Man',
32+
authorEmail: 'kool-aid@kraft.com',
33+
authorURL: 'http://kool-aid.com',
34+
title: 'oh',
35+
slug: 'wee',
36+
info: '',
37+
initHTML: '',
38+
setup: '',
39+
teardown: ''
40+
// `test` intentionally missing. will be set in each test
41+
};
42+
43+
done();
44+
});
45+
46+
lab.test('title with code is valid', (done) => {
47+
value.test = [
48+
{
49+
title: 't1',
50+
code: 't=1'
51+
},
52+
{
53+
title: 't2',
54+
code: 't=2'
55+
}
56+
];
57+
58+
Joi.validate(value, schema.testPage, (err) => {
59+
Code.expect(err).to.not.exist();
60+
61+
done();
62+
});
63+
});
64+
65+
lab.test('blank title with blank code is valid', (done) => {
66+
value.test = [
67+
{
68+
title: 't1',
69+
code: 't=1'
70+
},
71+
{
72+
title: '', // intentionally blank
73+
code: '' // intentionally blank
74+
},
75+
{
76+
title: 't3',
77+
code: 't=3'
78+
}
79+
];
80+
81+
Joi.validate(value, schema.testPage, (err, res) => {
82+
Code.expect(err).to.not.exist();
83+
Code.expect(res.test[1].title).to.equal('GENERATED_DEFAULT_DELETE_ME');
84+
Code.expect(res.test[1].code).to.equal('GENERATED_DEFAULT_DELETE_ME');
85+
86+
done();
87+
});
88+
});
89+
90+
lab.test('blank title with code is invalid', (done) => {
91+
value.test = [
92+
{
93+
title: 't1',
94+
code: 't=1'
95+
},
96+
{
97+
title: 't2',
98+
code: 't=2'
99+
},
100+
{
101+
title: '', // intentionally blank
102+
code: 't=3'
103+
}
104+
];
105+
106+
Joi.validate(value, schema.testPage, (err, res) => {
107+
Code.expect(err).to.not.exist();
108+
Code.expect(res.test[2].title).to.equal('GENERATED_DEFAULT_DELETE_ME');
109+
110+
done();
111+
});
112+
});
113+
114+
lab.test('title with blank code is invalid', (done) => {
115+
value.test = [
116+
{
117+
title: 't1',
118+
code: 't=1'
119+
},
120+
{
121+
title: 't2',
122+
code: 't=2'
123+
},
124+
{
125+
title: 't3',
126+
code: '' // intentionally blank
127+
}
128+
];
129+
130+
Joi.validate(value, schema.testPage, (err, res) => {
131+
Code.expect(err).to.not.exist();
132+
Code.expect(res.test[2].code).to.equal('GENERATED_DEFAULT_DELETE_ME');
133+
134+
done();
135+
});
136+
});
137+
});
24138
});
25139

26140
lab.experiment('comment', function () {

0 commit comments

Comments
 (0)