Refactor Post validation

src/api/controller.js

@@ -40,7 +40,8 @@ import {
   GeotagValidator,
   UserMessageValidator,
   SearchQueryValidator,
-  ProfilePostValidator
+  ProfilePostValidator,
+  PostValidator
 } from './validators';
 
 const SUPPORTED_LOCALES = Object.keys(
@@ -1630,8 +1631,15 @@ export default class ApiController {
     const Post = this.bookshelf.model('Post');
 
     try {
+      const validationResult = PostValidator.validate(ctx.request.body);
+      if (validationResult.error) {
+        this.processError(ctx, validationResult.error);
+        return;
+      }
+
+      const attributes = validationResult.value;
       const post = await Post.create({
-        ...ctx.request.body,
+        ...attributes,
         user_id: ctx.session.user
       });
       await post.refresh({ require: true, withRelated: POST_RELATIONS });

src/api/db/index.js

@@ -245,30 +245,18 @@ export function initBookshelfFromKnex(knex) {
       const type = this.get('type');
 
       // text_source
-      if (_.isString(data.text_source)) {
-        this.set('text_source', data.text_source);
-      } else {
-        throw new Error('"text_source" must be present');
-      }
+      this.set('text_source', data.text_source);
 
       // text_type
       if (type === 'story') {
-        if (_.isString(data.text_type) && _.includes(['markdown', 'html'], data.text_type)) {
-          this.set('text_type', data.text_type);
-        } else {
-          throw new Error('"text_type" must be equeal to either "markdown" or "html"');
-        }
+        this.set('text_type', data.text_type);
       }
 
       // title
       if (type === 'long_text') {
-        if (_.isString(data.title)) {
-          const more = this.get('more') || {};
-          more.title = data.title;
-          this.set('more', more);
-        } else {
-          throw new Error('"title" must be present');
-        }
+        const more = this.get('more') || {};
+        more.title = data.title;
+        this.set('more', more);
       } else if (type === 'story' && _.isString(data.title)) { // title is optional for story posts
         const more = this.get('more') || {};
         more.title = data.title;
@@ -538,10 +526,6 @@ export function initBookshelfFromKnex(knex) {
    */
   Post.create = async (data) => {
     return bookshelf.transaction(async (t) => {
-      if (!_.includes(Post.typesWithPages, data.type)) {
-        throw new Error(`${data.type}" type is not supported`);
-      }
-
       const post = new Post({
         id: uuid.v4(),
         user_id: data.user_id,
@@ -572,31 +556,15 @@ export function initBookshelfFromKnex(knex) {
 
       // Relations
       if (data.hashtags) {
-        if (_.isArray(data.hashtags)) {
-          if (data.hashtags.filter(tag => (countBreaks(tag) < 3)).length > 0) {
-            throw new Error('each of tags should be at least 3 characters wide');
-          }
-
-          await post.attachHashtags(_.uniq(data.hashtags), { transacting: t });
-        } else {
-          throw new Error('"hashtags" parameter is expected to be an array');
-        }
+        await post.attachHashtags(_.uniq(data.hashtags), { transacting: t });
       }
 
       if (data.schools) {
-        if (_.isArray(data.schools)) {
-          await post.attachSchools(_.uniq(data.schools), { transacting: t });
-        } else {
-          throw new Error('"schools" parameter is expected to be an array');
-        }
+        await post.attachSchools(_.uniq(data.schools), { transacting: t });
       }
 
       if (data.geotags) {
-        if (_.isArray(data.geotags)) {
-          await post.attachGeotags(_.uniq(data.geotags), { transacting: t });
-        } else {
-          throw new Error('"geotags" parameter is expected to be an array');
-        }
+        await post.attachGeotags(_.uniq(data.geotags), { transacting: t });
       }
 
       return post;

src/api/validators.js

@@ -120,3 +120,21 @@ export const SearchQueryValidator = Joi.object({
   sort: Joi.string().only(SEARCH_SORTING_TYPES),
   q: Joi.string().required()
 }).options({ stripUnknown: true });
+
+export const PostValidator = Joi.object({
+  type: Joi.string().only('short_text', 'long_text', 'story').required(),
+  text_source: Joi.string().required(),
+  text_type: Joi.string().when('type', {
+    is: 'story',
+    then: Joi.only('html', 'markdown').required(),
+    otherwise: Joi.forbidden()
+  }),
+  title: Joi.alternatives().when('type', {
+    is: ['story', 'long_post'],
+    then: Joi.string().min(1),
+    otherwise: Joi.forbidden()
+  }),
+  hashtags: Joi.array().items(Joi.string().min(3)),
+  schools: Joi.array().items(Joi.string()),
+  geotags: Joi.array().items(Joi.string())
+}).options({ abortEarly: false, stripUnknown: true });

test/integration/api/post.js

@@ -164,7 +164,7 @@ describe('Post', () => {
     });
 
     describe('POST /api/v1/posts', () => {
-      it('creates a post and copies text_source into text without processing', async () => {
+      it("doesn't allow text_type to be present for non-story posts", async () => {
         await expect(
           {
             session: sessionId,
@@ -177,11 +177,7 @@ describe('Post', () => {
               title: 'Title'
             }
           },
-          'body to satisfy',
-          {
-            text_source: '# header',
-            text: '# header'
-          }
+          'not to open'
         );
       });
 

test/unit/controllers/validators.js

@@ -47,3 +47,66 @@ describe('UserRegistrationValidator', () => {
     });
   });
 });
+
+describe('PostValidator', () => {
+  it("doesn't allow `text_type` when `type` is not 'story'", async () => {
+    const attributes = {
+      type: 'short_text',
+      text_type: 'html',
+      text_source: '123',
+    };
+
+    const result = PostValidator.validate(attributes);
+    expect(result.error.details, 'to satisfy', [
+      { path: 'text_type', type: 'any.unknown' },
+    ]);
+  });
+
+  it('fails when no attributes are provided', async () => {
+    const attributes = {};
+
+    const result = PostValidator.validate(attributes);
+    expect(result.error.details, 'to satisfy', [
+      { path: 'type', type: 'any.required' },
+      { path: 'text_source', type: 'any.required' },
+    ]);
+  });
+
+  it('fails on invalid attributes', () => {
+    const attributes = {
+      type: 'invalid',
+      text_source: '',
+      text_type: 'invalid',
+      title: 'A title',
+      hashtags: 123,
+      schools: 123,
+      geotags: 123,
+    };
+
+    const result = PostValidator.validate(attributes);
+    expect(result.error.details, 'to satisfy', [
+      { path: 'type', type: 'any.allowOnly' },
+      { path: 'text_source', type: 'any.empty' },
+      { path: 'text_type', type: 'any.unknown' },
+      { path: 'title', type: 'any.unknown' },
+      { path: 'hashtags', type: 'array.base' },
+      { path: 'schools', type: 'array.base' },
+      { path: 'geotags', type: 'array.base' },
+    ]);
+  });
+
+  it('succeeds when all attributes are valid', async () => {
+    const attributes = {
+      type: 'story',
+      text_source: '<h1>Test</test>',
+      text_type: 'html',
+      title: 'A title',
+      hashtags: ['One', 'Two'],
+      schools: ['One', 'Two'],
+      geotags: ['One', 'Two'],
+    };
+
+    const result = PostValidator.validate(attributes);
+    expect(result.error, 'to be falsy');
+  });
+});