Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Moving items cause duplicates on update #145

Closed
Rarst opened this issue Dec 18, 2014 · 4 comments
Closed

Moving items cause duplicates on update #145

Rarst opened this issue Dec 18, 2014 · 4 comments

Comments

@Rarst
Copy link
Contributor

Rarst commented Dec 18, 2014

Example:

It seems like hook had been moved around in core source and update created duplicate, rather than update existing entry. This likely need to be better handled on updates.

@coffee2code
Copy link
Contributor

More background on this:

In [30085-core], setup_postdata() was moved from being a function in global space to a class method within the class WP_Query.

This has two (so far) unintended consequences for the parser:

  1. setup_postdata() is now a class method and not a function. This causes a duplicative effect when updating. See the concurrently existing old https://developer.wordpress.org/reference/functions/setup_postdata/ and current https://developer.wordpress.org/reference/classes/wp_query/setup_postdata/. The check on import for an existing object relies on post_type (which has changed from wp-parser-function to wp-parser-method) and post_parent (which has changed from '' to the class's ID).
  2. Hooks that existed in a function affected by the previous item (e.g. the example in the ticket, 'the_post') are duplicated rather than updated. This stems from the first issue; because the hook's function is a new thing (post_parent), a new instance is assumed.

Not sure the best way to inform the parser about the first issue. I don't feel that we can reliably assume a same-named class method is necessarily the relocated version of a deleted function (nor does this likely happen very often). Maybe we can pass in a config file identifying relocations for the parser to take into consideration?

This may be somewhat moot if the parser deleted (or hid) resources that were not updated during a full parsing (optionally) (see: #147). The old, and now un-updated, version would become hidden. We just wouldn't automatically know for certain if it was because that resource was truly deleted or just moved.

There are more options for handling the second case (and thus this ticket specifically). A hook should only be fully documented in one place. So if an instance of wp-parser-hook is encountered, ignore the post_parent part of the check. If an existing object with the same post_name and post_type is found (and it is truly documented), update that hook instance. Related: #42.

@JDGrimes
Copy link
Contributor

There are more options for handling the second case (and thus this ticket specifically). A hook should only be fully documented in one place. So if an instance of wp-parser-hook is encountered, ignore the post_parent part of the check. If an existing object with the same post_name and post_type is found (and it is truly documented), update that hook instance. Related: #42.

I think #94 is related as well.

@DrewAPicture
Copy link
Member

setup_postdata() is now a class method and not a function.

Actually setup_postdata() is now both a function and class method. Most of the logic was moved to leverage the new method but both still exist. Seems like the biggest issue is really that hooks formerly associated with the function should be migrated to the new class method.

@DrewAPicture
Copy link
Member

#192 fixes the duplicate hooks being created issue only. If we want to look later and trash collecting the duplicates, we'll have to also look at merging all of the notes to the original and then simply updating the parent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants