Multiple Parent Hierarchies Lost on Export

The #2 patch can no longer be applied on 2.0.6 and up.

To make this easier to review, I've created a merge request. The patch also didn't apply for me, so I tried re-creating it: please check that the merge request still solves the problem, and report back here.

My editor automatically corrected some whitespace errors from the patch, but it may have missed some, and I'm confident that there will be other issues (e.g.: the name of the private function, __add_taxonomy_ancestors doesn't follow function naming conventions → ). Look for the new issues reported by PHPCS (and/or PHPStan) in the merge request pipeline.

A brief glance at the code in the merge request suggests that it has no tests. The Structure Sync maintainers prefer to accept merge requests that have passing automated tests. If you need help writing tests, please ask: I would be happy to help!

The patch by itself will not apply. So I created a new patch that also removes an endless loop condition if there is a Missing UUID for term reference. This way, at least you can get an import but with warnings.

This was missing adding the uuid to exports on some taxonomy terms.

@taiger, thank you!

Your patch in #10 fixes a lot of the clarity and coding-standards issues with the patch in #2, which is great: thank you very much! I hadn't noticed the missing-UUID bug myself, but I will check it out when I incorporate your patches into the merge request when I next have some contribution time.

I also noticed that the filename in the patch you posted to #11 refers to a different issue and unexpected comment number (i.e.: #3177106-4: Support terms with the same name in a tree structure and multi-parent terms → ). The patches seem mostly the same, so I'm going to assume that was unintentional (I couldn't count the number of times that has happened to me).

I worry that we might have been duplicating our efforts this week... it seems that we both spent time this week re-rolling the patch from #2: I committed my re-roll to merge request !41 at the same time that you rewrote #2 and posted your re-write to the patch in #10.

In the future (i.e.: not necessary for this issue), there are some things that you can do to reduce duplicate efforts, and make things easier for maintainers — which will usually result in a faster turn-around time!

As a maintainer, I have to make sure that the patch that I accept fixes the original issue that @kwfinken reported (i.e.: stays within scope → ). I feel reasonably confident that @kwfinken's own patch in #2 fixes the issue (i.e.: because the same person reported the issue and posted the patch). But, because you updated the patch and made your own changes in the patch posted to #10, it is non-trivial for me to see how we got from @kwfinken's patch in #2 to the patch in #10 — that's why it is going to take me some time to incorporate your changes! (I also maintain ~29 other modules with ~1700 issues across all of those modules — and my full-time client doesn't use this module, so I maintain this module on my volunteer time, which is limited by my responsibilities to my work, family, etc.!)

Because lints and tests are automatically run on merge requests → , I find them easier to review than patches these days (and I often convert patches into merge requests for this reason). If there is already a merge request in an issue, please consider contributing to the merge request instead of patches! Here's a video tutorial; or if you prefer written documentation, skim this introduction → ; and read how to clone and commit code to an issue fork → , and how to create a merge request → .

If you prefer to keep working with patches, and the latest patch no longer applies, you can re-roll a patch → . When re-rolling a patch, you can make things easier for a maintainer by making as few changes to the re-roll as you possibly can. Then — once you have posted the re-rolled, minimally-changed patch — make any improvements you want to make in one or more subsequent patches (it also helps to include interdiffs with the subsequent patches → ). By making as few changes as possible in the re-roll, I can easily compare the original patch (i.e.: that didn't apply) to the re-rolled patch: if they are mostly the same, then it's easier for me to see that they both fix the same problem. By making improvements in follow-up patches (with interdiffs), it is easier for me to identify and understand the improvements.

(aside: merge request !41 looks a lot like the patch in #2 because I re-rolled it with minimal changes, and I wanted to take a look at the pipeline results before making further changes)

Thank you for your detailed response!

I can see that I should have re-rolled the patch before making the additional changes. I will try to do that going forward and be more careful about patch naming.

This patch fixed an issue that my team was experiencing and I wanted to share it back to the community. Doing a merge request, once a patch has been reviewed, is even more helpful and I will consider that going forward.

The Structure Sync module has been essential for us to getting away from issues with the Content Sync module. Thanks for your hard work on this!