Support from Acquia helps fund testing for Drupal Acquia logo

Comments

gaydabura created an issue. See original summary.

gaydabura’s picture

Temporary solution.
Also we remove term itself from "parent" to prevent recursion

gaydabura’s picture

Assigned: gaydabura » Unassigned
Status: Active » Needs review
andypost’s picture

Status: Needs review » Needs work
Issue tags: +Needs tests

Please describe steps to reproduce to write tests

+++ b/src/DefaultContentManager.php
@@ -277,6 +277,11 @@ class DefaultContentManager implements DefaultContentManagerInterface {
+        unset($parents[$entity->id()]);

that needs inline comment Why

gaydabura’s picture

Status: Needs work » Needs review
FileSize
969 bytes

To reproduce:
1. Create vocabulary
2. Add terms with hierarchy
3. Export terms
4. Try to import terms
No hierarchy.

Status: Needs review » Needs work

The last submitted patch, 5: taxonomy_hierarchy_is-2687075-5.patch, failed testing.

gaydabura’s picture

+ unset($parents[$entity->id()]);
This needed to avoid recursion when term depends on itself. For some reason loadAllParents add term itself to tree.

gaydabura’s picture

Status: Needs work » Needs review
larowlan’s picture

Status: Needs review » Needs work
larowlan’s picture

andypost’s picture

Status: Needs work » Needs review
FileSize
3.16 KB

Here it is, still needs tests

jibran’s picture

+++ b/src/Normalizer/TermEntityNormalizer.php
@@ -0,0 +1,49 @@
+class TermEntityNormalizer extends ContentEntityNormalizer {

Given that this is the same normalizer we have in entity pilot. I think we should add this normalizer to core.

andypost’s picture

jibran’s picture

+++ b/src/DefaultContentServiceProvider.php
@@ -26,6 +28,20 @@ class DefaultContentServiceProvider extends ServiceProviderBase {
+      $container->setDefinition('default_content.normalizer.taxonomy_term.halt', $service_definition);

This is an alter function so we should move this service definition to yml file. Do we really need module exist check for taxonomy?

andypost’s picture

Assigned: andypost » Unassigned
Issue tags: -Needs tests
FileSize
2.1 KB
1.42 KB
4.47 KB

Here's a test

The last submitted patch, 15: 2687075-14-test-only.patch, failed testing.

andypost’s picture

@jibran suppose we have to check for module enable, otherwise default content module should set dependency

jibran’s picture

Status: Needs review » Reviewed & tested by the community

Ok that's fine so why not use DefaultContentServiceProvider::register() instead.

andypost’s picture

personally because there's alter already

Sam152’s picture

FYI, I created a sandbox called 'Better Normalizers ', https://www.drupal.org/sandbox/sam/2705067. The intention being, sharing this code between modules. Maybe copying them is easiest if they are likely to vary between these modules anyway, but it was required for aGov because we needed the file_entity normalizers but didn't want a dependency on file_entity itself.

sqndr’s picture

Is there a way to manually at the parent to the json file? I'm still not able to export the parent for taxonomy terms …

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 15: 2687075-14.patch, failed testing.

larowlan’s picture

git fetch-patch https://www.drupal.org/files/issues/2687075-14.patch
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  4581  100  4581    0     0   3990      0  0:00:01  0:00:01 --:--:--  3993
error: src/DefaultContentServiceProvider.php: No such file or directory
error: src/Tests/DefaultContentManagerIntegrationTest.php: No such file or directory
larowlan’s picture

Issue tags: +Needs reroll
aleevas’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
3.01 KB

Rerolled last patch.

Status: Needs review » Needs work

The last submitted patch, 25: taxonomy_hierarchy_is-2687075-25.patch, failed testing.

andypost’s picture

+++ b/src/DefaultContentServiceProvider.php
@@ -26,6 +28,21 @@ class DefaultContentServiceProvider extends ServiceProviderBase {
+    $modules = $container->getParameter('container.modules');
+    // @todo Get rid of after https://www.drupal.org/node/2543726
+    if (isset($modules['taxonomy'])) {

this part missed in reroll

andypost’s picture

Assigned: Unassigned » jibran
Status: Needs work » Needs review
FileSize
4.17 KB

Service provider gone, so patch restores

Tests needs review cos old ones are refactored

larowlan’s picture

Status: Needs review » Reviewed & tested by the community

I think this is ready too, will commit on Monday if still RTBC

Dane Powell’s picture

Patch #28 worked for me. Thanks.

Dane Powell’s picture

Note that I just filed #2796791: Recursive reference export, which might be a problem specific to this patch or something more general.

Hydra’s picture

#28 Still working for me :) THx!

  • larowlan committed 24c6a02 on 8.x-1.x authored by andypost
    Issue #2687075 by andypost, gaydabura, aleevas, larowlan, jibran:...
larowlan’s picture

Status: Reviewed & tested by the community » Fixed

Fixed, thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

jibran’s picture

Assigned: jibran » Unassigned