The size of the email field was increased in [#502968: Increase maximum allowable length of email addresses] however as init is set to the email entered by a user before a new user is created email address above 64 characters still can not be entered. This patch updates init to be 254 the same as email address to allow for email address above 64 characters to be entered.

Marking as critical as otherwise new feature introduced in above issue continues to be useless.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

andypost’s picture

FileSize
1.11 KB

Do not use hook_schema() from within hook_update_N() http://drupal.org/node/150220

So both fields' changed are placed within user_update_7005()

marcingy’s picture

Status: Needs review » Needs work

Patch in #1 doesn't change the size of the field and doesn't change the init field size in hook_schema

marcingy’s picture

Status: Needs work » Needs review
FileSize
1.33 KB

Reroll that updates field length and incorporates the changes in #1.

andypost’s picture

Status: Needs review » Reviewed & tested by the community

@marcingy Yes, I forget about this change.

patch is trivial so RTBC

andypost’s picture

FileSize
1.46 KB

Same patch but doc-block comment trimmed to standard.

andypost’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
1.47 KB

Grrr wrong patch...

marcingy’s picture

Status: Needs review » Reviewed & tested by the community

New doc block looks good not sure if we second a opinion but I'll mark it rbtc on account @andypost was gave it the thumbs up in #4.

andypost’s picture

@marcingy About #5 read more at Coding standards and about Doxygen http://drupal.org/node/1354#functions

andypost’s picture

Status: Reviewed & tested by the community » Needs work

Also it's could be great if you add comment about why 254 chars was chosen from #502968: Increase maximum allowable length of email addresses

marcingy’s picture

See the original comment for the issue clearly states why. And note the original doxygen was reviewed and accepted as part of an earlier patch hence my reason for not amending it.

marcingy’s picture

Status: Needs work » Needs review
aspilicious’s picture

+ * Change the users table to allow longer email addresses.

Please change the verb to "changes".
I know there are a lot of places in core not following the standard but we try to do our best with new patches.

Dries’s picture

I'm OK with this change. In Drupal, we tend to write 'e-mail' instead of 'email' though.

Damien Tournoud’s picture

Status: Needs review » Needs work

While we are at it: db_change_field() mandate that you drop and recreate the indexes on the changed fields. We have an index on 'mail', so the code should read:

  db_drop_index('users', 'mail');
  $mail_field = array(
    'type' => 'varchar',
    'length' => 254,
    'not null' => FALSE,
    'default' => '',
    'description' => "User's email address.",
  );
  db_change_field('users', 'mail', 'mail', $mail_field, array('indexes' => array('mail' => array('mail'))));
marcingy’s picture

Status: Needs work » Needs review
FileSize
1.92 KB

New version taking into account comments above
* Fixes verbage
* amends email to e-mail
* adds a reference to the original discussion node
* creates the index in hook update

aspilicious’s picture

If you going to add the reference, you have to put a newline between the first sentence of the docblock and the see

+ * Changes the users table to allow longer e-mail addresses.
+ * See http://drupal.org/node/502968 for further discussion.

marcingy’s picture

Reroll with #16

catch’s picture

Status: Needs review » Needs work

We don't usually put links to issues in code comments - otherwise we'd have thousands of them, and cvs annotate lets you find the issue with a bit of work, apart from that looks RTBC although I didn't test it.

marcingy’s picture

Status: Needs work » Needs review
FileSize
1.86 KB

Hopefully one last time....;)

catch’s picture

Status: Needs review » Reviewed & tested by the community
Dries’s picture

Project: Drupal core » HEAD to HEAD
Version: 7.x-dev »
Component: user.module » Code
Status: Reviewed & tested by the community » Needs work

Committed to CVS HEAD. I'm moving this to HEAD 2 HEAD.

David_Rothstein’s picture

Project: HEAD to HEAD » Drupal core
Version: » 7.x-dev
Component: Code » user.module
Status: Needs work » Fixed

Let's not move core issues to the queue of a module that about 5 people are using :) Instead we can just create separate issues in Head to Head and crosslink them. (Most people probably aren't interested in the Head to Head issue, and moving it there also prevents the core change from being widely visible or easily reopened for any necessary followups that might arise.)

In this case, @catch already opened a Head to Head issue preemptively anyway:

#809428: {users}.init length

I've posted a patch there.

Status: Fixed » Closed (fixed)

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