Merge pull request #23 from Financial-Times/bugfix/ReturnCanonicalUUI…

…DPrefLabelFromOrg Handle memberships to orgs with equivalent relationships and return canonical data
Financial-Times · May 29, 2018 · e1f69e5 · e1f69e5
2 parents 9bb531d + 3e783ff
commit e1f69e5
Show file tree

Hide file tree

Showing 5 changed files with 113 additions and 8 deletions.
diff --git a/people/cypher.go b/people/cypher.go
@@ -1,13 +1,13 @@
 package people
 
 import (
+	"errors"
+	"fmt"
 	fthealth "github.com/Financial-Times/go-fthealth/v1_1"
 	"github.com/Financial-Times/go-logger"
 	"github.com/Financial-Times/neo-model-utils-go/mapper"
 	"github.com/Financial-Times/neo-utils-go/neoutils"
 	"github.com/jmcvetta/neoism"
-	"errors"
-	"fmt"
 	"strings"
 )
 
@@ -116,12 +116,13 @@ func (pcw CypherDriver) Read(uuid string, transactionID string) (Person, bool, e
 						MATCH (canonical)<-[:EQUIVALENT_TO]-(p:Person)
                         OPTIONAL MATCH (p)<-[:HAS_MEMBER]-(m:Membership)
                         OPTIONAL MATCH (m)-[:HAS_ORGANISATION]->(o:Organisation)
+						OPTIONAL MATCH (o)-[:EQUIVALENT_TO]->(co:Organisation)
                         OPTIONAL MATCH (m)-[rr:HAS_ROLE]->(r:MembershipRole)
                         WITH    canonical,
-                                { id:o.uuid, types:labels(o), prefLabel:o.prefLabel} as o,
+                                { id:coalesce(co.prefUUID, o.uuid), types:coalesce(labels(co), labels(o)), prefLabel:coalesce(co.prefLabel, o.prefLabel)} as o,
                                 { id:m.uuid, types:labels(m), prefLabel:m.prefLabel, title:m.title, changeEvents:[{startedAt:m.inceptionDate}, {endedAt:m.terminationDate}] } as m,
                                 { id:r.uuid, types:labels(r), prefLabel:r.prefLabel, changeEvents:[{startedAt:rr.inceptionDate}, {endedAt:rr.terminationDate}] } as r
-                        WITH canonical, m, o, collect(r) as r ORDER BY o.uuid DESC
+                        WITH canonical, m, o, collect(r) as r ORDER BY o.id DESC
                         WITH canonical, collect({m:m, o:o, r:r}) as m
                         WITH m, { ID:canonical.prefUUID, types:labels(canonical), prefLabel:canonical.prefLabel, labels:canonical.aliases,
 								birthYear:canonical.birthYear, salutation:canonical.salutation, emailAddress:canonical.emailAddress,
@@ -163,12 +164,13 @@ func (pcw CypherDriver) ReadOldConcordanceModel(uuid string, transactionID strin
                         MATCH (identifier)-[:IDENTIFIES]->(p:Person)
                         OPTIONAL MATCH (p)<-[:HAS_MEMBER]-(m:Membership)
                         OPTIONAL MATCH (m)-[:HAS_ORGANISATION]->(o:Organisation)
+						OPTIONAL MATCH (o)-[:EQUIVALENT_TO]->(co:Organisation)
                         OPTIONAL MATCH (m)-[rr:HAS_ROLE]->(r:MembershipRole)
                         WITH    p,
-                                { id:o.uuid, types:labels(o), prefLabel:o.prefLabel} as o,
+                                { id:coalesce(co.prefUUID, o.uuid), types:coalesce(labels(co), labels(o)), prefLabel:coalesce(co.prefLabel, o.prefLabel)} as o,
                                 { id:m.uuid, types:labels(m), prefLabel:m.prefLabel, title:m.title, changeEvents:[{startedAt:m.inceptionDate}, {endedAt:m.terminationDate}] } as m,
                                 { id:r.uuid, types:labels(r), prefLabel:r.prefLabel, changeEvents:[{startedAt:rr.inceptionDate}, {endedAt:rr.terminationDate}] } as r
-                        WITH p, m, o, collect(r) as r ORDER BY o.uuid DESC
+                        WITH p, m, o, collect(r) as r ORDER BY o.id DESC
                         WITH p, collect({m:m, o:o, r:r}) as m
                         WITH m, { id:p.uuid, types:labels(p), prefLabel:p.prefLabel, labels:p.aliases,
 												     birthYear:p.birthYear, salutation:p.salutation, emailAddress:p.emailAddress,
@@ -327,4 +329,4 @@ func handleEmptyError(e error, defaultMessage string) error {
 	neoError.Message = defaultMessage
 
 	return neoError
-}
+}
diff --git a/people/cypher_test.go b/people/cypher_test.go
@@ -11,6 +11,7 @@ import (
 
 	"time"
 
+	"errors"
 	"github.com/Financial-Times/base-ft-rw-app-go/baseftrwapp"
 	"github.com/Financial-Times/concepts-rw-neo4j/concepts"
 	"github.com/Financial-Times/memberships-rw-neo4j/memberships"
@@ -20,7 +21,6 @@ import (
 	"github.com/Financial-Times/roles-rw-neo4j/roles"
 	"github.com/jmcvetta/neoism"
 	"github.com/stretchr/testify/assert"
-	"errors"
 )
 
 const (
@@ -171,6 +171,18 @@ func TestNewModelWithFullyNewModelMembershipRelatedConcepts(t *testing.T) {
 
 // TODO: When we concord to Factset we will need to handle a mixture of old model and new model
 
+// New Model and backwards compatibility tests
+func TestNewModelWithNewModelMembershipWithNonCanonicalOrganisation(t *testing.T) {
+	defer cleanDB(db, t, "7d0738b1-0ea2-47cb-bb82-e86744b389f0", "016037d4-5aa8-11e8-b45a-da24cd01f044", "7ceeafe5-9f9a-4315-b3da-a5b4b69c013a", "cd80a11e-5aa8-11e8-b45a-da24cd01f044", "e7d54d96-e653-4349-aec9-eb8ab601d62d", "c02d18d9-e98e-4bf4-b437-b1a5ea85b999", "b96f6e56-dc49-4332-80b8-58662ea21e89")
+	writeJSONToConceptsService(t, "./fixtures/newModel/MembershipRole-SmartyPants-7d0738b1-0ea2-47cb-bb82-e86744b389f0.json")
+	writeJSONToConceptsService(t, "./fixtures/newModel/Organisation-ShirleysSparkplugs-016037d4-5aa8-11e8-b45a-da24cd01f044.json")
+	writeJSONToConceptsService(t, "./fixtures/newModel/Person-Shirley-Rooney-7ceeafe5-9f9a-4315-b3da-a5b4b69c013a.json")
+	writeJSONToConceptsService(t, "./fixtures/newModel/Membership-ToNonCanonicalOrg-cd80a11e-5aa8-11e8-b45a-da24cd01f044.json")
+
+	person := readJSONtoPerson(t, "./fixtures/outputJSON/TestNewModelWithNewModelMembershipToNonCanonicalOrg.json")
+	readConceptAndCompare(t, person, "7ceeafe5-9f9a-4315-b3da-a5b4b69c013a")
+}
+
 func TestNewModelWithThingOnlyMembershipRelatedConceptsDoesNotReturnMembership(t *testing.T) {
 	// New model to org/person/role that is only a Thing - No membership should be returned
 

diff --git a/.../fixtures/newModel/Membership-ToNonCanonicalOrg-cd80a11e-5aa8-11e8-b45a-da24cd01f044.json b/.../fixtures/newModel/Membership-ToNonCanonicalOrg-cd80a11e-5aa8-11e8-b45a-da24cd01f044.json
@@ -0,0 +1,20 @@
+{
+  "prefUUID": "cd80a11e-5aa8-11e8-b45a-da24cd01f044",
+  "prefLabel": "Head of Repair",
+  "type": "Membership",
+  "membershipRoles" : ["7d0738b1-0ea2-47cb-bb82-e86744b389f0"],
+  "organisationUUID": "b96f6e56-dc49-4332-80b8-58662ea21e89",
+  "personUUID": "7ceeafe5-9f9a-4315-b3da-a5b4b69c013a",
+  "sourceRepresentations": [
+    {
+      "uuid": "cd80a11e-5aa8-11e8-b45a-da24cd01f044",
+      "prefLabel": "Head of Repair",
+      "authority": "Smartlogic",
+      "authorityValue": "cd80a11e-5aa8-11e8-b45a-da24cd01f044",
+      "type": "Membership",
+      "membershipRoles" : ["7d0738b1-0ea2-47cb-bb82-e86744b389f0"],
+      "organisationUUID": "b96f6e56-dc49-4332-80b8-58662ea21e89",
+      "personUUID": "7ceeafe5-9f9a-4315-b3da-a5b4b69c013a"
+    }
+  ]
+}
diff --git a/...xtures/newModel/Organisation-ShirleysSparkplugs-016037d4-5aa8-11e8-b45a-da24cd01f044.json b/...xtures/newModel/Organisation-ShirleysSparkplugs-016037d4-5aa8-11e8-b45a-da24cd01f044.json
@@ -0,0 +1,20 @@
+{
+  "prefUUID": "016037d4-5aa8-11e8-b45a-da24cd01f044",
+  "prefLabel": "Shirleys Sparkplugs",
+  "type": "Organisation",
+  "sourceRepresentations": [
+    {
+      "uuid": "b96f6e56-dc49-4332-80b8-58662ea21e89",
+      "prefLabel": "ABC Automotive",
+      "authority": "TME",
+      "authorityValue": "90210",
+      "type": "Organisation"
+    }, {
+      "uuid": "016037d4-5aa8-11e8-b45a-da24cd01f044",
+      "prefLabel": "Shirleys Sparkplugs",
+      "type": "Organisation",
+      "authority": "Smartlogic",
+      "authorityValue": "016037d4-5aa8-11e8-b45a-da24cd01f044"
+    }
+  ]
+}
diff --git a/people/fixtures/outputJSON/TestNewModelWithNewModelMembershipToNonCanonicalOrg.json b/people/fixtures/outputJSON/TestNewModelWithNewModelMembershipToNonCanonicalOrg.json
@@ -0,0 +1,51 @@
+{
+  "id": "http://api.ft.com/things/7ceeafe5-9f9a-4315-b3da-a5b4b69c013a",
+  "apiUrl": "http://api.ft.com/people/7ceeafe5-9f9a-4315-b3da-a5b4b69c013a",
+  "prefLabel": "Shirley Rooney",
+  "types": [
+    "http://www.ft.com/ontology/core/Thing",
+    "http://www.ft.com/ontology/concept/Concept",
+    "http://www.ft.com/ontology/person/Person"
+  ],
+  "directType": "http://www.ft.com/ontology/person/Person",
+  "memberships": [
+    {
+      "title": "Head of Repair",
+      "types": [
+        "http://www.ft.com/ontology/core/Thing",
+        "http://www.ft.com/ontology/concept/Concept",
+        "http://www.ft.com/ontology/organisation/Membership"
+      ],
+      "directType": "http://www.ft.com/ontology/organisation/Membership",
+      "organisation": {
+        "id": "http://api.ft.com/things/016037d4-5aa8-11e8-b45a-da24cd01f044",
+        "apiUrl": "http://api.ft.com/organisations/016037d4-5aa8-11e8-b45a-da24cd01f044",
+        "prefLabel": "Shirleys Sparkplugs",
+        "types": [
+          "http://www.ft.com/ontology/core/Thing",
+          "http://www.ft.com/ontology/concept/Concept",
+          "http://www.ft.com/ontology/organisation/Organisation"
+        ],
+        "directType": "http://www.ft.com/ontology/organisation/Organisation"
+      },
+      "roles": [
+        {
+          "id": "http://api.ft.com/things/7d0738b1-0ea2-47cb-bb82-e86744b389f0",
+          "apiUrl": "http://api.ft.com/things/7d0738b1-0ea2-47cb-bb82-e86744b389f0",
+          "prefLabel": "Smarty Pants",
+          "types": [
+            "http://www.ft.com/ontology/core/Thing",
+            "http://www.ft.com/ontology/concept/Concept",
+            "http://www.ft.com/ontology/MembershipRole"
+          ],
+          "directType": "http://www.ft.com/ontology/MembershipRole"
+        }
+      ]
+    }
+  ],
+  "emailAddress": "test@example.com",
+  "twitterHandle": "@something",
+  "descriptionXML": "Some text containing <strong>markup</strong>",
+  "_imageUrl": "http://someimage.jpg",
+  "imageUrl": "http://someimage.jpg"
+}