8000 Add check for comments that exceed DB col size by salcode · Pull Request #2858 · WP-API/WP-API · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content
This repository was archived by the owner on Sep 24, 2018. It is now read-only.

Add check for comments that exceed DB col size #2858

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

Add check for comments that exceed DB col size #2858

wants to merge 2 commits into from

Conversation

salcode
Copy link
@salcode salcode commented Oct 18, 2016

Add checks for these fields exceeding the DB col size and tests for each
field:

  • comment_author
  • comment_author_email
  • comment_author_url
  • comment_content

See #2785

@salcode
Add check for comments that exceed DB col size
f1d64ce 
Add checks for these fields exceeding the DB col size and tests for each
field:

- comment_author
- comment_author_email
- comment_author_url
- comment_content

See #2785
@salcode
Copy link
Author
salcode commented Oct 18, 2016

CONTRIBUTING.md asks that I leave a #reviewmerge comment, which I'm not familiar with. Hopefully, this comment with the hash tag is sufficient. If not, I'd appreciate any guidance (or a relevant link) on how to add this properly.

Thanks.

@rachelbaker
Copy link
Member

@salcode What do you think about moving this into the prepare_item_for_database() method so we are covered on create & update actions?

@rachelbaker
Copy link
Member

This PR would also benefit from a few unit tests.

@rachelbaker rachelbaker added this to the 2.0 milestone Oct 19, 2016
@schlessera
Copy link
Member

@rachelbaker, the original PR already includes the corresponding unit tests.

@salcode
Move DB col size check to prepare_item_for_database()
cedcab2 
Move check for comments that exceed DB col size from method
create_item() to prepare_item_for_database().

This allows the check to be performed on both comment create and update
rather than just on create.

See #2785
See #2858
@salcode
Copy link
Author
salcode commented Oct 20, 2016

@rachelbaker thanks for your suggestion. I've moved the DB column size check to prepare_item_for_database() so as to cover both create and update.

Currently, there are unit tests for create. Am I correct you're suggesting we add tests to cover update?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
Needs Refresh
Projects
None yet
Milestone
2.0
3E00
Development

Successfully merging this pull request may close these issues.
3 participants
@salcode @rachelbaker @schlessera
0