While working on #1560012: Port per-user issue notification email functionality to D7 and just thinking about nodechanges in general, I'm running into situations where I really wish I had the revision ID that was created when a given node changes field value is stored. Seems like an obvious thing to have, and would make life a lot easier in various ways. The only bummer are people who are already using this module, since I don't know of a great way to write a DB update to populate this for people that already installed and started using this. However, this module has *never* been officially released yet, not so much as an "unstable", so I kind of feel that people already using it are taking the risk of schema changes like this. ;)
Stay tuned for a patch. I'm just rebuilding my local test site now to make sure this is working as expected...
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | 2013020-4.project-issue-migrate.patch | 1.78 KB | dww |
| #1 | 2013020-1.nodechanges-store-vid.patch | 3.06 KB | dww |
| #1 | 2013020-1.project-issue-migrate.patch | 1.41 KB | dww |
Comments
Comment #1
dwwLike so. This is storing both the nid and the vid, not just the vid, to make it easier to use this data. Working great in local testing. I'm also attaching an (untested) patch to project_issue for the migrate code to set these values correctly during migration.
Any objections?
Comment #2
chx commentedI do not see why not -- I guess denormalizing the nid is a good one, too.
Comment #3
drummLooks like a useful improvement.
In project_issue.migrate.inc, $node is not a real node in this area. vid is populated, but nid is not. It can be added after
The code should be
Comment #4
dwwThanks for the review and fix for project_issue.migrate.inc, drumm! Here's an update patch for that.
Meanwhile, what do y'all think about a hook_update_N() to try to do a schema migration for existing sites? In spite of the -dev only status, there are supposedly 131 sites already using this. Shall we hold their hands or throw them under the bus? ;)
Thanks!
-Derek
Comment #5
drummComment #6
drummI went ahead and committed what's here. It looks good and we don't need to guarantee an upgrade path for -dev. It would be great if another developer could come jump in and add this.
I did make an addition to project_issue.migrate.inc to save the data, which doesn't go through the API.
Comment #7
drummBringing this back in scope because our upgrade script is slow.
Comment #8
drummA bit more detail on #7: #2058163: Undefined $field_issue_changes_nid PHP Notices on git7site (Probably not nodechanges, but starting here) was the symptom. The project_issue migration initially failed because the new columns didn't get added to
->fields($insertfields)http://drupalcode.org/project/project_issue.git/commitdiff/92e4097?hp=2a.... That's fixed, but mistakenly didn't make it onto 7.devdrupal.org before the upgrade started again. An upgrade path within nodechanges would get these columns added on git7site in the meantime.Comment #9
drummI committed http://drupalcode.org/project/nodechanges.git/commitdiff/b859033?hp=72c9... and it sucessfulyl ran on git7site.