replace 'content' with 'entity', eg $content_type with $entity_type

Seems it's actually a bit more complicated than this, as flag types can defined on any kind of object -- in fact anything that has an ID that can fit into the table columns in {flags} and {flag_content}.

So probably:

content_type -> object_type
content_id -> object_id
type/ subtype -> ??? keep as subtype but use bundle within the flag_entity class?
content -> object

and also:

{flag_content} -> {flagging}
{flags} -> {flag}

So before we do this we should try to round up and commit all the issues that make changes to the database schema, because otherwise backporting them will be much more work.

So far I only know of #1650512: Index to improve view performance.

*sigh*

I'm still debating whether to replace 'content_type' with 'entity_type' or 'object_type':

- 'object_type': Technically, flags can go on anything that cares to declare itself to flags. You could define a flag type on poodles, and provided you output the flag link with the right parameters, the flagging table will just say 'ok you've flagged poodle id 1'. So this is the more correct term to use.

- 'entity_type': *However*, the vast majority of flags are going to be on entities: nodes, terms, users, commerce products and orders, and so on. It would be bumpy DX for all these cases to see 'object_type' here where in the rest of Drupal it's 'entity_type'.

So it boils down to either:

- the correct term which in 90% of cases is going to require a bit of brain-adjustment because they're also going to be entities, or
- the term that's going to look right 90% of the time and require a bit of 'oh I'm flagging poodles and so my entity type is poodle even though they're not really an entity' DX bump for the 10% exceptions.

Put like that, it seems like a no-brainer to go for 'entity_type': the 80-20 rule.

Here's a monster patch. It doesn't yet tackle all of the database updates, so it should only be tried with fresh installs to make sure everything works.

The patch file itself is in git format-patch format, thus a list of all the commits on my local branch. The advantage is that it's easier to follow the changes step by step. Also, we can use the log to write a change record later.

Apply it with 'git am' rather than 'patch', and do so on a local branch rather than 7.x-3.x (or be prepared to reset afterwards, as this will create commits!). (More details on this trick here: http://www.drupaler.co.uk/blog/moving-git-local-branch-one-local-another.)

I'm halfway through writing the column updates, which is fairly hairy and tedious due to all the indexes that have to be dropped and re-created.

Also I'm stumped as to whether/what to rename the 'fcid' column to.

... I guess the testbot won't like that file. So here's a single patch too.

Hold your horses, it still needs the update functions! Thanks for the vote of confidence though! :)

> that "fcid" seem a little obscure given the renaming of the table to "flaggings". The problem is, I can't think of anything better.

Indeed.
I was thinking we take Commerce's pattern and do 'flagging_id'. But I really can't be bothered to change flag.fid to flag.flag_id to match!!!

The other thing is there are a lot of $flagged_content (or is it $flag_content) variables internal to functions. I am wary of changing those to $flagging when they are NOT the loaded entity, but just table row data (or partial rows), as that's a DX WTF. I should check none of these are in hooks or API-ish function definitions, and then leave the rest for follow-up :)

OK here's the patch with the update functions.

To apply this you need a git checkout on branch 7.x-3.x and:

$ git co -b 1681540-local-branch
$ git am 1681540.11.flag.rename-cataclysm.patch

I've only tested this on a localhost with a handful of flaggings, so I would *really* appreciate it if someone could test it with some real site data (on a development copy of course!).

*sigh* I should really do the fcid column too, makes sense to do it now.

*double sigh* This is going to have to come first so the update function is the same: #1709498: lengths of db columns for entity and bundle names are too short.

> Something in one of the recent commits broke the patch. I had to rollback to 142e7589274a6a19bd216ddeb6fb0c23a5d731cf to get it to apply.

Ah, that was me. I made two commits last night.
I rebased my local branch for this issue and fixed the conflicts but didn't get round to posting a patch.

Thanks for trying it out!

I'll do the fcid -> flagging_id soon.

Here's a patch rebased to the current 7.x-3.x, just to keep everyone in the loop :)

> but they were already renamed. Might have been my fault.

Yeah, you need to not just restore the last DB from backup_migrate module, but then ALSO delete manually the renamed tables: B&M does not delete tables it doesn't have in the backup tarball.

That approach is fine too -- it's what I tend to do for ongoing development. For this issue though I knew I was going to have to rollback quite a few times.

I was under the impression you're using flag for a live site? Once I've done the fcid rename, could you test the patch with a copy of that?

Last patch, I hope.

(This is a patch against ed26176bb6f8a4aea6cc31d54a167a1e97e9b766, one back from the current 7.x.3.x. Testbot will fail.)

Note: I'm not changing the names of views handlers because AFAIK there's no easy way to change a user's existing views to account for this.

#22: 1681540.22.flag_.rename-cataclysm.patch queued for re-testing.

That was because of the commit from #1689400: make use of machine name field in admin UI. I've just reverted that, so let's test again.

If the testbot agrees, I think we're finally about able to commit this.

What would be great though would be a review from someone with real-world flag data to upgrade (on a test copy of course!).

> Note: I'm not changing the names of views handlers because AFAIK there's no easy way to change a user's existing views to account for this.

Turns out I'm wrong :)

#1714082: code and docs disagree on renaming of fields.

Here's the patch once again with the Views changes too. Should be all done. For real this time! Honest!

Hooray. Thanks for all your help on this. I'll commit this in the next day or so and we can get moving on other things.

Issue #1681540 by joachim: Renamed database tables and columns, properties, functions, and variables to match Drupal 7 entity terminology.

Woohoo!

I merged this in with --no-fast-forward, so we keep the individual commits, should we ever need to go debugging what was changed here.

For big, complex issues where we have a local branch that's got clean commits, sure.

For a local branch that's a total mess of changes and clean-ups (as my local branches usually are ;) -- best not ;)

Oh. Yes. Nearly forgot. This needs a change notification. I'll try and get round to that in the next couple of days.

Argh, rules. Could you file this as a new bug please, and attach the exported rule please?

OK, I think this covers it: http://drupal.org/list-changes/flag