After #1497374: Switch from Field-based storage to Entity-based storage, field names don't have to be unique across entity types.
No need to prefix the 'block_body' field to differentiate it from node 'body' anymore...

Files: 
CommentFileSizeAuthor
#9 2083721-block_body-rename-9.patch10.22 KBlongwave
PASSED: [[SimpleTest]]: [MySQL] 58,853 pass(es).
[ View ]
#2 renamed-block-body-field-2083721-1.patch10.45 KBbenjy
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch renamed-block-body-field-2083721-1.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Comments

yched’s picture

Same for the label, could be just 'Body' ? (well, not that we had constraints on label like we had on field name so far...)

benjy’s picture

Status:Active» Needs review
StatusFileSize
new10.45 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch renamed-block-body-field-2083721-1.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

I just did a find and replace, see what the bot thinks

dsnopek’s picture

It looks like the patch in #2 changes a pre-existing update function: block_update_8008(). Shouldn't it instead add a new update function which renames 'block_body' to 'body'? That way people who already migrated through 8008 will get the update too. I don't really know the policy on this for core development, though, so that's an actual question. :-)

yched’s picture

thanks @dsnopek!

Shouldn't it instead add a new update function which renames 'block_body' to 'body'?

No, we don't support HEAD to HEAD updates while we're still in alpha phase.

Would need an approval from the "Block API" folks, but this looks good to me.

larowlan’s picture

+1 from me

yched’s picture

yched’s picture

Status:Needs review» Reviewed & tested by the community

As per #5, the maintainer of custom_block.module approves, so this is RTBC if it still passes tests.

Status:Reviewed & tested by the community» Needs work

The last submitted patch, renamed-block-body-field-2083721-1.patch, failed testing.

longwave’s picture

Status:Needs work» Needs review
StatusFileSize
new10.22 KB
PASSED: [[SimpleTest]]: [MySQL] 58,853 pass(es).
[ View ]

Rerolled.

yched’s picture

Status:Needs review» Reviewed & tested by the community

Thanks! RTBC if green.

alexpott’s picture

Status:Reviewed & tested by the community» Fixed

Committed af6cd3d and pushed to 8.x. Thanks!

effulgentsia’s picture

Great! @yched: should we do the same for comment_body?

larowlan’s picture

comment_body would require an upgrade path as its an existing field in D7, so perhaps not worth the hassle?

yched’s picture

+1 on renaming comment_body as well, but yes, the upgrade path is not going to be trivial...

swentel’s picture

Can't we more or less ignore that now ? Or is this even going to be hard with migrate ?

yched’s picture

Not sure what are the current practices for stuff that "would need an upgrade" while migrate is being figured out.
The upgrade here would mean:
- changing the field name in field and instance CMI records
- updating entity displays
- renaming db tables & columns
- D7 views are not strictly our problem, but would end up broken

larowlan’s picture

In terms of an upgrade path, is there any reason we couldn't leave the old fields marked as comment_body but create any new ones as body?
The places where the field name is referenced in core (other than tests)
rdf.module
standard profile's comment rdf mapping
recent comments view
comment tokens
comment admin page (although this is wrapped in a check to see if it exists first so irrelevant and only used for the truncated title attribute)
comment manager - but this is for creating the field so would update.

Of those I think the tokens, rdf module and recent comments view are troublesome. But do they warrant an upgrade path?

Status:Fixed» Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.