When enabling Profile Complete Percent, I get the following error:

PDOException: SQLSTATE[22001]: String data, right truncated: 7 ERROR: value too long for type character varying(32): INSERT INTO drupal_block (module, delta, theme, status, weight, region, pages) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2, :db_insert_placeholder_3, :db_insert_placeholder_4, :db_insert_placeholder_5, :db_insert_placeholder_6); Array ( ) in drupal_write_record() (regel 6975 van /home/peter/src/drupal7-testcases/includes/common.inc).

This is due to one of my profile2 types, which has a very long name. The "profile" table has a "type" column of type VARCHAR(32), which holds the "machine name" of the profile type. The "block" table has a delta column which is also VARCHAR(32), and PCP prefixes the name with "pcp_profile2_", thus lengthening the field potentially beyond 32 characters.

I'm running PostgreSQL and it complains early (immediately when enabling pcp). I'm pretty sure if you run it on MySQL, it will silently truncate the delta name on installation, and this will later cause pcp_block_view() to fail with a warning like Undefined variable: block in pcp_block_view() (line 133 of /path/to/pcp/pcp.module) because the comparison if ($delta == 'pcp_profile2_' . $bundle) will fail, since $delta holds the truncated version of the name.

Attached is a simple fix: it md5-encodes the name and uses that as the delta (this trick is also used by ctools in some places AFAICT). Alternatively, the delta could simply use the profile2-name as-is, but that could clash with any other blocks pcp might declare in the future (theoretically it could conflict with pcp_profile_percent_complete if someone names their profile2 type like that, but I admit that's not too likely). Shortening and prefixing the name could also work but also potentially causes name clashes if there's a long shared prefix (or suffix, if you chop off the end) in profile types.

I decided to keep the "pcp_profile2_TYPE_hide_block_on_complete" variables since they can be longer than 32 characters (128, in fact) and variables must be globally unique. Also, the deinstallation procedure uses LIKE 'pcp_%' to delete its variables and it's nice to have descriptive variable names.

I'm unsure whether a database upgrade procedure is necessary - as soon as a user visits the "blocks" page, the hook_block_info() is invoked and the table repopulated. This means that any old blocks are no longer available and an upgrade won't work anymore. On the other hand, if the user doesn't visit the blocks page there will probably be warnings and notices.

CommentFileSizeAuthor
0001-Ensure-block-delta-name-will-fit-in-varchar-32-by-md.patch1.55 KBPeter Bex

Comments

BarisW’s picture

BarisW commented

Thanks for the patch and good find.

We might use the same method as FacetAPI does: it only md5's the block name if its longer than 32 chars.
By doing so, we don't need an upgrade script as current users won't see a difference.

Agreed?

Peter Bex’s picture

Peter Bex commented

Perfect!

BarisW’s picture

BarisW commented
Status: Needs review » Fixed

Thanks. I've added a small helper function to fetch the block system name: 

<?php
function _pcp_block_identifier($bundle) {
  $block_name = 'pcp_profile2_' . $bundle;
  if (strlen($block_name) > 32) {
    $block_name = md5($block_name);
  }
  return $block_name;
}
?>

Committed to .dev

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