Skip to content
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.

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

Improved server side JSON validation #188

Merged
merged 4 commits into from
Jan 13, 2017
Merged

Conversation

mbarton
Copy link
Contributor

@mbarton mbarton commented Jan 11, 2017

Refactored out JSON validation in Api2 into a single method, which returns a BadRequest response detailing the errors.

For example, missing out fields in the create UI no longer causes a 500 but returns a list of the missing fields:

/posterImage -> ValidationError(List(error.path.missing),WrappedArray())
/youtubeCategoryId -> ValidationError(List(error.path.missing),WrappedArray())
/channelId -> ValidationError(List(error.path.missing),WrappedArray())

Also update the front-end to clear out saving/publishing/searching state when an error occurs. This stops the save button from spinning forever.

NB: the error still shows up in red at the top forever (#187)

}

val updatedAtom = PublishAtomCommand(id).process()
Ok(Json.toJson(updatedAtom))
Copy link
Member

@akash1810 akash1810 Jan 13, 2017

Choose a reason for hiding this comment

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

as we've removed the try/catch what happens when PublishAtomCommand(id).process() throws? Do we get a 500 and a sensible message still?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops sorry. I've put back the try/catch

}
}

def getAuditTrailForAtomId(id: String) = APIHMACAuthAction { implicit req =>
Ok(Json.toJson(auditDataStore.getAuditTrailForAtomId(id)))
}

private def parse[T](raw: UserRequest[AnyContent])(fn: T => Result)(implicit reads: Reads[T]): Result = try {
Copy link
Member

Choose a reason for hiding this comment

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

nice

@@ -2,23 +2,23 @@ package controllers

import javax.inject.Inject

import _root_.util.{AWSConfig, ExpiryPoller, YouTubeConfig, YouTubeVideoUpdateApi}
Copy link
Member

Choose a reason for hiding this comment

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

_root_ silly intellij!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah. I double checked it was like that before I changed things

@mbarton mbarton merged commit 65b6e1e into master Jan 13, 2017
@mbarton mbarton deleted the mbarton/create-validation-fix branch January 13, 2017 12:37
@prout-bot
Copy link

Seen on PROD (merged by @mbarton 334 hours, 57 minutes and 1 second ago) Please check your changes!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants