Contribs aren't notified of a reversion, so they can't replace their data. This is a major problem resulting in the loss of data.

We either need another op added to hook_nodeapi, implement a hook_revert, or just delete newer revisions so the the one being reverted to is the most current. I personally prefer the last choice, but I suspect I am in the minority on that. I'll be happy to work on a patch when I see which way people are leaning on this.

Comments

nancydru’s picture

It looks like the nodeapi option is very simple: In node_revision_revert_confirm_submit(), right after node_save($node_revision), just add node_invoke_nodeapi($node_revision, 'revert revision');.

The contrib then has access to the old information and can get the current revision number from the database and update its tables.

dropcube’s picture

Title: No revision revert hook. » Add hook_node_revert_revision()
Category: bug » task
Issue tags: +DX (Developer Experience)

Currently, modules could implement hook_node_update($node) and check for !empty($node->revision), and then perform the required actions there. However, a hook_node_revert_revision would be a cleaner an more consistent API. Let's see what other says.

Anyone know a real use case in contribs where this hooks would be needed ?

nancydru’s picture

Yes, that's why I posted it. In Web Links we have several fields to add to a node. This includes the URL being linked to. We support versioning of this data by keeping nid and vid. The other day I had need to revert a revision and discovered that the old data was lost. That's when I discovered that there is no good way to really get back to the old version.

I believe I would have a similar problem in Announcements and Quotes.

I will play around with your idea, but I have my doubts that everything I need is there (like the previous vid).

BTW, I assume you mean hook_update as I don't see a hook_node_update in the APIs.

nancydru’s picture

Yes, there is a field for "old_vid" that points to the one being reverted to, however, that field is also present when making a new revision. How would one tell the difference, other than by a query to see if it already existed? Can I rely on $node->op not being set on a reversion?

dropcube’s picture

I assume you mean hook_update as I don't see a hook_node_update in the APIs.

I mean for 7.x, for 6.x hook_nodeapi() right ?

nancydru’s picture

Oops, sorry, I was trying to fix my 6.x version, so that's where my mind was.

catch’s picture

Version: 7.x-dev » 8.x-dev

This would still be great, but it's too late for D7 since there's already a way to act on revision reverts via the API even if it's not very nice.

catch’s picture

Priority: Critical » Major

Downgrading all D8 criticals to major per http://drupal.org/node/45111

ekaterina’s picture

Can someone create a patch to add new $op = 'revert revison' to hook_nodeapi() for Drupal 6.x please?

nancydru’s picture

I will have to get a newer code base for D7.

ekaterina’s picture

unfortunately I need it für D6 :-(

Also I found out, that all data loaded with the hook_load(), will be correctly reverted. But what should I do if I have many data related to one node. Should I load all data with this hook? What about performance? The hook_load() will be called often and not only when the user want to revert the revision.

nancydru’s picture

This issue is not about loading. However you load now shouldn't change.

horuskol’s picture

I have a second use case - I want to stop the automatic publishing of the reversion in order to force moderation of the content change by a second user.

nancydru’s picture

horuskol’s picture

I tried that module - it doesn't stop the automatic publishing when someone reverts.

quicksketch’s picture

Priority: Major » Normal

This is a request for a dedicated hook for clarity, though as noted above, it's entirely unnecessary. The current hook_node_update() hook already includes $node->old_vid and $node->vid which can be used to identify a new revision. A lot of modules have already solved this problem using this approach and in my opinion, this change is unnecessary.

In any case I'm downgrading this issue so it doesn't prevent addition of new features per the new policy instituted at http://drupal.org/node/1201874.

nancydru’s picture

Issue summary: View changes
Status: Active » Closed (won't fix)