Skip to content

allow removing of code snippet #407

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 11, 2017
Merged

allow removing of code snippet #407

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
eb344c5
allow removing of code snippet
maxbeatty Jul 10, 2017
0d1f238
prove fix works w/ tests
maxbeatty Jul 10, 2017
414c2ff
Merge branch 'master' into rm-snippet
mathiasbynens Jul 11, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions server/lib/defaults.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ const regex = require('./regex');
const MEDIUM_TEXT_LENGTH = 16777215;
exports.mediumTextLength = MEDIUM_TEXT_LENGTH;

exports.deleteMe = 'GENERATED_DEFAULT_DELETE_ME';

exports.errors = {
title: 'You must enter a title for this test case.',
slug: 'The slug can only contain alphanumeric characters and hyphens.',
Expand Down
8 changes: 2 additions & 6 deletions server/lib/schema.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,8 @@ exports.testPage = Joi.object().keys({
setup: mediumText,
teardown: mediumText,
test: Joi.array().required().min(2).items(Joi.object().required().keys({
title: Joi.string().required().trim().allow('').empty('').max(255).when('code', {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

required and allow('') apparently don't mix

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hah! Good catch!

is: '',
then: Joi.string().length(0), // must be blank too so can delete
otherwise: Joi.string().min(1)
}),
code: Joi.string().required().trim().allow('').max(defaults.mediumTextLength),
title: Joi.string().trim().allow('').empty('').default(defaults.deleteMe).max(255),
code: Joi.string().trim().allow('').empty('').default(defaults.deleteMe).max(defaults.mediumTextLength),
defer: Joi.string().default('n').valid('y', 'n'),
testID: Joi.number().integer().optional() // only present when editing
}))
Expand Down
8 changes: 6 additions & 2 deletions server/repositories/tests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
const defaults = require('../lib/defaults');

const name = 'repositories/tests';
const table = 'tests';

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

tests.forEach(test => {
// setting existing test title and code to blank indicates it should be deleted ...
if (!test.title && !test.code) {
// setting existing test title and code to blank indicates it should be deleted
// Joi converts this to the default ...
if (test.title === defaults.deleteMe && test.code === defaults.deleteMe) {
// ... if it's theirs to delete ...
if (isOwn && test.testID) {
server.log('info', `deleting test ${test.testID} from page ${pageID}`);
queries.push(db.genericQuery(`DELETE FROM ?? WHERE pageID = ${pageID} AND testID = ${test.testID}`, [table]));
}
// ... otherwise skip over the test
Expand Down
5 changes: 5 additions & 0 deletions server/web/edit/index.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,11 @@ This edit will create a new revision.
</fieldset>
{{/each}}
</fieldset>

<p>
<em>NB: leave <code>Title</code> and <code>Code</code> blank to remove code snippet.</em>
</p>

<div class="buttons">
<input type="submit" class="submit" value="Save test case" title="Save and view test case">
</div>
Expand Down
76 changes: 43 additions & 33 deletions server/web/edit/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,51 +79,61 @@ exports.register = function (server, options, next) {
};

Joi.validate(request.payload, schema.testPage, function (err, pageWithTests) {
let errObj = {};
if (err) {
let errObj = {};
server.log(['error'], err);
try {
const valErr = err.details[0];
switch (valErr.path) {
case 'title':
errObj.titleError = defaults.errors.title;
break;
default:
const idx = valErr.path.split('.')[1];
switch (valErr.context.key) {
case 'title':
request.payload.test[idx].codeTitleError = defaults.errors.codeTitle;
break;
case 'code':
request.payload.test[idx].codeError = defaults.errors.code;
break;
default:
throw err;
}
if (err.details[0].path === 'title') {
errObj.titleError = defaults.errors.title;
} else {
throw err;
}
} catch (ex) {
server.log(['error'], ex);
errObj.genError = defaults.errors.general;
}
errResp(errObj);
} else {
let isOwn = false;
// additional validation that's not possible or insanely complex w/ Joi
let wasAdditionalValidationError = false;
pageWithTests.test.forEach((t, idx) => {
const missingTitle = t.title === defaults.deleteMe;
const missingCode = t.code === defaults.deleteMe;

pagesService.getBySlug(request.params.testSlug, request.params.rev)
.then(values => {
const prevPage = values[0];
const own = request.yar.get('own') || {};
isOwn = own[prevPage.id];
const isAdmin = request.yar.get('admin');
let update = !!(isAdmin || isOwn);
return pagesService.edit(pageWithTests, update, prevPage.maxRev, prevPage.id);
})
.then(resultingRevision => {
request.yar.set('authorSlug', pageWithTests.author.replace(' ', '-').replace(/[^a-zA-Z0-9 -]/, ''));
if (missingTitle && !missingCode) {
wasAdditionalValidationError = true;
request.payload.test[idx].codeTitleError = defaults.errors.codeTitle;
}

if (missingCode && !missingTitle) {
wasAdditionalValidationError = true;
request.payload.test[idx].codeError = defaults.errors.code;
}
});

const r = resultingRevision > 1 ? `/${resultingRevision}` : '';
if (wasAdditionalValidationError) {
errResp(errObj);
} else {
let isOwn = false;

pagesService.getBySlug(request.params.testSlug, request.params.rev)
.then(values => {
const prevPage = values[0];
const own = request.yar.get('own') || {};
isOwn = own[prevPage.id];
const isAdmin = request.yar.get('admin');
let update = !!(isAdmin || isOwn);
server.log('debug', `isAdmin: ${isAdmin} isOwn: ${isOwn} update: ${update}`);
return pagesService.edit(pageWithTests, update, prevPage.maxRev, prevPage.id);
})
.then(resultingRevision => {
request.yar.set('authorSlug', pageWithTests.author.replace(' ', '-').replace(/[^a-zA-Z0-9 -]/, ''));

reply.redirect(`/${request.params.testSlug}${r}`);
}).catch(errResp);
const r = resultingRevision > 1 ? `/${resultingRevision}` : '';

reply.redirect(`/${request.params.testSlug}${r}`);
}).catch(errResp);
}
}
});
}
Expand Down
73 changes: 42 additions & 31 deletions server/web/home/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ exports.register = function (server, options, next) {
};

Joi.validate(request.payload, schema.testPage, function (er, pageWithTests) {
let errObj = {};
if (er) {
var errObj = {};
// `abortEarly` option defaults to `true` so can rely on 0 index
// but just in case...
try {
Expand All @@ -65,44 +65,55 @@ exports.register = function (server, options, next) {
errObj.slugError = defaults.errors.slug;
break;
default:
const idx = valErr.path.split('.')[1];
switch (valErr.context.key) {
case 'title':
request.payload.test[idx].codeTitleError = defaults.errors.codeTitle;
break;
case 'code':
request.payload.test[idx].codeError = defaults.errors.code;
break;
default:
throw new Error('unknown validation error');
}
throw new Error('unknown validation error');
}
} catch (ex) {
errObj.genError = defaults.errors.general;
}
errResp(errObj);
} else {
// Joi defaults any properties not present in `request.payload` so use `payload` from here on out
var payload = pageWithTests;
// additional validation that's not possible or insanely complex w/ Joi
let wasAdditionalValidationError = false;
pageWithTests.test.forEach((t, idx) => {
const missingTitle = t.title === defaults.deleteMe;
const missingCode = t.code === defaults.deleteMe;

if (missingTitle && !missingCode) {
wasAdditionalValidationError = true;
request.payload.test[idx].codeTitleError = defaults.errors.codeTitle;
}

if (missingCode && !missingTitle) {
wasAdditionalValidationError = true;
request.payload.test[idx].codeError = defaults.errors.code;
}
});

if (wasAdditionalValidationError) {
errResp(errObj);
} else {
// Joi defaults any properties not present in `request.payload` so use `payload` from here on out
var payload = pageWithTests;

pagesService.checkIfSlugAvailable(server, payload.slug)
.then(isAvail => {
if (!isAvail) {
errResp({
slugError: defaults.errors.slugDupe
});
} else {
return pagesService.create(payload)
.then(resultPageId => {
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 -]/, ''));
reply.redirect('/' + payload.slug);
pagesService.checkIfSlugAvailable(server, payload.slug)
.then(isAvail => {
if (!isAvail) {
errResp({
slugError: defaults.errors.slugDupe
});
}
})
.catch(errResp);
} else {
return pagesService.create(payload)
.then(resultPageId => {
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 -]/, ''));
reply.redirect('/' + payload.slug);
});
}
})
.catch(errResp);
}
}
});
}
Expand Down
114 changes: 114 additions & 0 deletions test/unit/server/lib/schema.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
var Lab = require('lab');
var Code = require('code');
const Joi = require('joi');
const schema = require('../../../../server/lib/schema');

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

done();
});

lab.experiment('test array', () => {
let value;

lab.beforeEach((done) => {
value = {
author: 'Pitcher Man',
authorEmail: 'kool-aid@kraft.com',
authorURL: 'http://kool-aid.com',
title: 'oh',
slug: 'wee',
info: '',
initHTML: '',
setup: '',
teardown: ''
// `test` intentionally missing. will be set in each test
};

done();
});

lab.test('title with code is valid', (done) => {
value.test = [
{
title: 't1',
code: 't=1'
},
{
title: 't2',
code: 't=2'
}
];

Joi.validate(value, schema.testPage, (err) => {
Code.expect(err).to.not.exist();

done();
});
});

lab.test('blank title with blank code is valid', (done) => {
value.test = [
{
title: 't1',
code: 't=1'
},
{
title: '', // intentionally blank
code: '' // intentionally blank
},
{
title: 't3',
code: 't=3'
}
];

Joi.validate(value, schema.testPage, (err, res) => {
Code.expect(err).to.not.exist();
Code.expect(res.test[1].title).to.equal('GENERATED_DEFAULT_DELETE_ME');
Code.expect(res.test[1].code).to.equal('GENERATED_DEFAULT_DELETE_ME');

done();
});
});

lab.test('blank title with code is invalid', (done) => {
value.test = [
{
title: 't1',
code: 't=1'
},
{
title: 't2',
code: 't=2'
},
{
title: '', // intentionally blank
code: 't=3'
}
];

Joi.validate(value, schema.testPage, (err, res) => {
Code.expect(err).to.not.exist();
Code.expect(res.test[2].title).to.equal('GENERATED_DEFAULT_DELETE_ME');

done();
});
});

lab.test('title with blank code is invalid', (done) => {
value.test = [
{
title: 't1',
code: 't=1'
},
{
title: 't2',
code: 't=2'
},
{
title: 't3',
code: '' // intentionally blank
}
];

Joi.validate(value, schema.testPage, (err, res) => {
Code.expect(err).to.not.exist();
Code.expect(res.test[2].code).to.equal('GENERATED_DEFAULT_DELETE_ME');

done();
});
});
});
});

lab.experiment('comment', function () {
Expand Down
Loading