Pre-beta update hook code

I think we can improve it by addressing #2678756-77: Allow config entities to be flagged.

Thanks for filing this :)

Though I would just post the code as plain PHP to be run by Drush, and post it in the issue summary rather than a file. We definitely do not want people to run this code as update hooks, as then they won't be able to run a future, real update 8001 without hacking their site state.

Schema module is really useful for doing this sort of thing -- however, the D8 version is on github and needs a few tweaks to get it working: #2334557: Drupal 8 Version.

Adding partial update code for #2845567: make use of ContentEntityBase::baseFieldDefinitions(); make flag_id an entity reference field.

Not sure why the code for #2845567: make use of ContentEntityBase::baseFieldDefinitions(); make flag_id an entity reference field isn't working.

On the one hand, it appears to change the database structure. But when I run 'drush entity-updates' to deal with an unrelated update, I get this error:

Drupal\Core\Entity\Exception\FieldStorageDefinitionUpdateForbiddenException: The SQL storage cannot change the schema for an existing    [error]
field (flag_id in flagging entity) with data. in Drupal\Core\Entity\Sql\SqlContentEntityStorageSchema->updateSharedTableSchema() (line
1403 of /Users/joachim/Sites/8-drupal/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorageSchema.php).
Failed: Drupal\Core\Entity\Exception\FieldStorageDefinitionUpdateForbiddenException: !message in                                         [error]
Drupal\Core\Entity\Sql\SqlContentEntityStorageSchema->updateSharedTableSchema() (line 1403 of
/Users/joachim/Sites/8-drupal/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorageSchema.php).

But when I run 'drush entity-updates' to deal with an unrelated update, I get this error

Got this error too. Is there a way to fix this without reinstall the module?

> Not sure why the code for #2845567: make use of ContentEntityBase::baseFieldDefinitions(); make flag_id an entity reference field isn't working.

Because you just change the database schema, but you didn't tell the entity definitions update manager that you did so.

See for exampe poll_update_8001(). you need to load the generated/stored sql schema and field definitions and update that as well.

I still don't quite understand why you're against adding update functions, you'll likely spend more time on closing/answering support requests than maintaining those and I would even write them as I mentioned once, because I eventually will have to do that anyway for our projects. But currently I'm simply waiting until the first beta.

The problem is that many projects provide updates in alpha versions (including most that I maintain, at least big ones like redirect) so it's hard for users to figure out when they can use an alpha and when not. Also, flag is taking its time to get out of the alpha phase ;)

Based on @berdir 's suggestion, here is a hook_update() I did in my site module, and which looks like it could become flag_update_80xx(). It prevents the error when doing a second drush entup, which happens with just the current version of flag:

function somesitemodule_update_8042() {
  $bundle_schema = [
    'description' => 'The Flag ID.',
    'type' => 'varchar_ascii',
    'length' => \Drupal\Core\Entity\EntityTypeInterface::BUNDLE_MAX_LENGTH,
    // 'binary' => FALSE,
    'not null' => TRUE,
  ];

  /** @var \Drupal\Core\Database\Schema $schema */
  $schema = \Drupal::database()->schema();
  $schema->changeField('flagging', 'flag_id', 'flag_id', $bundle_schema);
  $schema->dropIndex('flagging', 'flag_id');
  $schema->dropIndex('flagging', 'flagging_field__flag_id__target_id');
  $schema->addIndex('flagging', 'flagging_field__flag_id__target_id', ['flag_id'], ['fields' => ['flag_id' => $bundle_schema]]);

  // Update the field storage repository.
  /** @var \Drupal\Core\Entity\EntityFieldManagerInterface $efm */
  $efm = \Drupal::service('entity_field.manager');
  /** @var \Drupal\Core\KeyValueStore\KeyValueStoreInterface $kv */
  $kv = \Drupal::service('keyvalue');
  /** @var \Drupal\Core\Entity\EntityLastInstalledSchemaRepositoryInterface $repo */
  $repo = \Drupal::service('entity.last_installed_schema.repository');

  $efm->clearCachedFieldDefinitions();
  $storage_definition = $efm->getFieldStorageDefinitions('flagging')['flag_id'];
  $repo->setLastInstalledFieldStorageDefinition($storage_definition);

  // Update the stored field schema.
  // @todo: There has to be a better way to do this.
  $kv_collection = 'entity.storage_schema.sql';
  $kv_name = 'flagging.field_schema_data.flag_id';
  $field_schema = $kv->get($kv_collection)->get($kv_name);
  $field_schema['flagging']['fields']['flag_id'] = $bundle_schema;
  $field_schema['flagging']['indexes']['flagging_field__flag_id__target_id'] = ['flag_id'];
  $kv->get($kv_collection)->set($kv_name, $field_schema);
}

If it runs in a drush updb before drush entup, the second drush entup doesn't fail like it does with just flag module enabled.

Any ideas for improvements ?

> I still don't quite understand why you're against adding update functions, you'll likely spend more time on closing/answering support requests than maintaining those and I would even write them as I mentioned once, because I eventually will have to do that anyway for our projects. But currently I'm simply waiting until the first beta.

Because then it wouldn't be an alpha any more, because users would have the expectation that we'd fix all of their breakages, such as changes to their views and so on. I am tempted to just say, 'what the hell, if people file hook_update_N() patches then I will commit them blindly' but then I don't want that to lead to issues that break stuff getting held up with writing the updates, and then some changes get updates and some don't, and we still have problems with expectations.

That said, looking at the beta blockers in issue queue, I think we're done with schema-breaking things, it's just API-breaking things from here on. Which is going to lead to even more support requests, as people don't read the release notes (me included, if I'm honest...) and we can't do updates that rewrite people's code...

> you'll likely spend more time on closing/answering support requests than maintaining those

You're thinking it's the same kind of time, but it's not. I can rattle off a reply to a support request at 07:50 on the tablet while I'm waiting for my 2-year-old to come downstairs for breakfast, but I can't do any coding in that length of time. I can do issue triage at 21:00 when the kids are in bed and I'm exhausted and watching wallpaper television, but I don't have the mental energy to fix bugs.

> Also, flag is taking its time to get out of the alpha phase ;)

Yeah... :( Flag 4.x as it stands still feels like it's only doing about 50% of what 3.x does. I am sometimes sorely tempted to just take 3.x, do the bare minimum to get it working on Drupal 8, and release that, and then gradually add bits of 4.x into it. I honestly don't know at this point what would be the quickest path to a stable release.

Hi,

So i guess I'm stuck too :)

@fgm's update function in #10 works fine and allows me to update / create my custom entity type.

Cheers !

Hi fgm,
I'm curious to know what commit did you write this update hook on? I'm currntly on this one: "http://cgit.drupalcode.org/flag/commit/?h=8.x-4.x&id=4f9093a3da47d95dae1...". It doesn't seem to work for our Open Social Distro. The hook is being executed and it says everything went fine, but still a drush entup gives me:

The following updates are pending:

flagging entity type :
  The Flagging entity type needs to be updated.
  The Session ID field needs to be installed.

Any clue? Help would be much appreciated

Hi jochemvn,

yep after the update, drush entup still gave flagging entity type to update, but this time, it wont fail (at least it did not on my install)

regards

@jochemvn: like @kgaut implied, you need to run entup /twice/ after this hook to get the update to be complete. Since it never hurts to do it anyway, I actually always run it thrice in my CI update jobs.

A better update hook could probably avoid this step, but it's sufficient for now until an actual update path is validated by @joachim.

Another +1 for the update script in #10, it solved the problems I was running into after updating from alpha1 to either alpha2 or the latest -dev snapshot.

The update script from #10 in patch format.

Now that the patch is in the right form, I think there's enough above for this to be RTBC.

Thanks everyone!

I'm still seeing entity-update issues on 8.7 even though this update hook ran

`Flagging: The Flag field needs to be updated.`

should I open a new issue or happy to continue here?

My definitions and what's in the kv look good

KV:

$entityStorageSchemaSql->get('flagging.field_schema_data.flag_id');
=> [
     "flagging" => [
       "fields" => [
         "flag_id" => [
           "description" => "The Flag ID.",
           "type" => "varchar_ascii",
           "length" => 32,
           "not null" => true,
         ],
       ],
       "indexes" => [
         "flagging_field__flag_id__target_id" => [
           "flag_id",
         ],
       ],
     ],
   ]

Table def:

describe flagging;
+-------------+------------------+------+-----+---------+----------------+
| Field       | Type             | Null | Key | Default | Extra          |
+-------------+------------------+------+-----+---------+----------------+
| id          | int(10) unsigned | NO   | PRI | NULL    | auto_increment |
| flag_id     | varchar(32)      | NO   | MUL | NULL    |                |
| uuid        | varchar(128)     | NO   | UNI | NULL    |                |
| entity_type | varchar(255)     | YES  |     | NULL    |                |
| entity_id   | varchar(255)     | YES  |     | NULL    |                |
| global      | tinyint(4)       | YES  |     | NULL    |                |
| uid         | int(10) unsigned | NO   | MUL | NULL    |                |
| created     | int(11)          | YES  |     | NULL    |                |
| session_id  | varchar(255)     | YES  |     | NULL    |                |
+-------------+------------------+------+-----+---------+----------------+
9 rows in set (0.008 sec)

Indexes:

MySQL [lgtoolbox_prod]> show indexes from flagging;
+----------+------------+------------------------------------+--------------+-------------+-----------+-------------+----------+--------+------+------------+---------+---------------+
| Table    | Non_unique | Key_name                           | Seq_in_index | Column_name | Collation | Cardinality | Sub_part | Packed | Null | Index_type | Comment | Index_comment |
+----------+------------+------------------------------------+--------------+-------------+-----------+-------------+----------+--------+------+------------+---------+---------------+
| flagging |          0 | PRIMARY                            |            1 | id          | A         |         263 |     NULL | NULL   |      | BTREE      |         |               |
| flagging |          0 | flagging_field__uuid__value        |            1 | uuid        | A         |         263 |     NULL | NULL   |      | BTREE      |         |               |
| flagging |          1 | flagging_field__uid__target_id     |            1 | uid         | A         |         131 |     NULL | NULL   |      | BTREE      |         |               |
| flagging |          1 | flagging_field__flag_id__target_id |            1 | flag_id     | A         |           4 |     NULL | NULL   |      | BTREE      |         |               |
+----------+------------+------------------------------------+--------------+-------------+-----------+-------------+----------+--------+------+------------+---------+---------------+

And the character set on that column is ascii

I am not sure if 2 years ago "entity.definition_update_manager" and "entity_field.manager" services exist, but they were there for sure since 8.2, but I think the update 8401 should be something like in the attached patch. I have incremented the update number of course up to 8402.

Made a typo in \Drupal. Here is the updated patch

I suggest you create a new issue, nobody is going to look at this :)

Saw the notification but will have forgotten about this in a minute again.

#25 Solved issues in my case (Drupal 8.8, FLag 4.0 beta1).