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

Do not assume nodes of all types have 'body' fields #263

Closed
wants to merge 1 commit into from
Closed

Do not assume nodes of all types have 'body' fields #263

wants to merge 1 commit into from

Conversation

bartfeenstra
Copy link
Contributor

All nodes have titles and types, but not all of them have bodies. This PR removes the assumption that this is so.

@jhedstrom
Copy link
Owner

This makes sense. The failing tests that rely on there being a body field will need to be updated I think.

@jhedstrom jhedstrom added this to the 3.2.0 release milestone Mar 26, 2016
@bartfeenstra
Copy link
Contributor Author

I stopped using these methods since they're hard to work with, and switched to creating entities using the Entity API directly. Therefore I cannot spend time on updating the tests for this PRs.

@jhedstrom jhedstrom modified the milestones: 3.2.0 release, 4.0 release Jun 16, 2016
@jonathanjfshaw
Copy link
Contributor

I don't think there are any failing tests here, just outdated travis build failures. Should be green if tests are rerun.

Another reason why this patch is good is that the previous approach doesn't set the text format for the body field, so you end up with a blank format value which no node created in normal site use would have. This bit me today.

@pfrenssen
Copy link
Collaborator

Good point, rebased this on the latest master branch and trying a new test run in #365

I started a new PR since @bartfeenstra has indicated he doesn't have the time to follow up on this. Would still like to have this in though.

Let's continue in #365,

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

Successfully merging this pull request may close these issues.

4 participants