add hat description field

Working on this. This is a really amateur question - what does the update function need to accomplish? I have the other things taken care of. I am assuming it's a function that would add the field column to the table when you run update.php, but I'm too green to know where to look to accomplish this embarassingly simple task.

Ok here it is so far. I'm still looking around for a simple 'added a new field to this entity so I need this simple update hook example'.

Maybe this is it? Borrowed from party label update function in party.install .

On a secondary note, I should be describing these as party-hat-description-field or something of the like.

oops! naming it 7005 would conflict with the hat parent field, it should be changed to 7006 or something thereafter. my bad.

Ok, fixed, alongside the issue of using the new 'lenth' property in the schema ;)

Thanks for the friendly feedback

Do people think this should be a text-area rather than a text field? Or do we want to force these descriptions to be really short?

Sweet that makes sense. Maybe there's a better name for this then? Like 'tagline' or something?

I think 'description' works fine as a field name for a short description (see: Views UI). For more detail, you might want to use 'help' or 'summary'. This should be fine for now.

Also, with this length, we could potentially fit it into the 'Manage Hats' UI as an additional column? What about that? Might get a little scrunched on smaller displays. We could hide the machine name displays if we wanted to do that.

In the Content Types system it's a text area, and they fit it in in small grey writing under the Content Type name. Wouldn't it be better to follow this convention?

BTW, thanks for all the work you're putting in rerooting! It's good to have you on board!

How about 'brief description'. If folks want a more detailed description they can add a field via the UI (which we are keeping, correct?)

Here's a patch with the description as a text area. I don't know which one we want to use.

Either way, I'd like this committed quick so we can get #1613440: Give hats a parent hat which is optionally empty in.

I think we should keep it as a normal text field, since the plan is to put it into admin UI for hats. If we make it that long, it won't fit.

255 chars seems really short though, I was just basing it off of the Content types system that has a text area and can cope with longer descriptions. I'm happy to go with a text field for now - especially as for longer descriptions we can add a field. Once it's in we can always change it later.

I'll commit this now.

Cool! We're in! I made a couple of final tweaks, so the final patch is here. But we're all committed now!

Thanks rerooting! that's gone in under your name.

Oh man, I was committing this as you posted. We can roll back? Or make another issue?

I've got two big hunks of work waiting on hat parents though.

Majorly fast decision made on IRC to change this to following the node-types convention.

@rerooting sorry to mess this about a bit. We want to do it quick so we don't have to faff with more update functions. I tried to find you on IRC but couldn't - me and joachim normally hang out on #drupal-crm

Here's the final patch

oh hey! we should hang out on #drupal-crm, im ususally available as rerooting on #drupal-contrib when I am on IRC.

I was away from the drupal world working on a node.js/titanium project for the past few hours. I'd be glad to get myself and scresante on #drupal-crm regularly since we are becoming more involved with party.

Looks good though, want me to re-roll OOTB hats patch for this new field?

Hmmm, I had that issue with the patch in number 10 - i assumed it was down to me having rolled back updates and stuff and the hook schema caching being really aggressive.

Is that a SQL thing? Can TEXT columns not have defaults? If not, how can they be 'not null'?

I think that the issue has to do with defaults.

http://drupalcode.org/project/drupal.git/blob/refs/heads/7.x:/modules/no... for example

I just got this error again when running the update.

PDOException: SQLSTATE[42S22]: Column not found: 1054 Unknown column 'base.description' in 'field list': SELECT base.hid AS hid, base.name AS name, base.label AS label, base.description AS description, base.data AS data, base.required AS required, base.status AS status, base.module AS module FROM {party_hat} base WHERE (base.status IN (:db_condition_placeholder_0, :db_condition_placeholder_1, :db_condition_placeholder_2)) ; Array ( [:db_condition_placeholder_0] => 3 [:db_condition_placeholder_1] => 2 [:db_condition_placeholder_2] => 6 ) in EntityAPIController->query() (line 152 of /home/deva/public_html/sites/all/modules/entity/includes/entity.controller.inc

Ok, so you only that error if you try and upgrade 7005 and 7006 at the same time. Because hook_schema has already been updated, the entity_load and party_hat_save methods both look for a description column when loading and saving the Party Hats.

However, only running 7006 gives a different error.

Update #7006
Failed: PDOException: SQLSTATE[01000]: Warning: 1265 Data truncated for column 'description' at row 1: ALTER TABLE {party_hat} CHANGE `description` `description` MEDIUMTEXT NOT NULL COMMENT 'Some text describing the purpose of the hat'; Array ( ) in db_add_field() (line 2809 of /home/deva/public_html/includes/database/database.inc).

I guess this is because we have a NOT NULL text field, but then don't put a value in it (for all the hats that already exist in the database) I guess the simple solution is to make description not NOT NULL.

> CHANGE `description` `description` MEDIUMTEXT NOT NULL

You sure you weren't re-running the update? Why would it do a CHANGE?
Normally that would be 'ALTER TABLE tablename ADD newfieldname ...'

http://api.drupal.org/api/drupal/includes%21database%21mysql%21schema.in...

When something is 'NOT NULL' drupal seems to always run that as a changeField later. I don't know why. This also explains why the field gets created when the update runs.

So how does that manage to be NOT NULL and not have a default???

I guess cos It's a core module they haven't had to worry about update hooks adding this field to a table that doesn't already exist.

Also, we just hit something similar when saving a Party Hat. We'll need to add something to the save() method to atleast set description as an empty string, otherwise you'll get errors on save. Alternatively, we can make description not NOT NULL

Yeah, maybe not null => true is what we want.

See what our friends at profile2 did with a text field:

http://drupalcode.org/project/profile2.git/blob/refs/heads/7.x-1.x:/prof...

I don't think we can set a default value such as '' like we can with a varchar.

Following profile2's pattern seems like a good idea to me.

Oh yeah, I didn't read that properly

Here's a patch that should mean every PartyHat object has a description. this should allow us to keep the NOT NULL on description.

We may also want to check whether the description field is null inside PartyHat->save()

Ok, I just pushed this (I fixed the white-space error first). Tested it out and it seemed to fix saving a party.

If you programmatically create a Party Hat and saved it without explicitly giving a discription, the save would fail.

This should work as of a while ago, or so it seems:

#392696: Save default values on insert

Looking into this further...