Fix warnings after upgrade to alpha22 (entity definitions)

Hey Jurgen,

Damn, and I thought I tested everything well! :)

To be fair, I didn't even look at the status report after upgrading my own site (something I do standard upgrading client sites haha) and I see the same messages about updating those two entity types. Not sure exactly why they are there, will investigate.

I think we can ignore the warning about the backup table, since it's not actively used. At some point, this table can be dropped (although I'm not sure whether I'm going to write an update hook for that). It's there so I could rerun the drush command for creating timeline items over and over again.

This showed many screens of notices about created timeline items.

Do you mean PHP notices, or just the notices which drush is outputting? Thy start with something like "Checking activity id .. "
And at the end, it should show some stats: "Checked {number}, created {number}, skipped {number).

So I think that one is going fine, if there are at least some items created :)

Are you saying I could drop that table manually if the drush command activitypub:create-timeline-items ran successfully?

Yes, it's safe to drop. I haven't touched it anymore after I ran the upgrade myself a week ago. At some point, it becomes pointless anyway to rerun that drush command, especially when new activities have arrived in the inbox. I'll change the upgrade path though so the primary key is created.

It's the drush output, including a final notice saying "Checked 672, created 657, skipped 15".

That looks good!

I'll have a look at the entity type update messages.

Ok, first patch for the indexes is in.

The Drupal code line.

\Drupal::database()->query("ALTER TABLE {activitypub_activity_backup} ADD PRIMARY KEY (id)");

Or if you want to add it via mysql:

ALTER TABLE activitypub_activity_backup ADD PRIMARY KEY (id);

So the entity updates for the actor and activity are mostly entity definition changes (storage class for actor, and label updates for activities), I would have expected that this is refreshed automatically.

There's also a field removed (is_read) and added for activity (changed), but those work during the update - oterwhise, a lot things would go wrong :)

Looking how to refresh those now, but as far as I can see (and I'm running this for at least a week now on my site), all functionality should work normal. Oh my, I'm not a fan of entity updates :)

Renaming

I can fix the warning for actor, but not yet for activity. It looks like it wants to install a new index activitypub_activity__status with status and id field, but that's not something I've added (as far as I can see). Digging further, to see what on earth this is causing.

So it looks like I'm bitten by something stupid: I renamed my storage class and now the update manager thinks it needs to run a complete migration. See #2336895: Allow entity type and field storage definition objects to be compared for definition equality

Renaming it to the original makes it all work again. Going to see if I can rename it somehow.

Ok, the update hook works when we start 'fresh'. It works half when the previous updates had run.

In that case, for some reason, there was a change in the collection field (and I understand why).
But, then we need to manually use devel_entity_updates and running drush updb makes the warnings go away.

So we'll need a two step for users who have done the upgrade.

So, the good news is, you can ignore the warnings because drupal core has some missing code when something simple changes in an entity definition. The update hook that is in now fixes it partially for us, so you'll have to run devel entity updates as well, just to get rid of the warning.

I'll try to get the shared inbox functionality in as soon as possible (luckily I have grant, so I'm working very hard on it!), but you can safely use the patch if you want already.

Ok, last commit, sorry for the noise :)

I'm adding the primary key in a separate new update hook, so running that one in case you have added it already will fail, but it's necessary for installations which have not done this manually.

It's very likely the patch won't apply on your code base since there are some commits in dev already, but you can safely remove activitypub_update_8030.

Or, you can just wait until the next alpha :)

Merged in, updated the alpha22 release notes page as well.