MBS-11371: Group members in JSON-LD by date, do not squash (metabrainz#2129)

The code used to squash all "member of" rels between a group
and a person into one relationship for JSON-LD, whether that made sense
or not (for example, it'd happily squash two relationships into
"start date 2020, end date 2010").
We talked to Google people and they said they thought
having one role entry per date set was better, so this code
does that. We still group several relationships from the same date
(to display "guitar, piano" together if the same member played both
with the same dates).

Author: reosarevok

lib/MusicBrainz/Server/WebService/Serializer/JSON/LD/Artist.pm

@@ -41,7 +41,18 @@ around serialize => sub {
         if (@members) {
             my %seen_members;
             for my $member (@members) {
-                $seen_members{$member->target->gid} = member_relationship($member, $inc, $stash, $seen_members{$member->target->gid});
+                # We separate different dates, but keep together
+                # different roles with the same dates
+                my $key = $member->target->gid;
+                my $begin_date = $member->link->begin_date;
+                my $end_date = $member->link->end_date;
+                if ($begin_date && $begin_date->defined_run) {
+                    $key .= '-begin-' . join('-', $begin_date->defined_run);
+                }
+                if ($end_date && $end_date->defined_run) {
+                    $key .= '-end-' . join('-', $end_date->defined_run);
+                }
+                $seen_members{$key} = member_relationship($member, $inc, $stash, $seen_members{$key});
             }
             $ret->{member} = [ map { $seen_members{$_} } sort keys %seen_members ];
         }
@@ -83,20 +94,15 @@ sub member_relationship {
         $is_new = 1;
     }
 
-    # XXX: given two rels with different dates, this assumes it should take the
-    # earliest start date and the latest end date, assuming undef as
-    # indefinitely in the future. This may not be correct, as different roles
-    # may have different dates associated with them, but it seems likely to be
-    # the most accurate representation of the membership dates
-    if ($relationship->link->begin_date && $relationship->link->begin_date->defined_run) {
+    if ($is_new && $relationship->link->begin_date && $relationship->link->begin_date->defined_run) {
         my @run = $relationship->link->begin_date->defined_run;
         my $date = PartialDate->new(year => $run[0], month => $run[1], day => $run[2]);
-        $ret->{startDate} = $date->format if ($is_new || !$ret->{startDate} || PartialDate->new($ret->{startDate}) > $date);
+        $ret->{startDate} = $date->format;
     }
-    if ($relationship->link->end_date && $relationship->link->end_date->defined_run) {
+    if ($is_new && $relationship->link->end_date && $relationship->link->end_date->defined_run) {
         my @run = $relationship->link->end_date->defined_run;
         my $date = PartialDate->new(year => $run[0], month => $run[1], day => $run[2]);
-        $ret->{endDate} = $date->format if ($is_new || ($ret->{endDate} && PartialDate->new($ret->{endDate}) < $date));
+        $ret->{endDate} = $date->format;
     }
 
     # XXX: role names are instruments (or, where available, credits), not

t/lib/t/MusicBrainz/Server/Controller/Artist/Show.pm

@@ -120,10 +120,15 @@ test 'Embedded JSON-LD `member` property' => sub {
                (3, 'efac67ce-33ae-4949-8fc8-3d2aeafcbefb', 'Person B', 'Person B');
 
     INSERT INTO link (id, link_type, begin_date_year, end_date_year)
-        VALUES (1, 103, 2001, 2002), (2, 103, 1999, 2002);
+        VALUES (1, 103, 2001, 2002), (2, 103, 1999, 2002),
+               (3, 103, 1999, 2002), (4, 103, 2005, NULL);
 
     INSERT INTO l_artist_artist (id, link, entity0, entity1, entity0_credit)
-        VALUES (1, 1, 2, 1, 'A.'), (2, 2, 3, 1, 'B.');
+        VALUES (1, 1, 2, 1, 'A.'), (2, 2, 3, 1, 'B.'),
+               (3, 3, 3, 1, 'B.'), (4, 4, 3, 1, 'B.');
+
+    INSERT INTO link_attribute (link, attribute_type)
+        VALUES (2, 229), (3, 125), (4, 229);
 EOSQL
 
     $mech->get_ok('/artist/dcb48a49-b17d-49b9-aee5-4f168d8004d9');
@@ -152,7 +157,17 @@ EOSQL
                 'endDate' => '2002',
                 '@type' => 'OrganizationRole',
                 'startDate' => '1999',
-                'roleName' => []
+                'roleName' => ['guitar','drums']
+            },
+            {
+                'member' => {
+                    '@id' => 'http://musicbrainz.org/artist/efac67ce-33ae-4949-8fc8-3d2aeafcbefb',
+                    'name' => 'Person B',
+                    '@type' => 'MusicGroup'
+                },
+                '@type' => 'OrganizationRole',
+                'startDate' => '2005',
+                'roleName' => 'guitar'
             }
         ],
         '@context' => 'http://schema.org'