mozilla / api.webmaker.org

Services for Webmaker
https://api.webmaker.org
17 stars 14 forks source link

Allow empty descriptions #197

Closed xmatthewx closed 8 years ago

xmatthewx commented 8 years ago

Let's not require descriptions on projects. As currently set, a user who edits their title without entering a description gets a silent failure. This prevents naming simple projects that don't really need a description.

ryanwarsaw commented 8 years ago

@xmatthewx Just to clarify on this in-depth, what I meant was that the parameter description within the patch request, can't be an empty string because it'll return a 400 bad request. When we update the project title, we make a call to the API using this patch request and also update the description parameter with its still empty string value.

Basically, we update the project title and description at the same time, but this is only an issue because the update of the description causes a 400 bad request response. The patch request doesn't require a description, we just update it anyways which can be a good thing.

If we attach code on webmaker-core that looks for an empty string and then skips updating that description with api.webmaker.org, we would essentially inverse the issue and it would be impossible to delete your entire description, because it wouldn't be updated (due to the new code attached).

I would be more than glad to take this ticket if @cadecairos doesn't mind, it would also give me a chance to familiarize myself with the this API, and learn more about how things are working in the background once they get sent out.

xmatthewx commented 8 years ago

Thanks @ryanwse. :+1: from me if @cadecairos is available to review your work.

Is this a correct summary?

ryanwarsaw commented 8 years ago

@xmatthewx Correct. I freshly extracted this from a local instance of the API, can confirm the issue is on the validation side and I am working on filing a PR with the fix soon.

"payload":{"statusCode":400,"error":"Bad Request","message":"child "description" fails because ["description" is not allowed to be empty]"

xmatthewx commented 8 years ago

Excellent. @cadecairos - good to push to master? If so, let me know when it lands and I'll test it on device.

Thanks @ryanwse !!

cadecairos commented 8 years ago

@xmatthewx deployed to production

xmatthewx commented 8 years ago

works like a charm :smiley: