Initiatives: add EdgeSpec support to Initiative type (#1764)

Because the `Initiative` type now supports `EdgeSpec`, we're no longer discarding entries when converting between `InitiativeFile` and `Initiative`. To make this commit smaller and easier to review, we're not yet adding support to add `NodeEntry` to the graph though, instead `createWeightedGraph` ignores the entries for now.
2025-01-12 21:54:37 +00:00 · 2020-04-27 18:31:10 +02:00 · 2020-04-27 18:31:10 +02:00 · 9e407b3cac
commit 9e407b3cac
parent 0a671025d7
6 changed files with 133 additions and 54 deletions
--- a/src/plugins/initiatives/snapshots/initiativesDirectory.test.js.snap
+++ b/src/plugins/initiatives/snapshots/initiativesDirectory.test.js.snap
@ -127,20 +127,32 @@ exports[`plugins/initiatives/initiativesDirectory loadDirectory should handle an
      \\"http://foo.bar/A/champ\\"
    ],
    \\"completed\\": true,
-    \\"contributions\\": [
-      \\"http://foo.bar/A/contrib\\"
-    ],
-    \\"dependencies\\": [
-      \\"http://foo.bar/A/dep\\"
-    ],
+    \\"contributions\\": {
+      \\"entries\\": [
+      ],
+      \\"urls\\": [
+        \\"http://foo.bar/A/contrib\\"
+      ]
+    },
+    \\"dependencies\\": {
+      \\"entries\\": [
+      ],
+      \\"urls\\": [
+        \\"http://foo.bar/A/dep\\"
+      ]
+    },
    \\"id\\": [
      \\"INITIATIVE_FILE\\",
      \\"http://example.com/initiatives\\",
      \\"initiative-A.json\\"
    ],
-    \\"references\\": [
-      \\"http://foo.bar/A/ref\\"
-    ],
+    \\"references\\": {
+      \\"entries\\": [
+      ],
+      \\"urls\\": [
+        \\"http://foo.bar/A/ref\\"
+      ]
+    },
    \\"timestampMs\\": 1578520917711,
    \\"title\\": \\"Initiative A\\",
    \\"weight\\": {
@ -153,20 +165,32 @@ exports[`plugins/initiatives/initiativesDirectory loadDirectory should handle an
      \\"http://foo.bar/B/champ\\"
    ],
    \\"completed\\": false,
-    \\"contributions\\": [
-      \\"http://foo.bar/B/contrib\\"
-    ],
-    \\"dependencies\\": [
-      \\"http://foo.bar/B/dep\\"
-    ],
+    \\"contributions\\": {
+      \\"entries\\": [
+      ],
+      \\"urls\\": [
+        \\"http://foo.bar/B/contrib\\"
+      ]
+    },
+    \\"dependencies\\": {
+      \\"entries\\": [
+      ],
+      \\"urls\\": [
+        \\"http://foo.bar/B/dep\\"
+      ]
+    },
    \\"id\\": [
      \\"INITIATIVE_FILE\\",
      \\"http://example.com/initiatives\\",
      \\"initiative-B.json\\"
    ],
-    \\"references\\": [
-      \\"http://foo.bar/B/ref\\"
-    ],
+    \\"references\\": {
+      \\"entries\\": [
+      ],
+      \\"urls\\": [
+        \\"http://foo.bar/B/ref\\"
+      ]
+    },
    \\"timestampMs\\": 1578520917722,
    \\"title\\": \\"Initiative B\\",
    \\"weight\\": {
@ -179,17 +203,56 @@ exports[`plugins/initiatives/initiativesDirectory loadDirectory should handle an
      \\"http://foo.bar/C/champ\\"
    ],
    \\"completed\\": false,
-    \\"contributions\\": [
-    ],
-    \\"dependencies\\": [
-    ],
+    \\"contributions\\": {
+      \\"entries\\": [
+        {
+          \\"contributors\\": [
+            \\"https://foo.bar/C/contrib-user\\"
+          ],
+          \\"key\\": \\"add-test-contrib\\",
+          \\"timestampMs\\": 1578520917733,
+          \\"title\\": \\"Add test contrib\\",
+          \\"weight\\": 10
+        }
+      ],
+      \\"urls\\": [
+      ]
+    },
+    \\"dependencies\\": {
+      \\"entries\\": [
+        {
+          \\"contributors\\": [
+            \\"https://foo.bar/C/dep-user\\"
+          ],
+          \\"key\\": \\"add-test-dependency\\",
+          \\"timestampMs\\": 1578002517700,
+          \\"title\\": \\"Add test dependency\\",
+          \\"weight\\": null
+        }
+      ],
+      \\"urls\\": [
+      ]
+    },
    \\"id\\": [
      \\"INITIATIVE_FILE\\",
      \\"http://example.com/initiatives\\",
      \\"initiative-C.json\\"
    ],
-    \\"references\\": [
-    ],
+    \\"references\\": {
+      \\"entries\\": [
+        {
+          \\"contributors\\": [
+            \\"https://foo.bar/C/ref-user\\"
+          ],
+          \\"key\\": \\"add-test-reference\\",
+          \\"timestampMs\\": 1578520917733,
+          \\"title\\": \\"Add test reference\\",
+          \\"weight\\": null
+        }
+      ],
+      \\"urls\\": [
+      ]
+    },
    \\"timestampMs\\": 1578520917733,
    \\"title\\": \\"Initiative C\\",
    \\"weight\\": {
--- a/src/plugins/initiatives/createGraph.js
+++ b/src/plugins/initiatives/createGraph.js
@ -12,6 +12,7 @@ import {type WeightedGraph as WeightedGraphT} from "../../core/weightedGraph";
 import * as WeightedGraph from "../../core/weightedGraph";
 import type {NodeWeight} from "../../core/weights";
 import type {ReferenceDetector, URL} from "../../core/references";
+import type {EdgeSpec} from "./edgeSpec";
 import type {Initiative, InitiativeRepository} from "./initiative";
 import {addressFromId} from "./initiative";
 import {
@ -90,9 +91,14 @@ export function createWeightedGraph(

    // Generic approach to adding edges when the reference detector has a hit.
    const edgeHandler = (
-      urls: $ReadOnlyArray<URL>,
+      edges: $ReadOnlyArray<URL> | EdgeSpec,
      createEdge: EdgeFactoryT
    ) => {
+      // TODO: this is a temporary implementation, which takes an EdgeSpec and
+      // just takes the $ReadOnlyArray<URL>. This is only done to allow support
+      // for graphing NodeEntries to be added in a separate commit.
+      const urls = Array.isArray(edges) ? edges : edges.urls;
+
      for (const url of urls) {
        const addr = refs.addressFromUrl(url);
        if (!addr) continue;
--- a/src/plugins/initiatives/createGraph.test.js
+++ b/src/plugins/initiatives/createGraph.test.js
@ -26,9 +26,9 @@ function _createInitiative(overrides?: $Shape<Initiative>): Initiative {
    title: "Unset test initiative",
    timestampMs: Timestamp.fromNumber(123),
    completed: false,
-    dependencies: [],
-    references: [],
-    contributions: [],
+    contributions: {urls: [], entries: []},
+    dependencies: {urls: [], entries: []},
+    references: {urls: [], entries: []},
    champions: [],
    ...overrides,
  };
@ -237,7 +237,10 @@ describe("plugins/initiatives/createGraph", () => {
        // Given
        const {repo, refs} = example();
        repo.addInitiative({
-          dependencies: ["https://example.com/1"],
+          dependencies: {
+            urls: ["https://example.com/1"],
+            entries: [],
+          },
        });

        // When
@ -253,7 +256,10 @@ describe("plugins/initiatives/createGraph", () => {
        // Given
        const {repo, refs} = example();
        repo.addInitiative({
-          references: ["https://example.com/2"],
+          references: {
+            urls: ["https://example.com/2"],
+            entries: [],
+          },
        });

        // When
@ -269,7 +275,10 @@ describe("plugins/initiatives/createGraph", () => {
        // Given
        const {repo, refs} = example();
        repo.addInitiative({
-          contributions: ["https://example.com/3"],
+          contributions: {
+            urls: ["https://example.com/3"],
+            entries: [],
+          },
        });

        // When
@ -304,7 +313,10 @@ describe("plugins/initiatives/createGraph", () => {
        const {repo, refs} = example();
        refs.addReference("https://example.com/1", exampleNodeAddress(1));
        repo.addInitiative({
-          dependencies: ["https://example.com/1", "https://example.com/99"],
+          dependencies: {
+            urls: ["https://example.com/1", "https://example.com/99"],
+            entries: [],
+          },
        });

        // When
@ -335,7 +347,10 @@ describe("plugins/initiatives/createGraph", () => {
        const {repo, refs} = example();
        refs.addReference("https://example.com/2", exampleNodeAddress(2));
        repo.addInitiative({
-          references: ["https://example.com/2", "https://example.com/99"],
+          references: {
+            urls: ["https://example.com/2", "https://example.com/99"],
+            entries: [],
+          },
        });

        // When
@ -366,7 +381,10 @@ describe("plugins/initiatives/createGraph", () => {
        const {repo, refs} = example();
        refs.addReference("https://example.com/3", exampleNodeAddress(3));
        repo.addInitiative({
-          contributions: ["https://example.com/3", "https://example.com/99"],
+          contributions: {
+            urls: ["https://example.com/3", "https://example.com/99"],
+            entries: [],
+          },
        });

        // When
--- a/src/plugins/initiatives/initiative.js
+++ b/src/plugins/initiatives/initiative.js
@ -4,6 +4,7 @@ import {type URL} from "../../core/references";
 import {type NodeAddressT, NodeAddress} from "../../core/graph";
 import {type NodeWeight} from "../../core/weights";
 import {type TimestampMs} from "../../util/timestamp";
+import {type EdgeSpec} from "./edgeSpec";
 import {initiativeNodeType} from "./declaration";

 // Composite ID, used as input for NodeAddressT.
@ -48,9 +49,9 @@ export type Initiative = {|
  +timestampMs: TimestampMs,
  +weight?: InitiativeWeight,
  +completed: boolean,
-  +dependencies: $ReadOnlyArray<URL>,
-  +references: $ReadOnlyArray<URL>,
-  +contributions: $ReadOnlyArray<URL>,
+  +dependencies: EdgeSpec,
+  +references: EdgeSpec,
+  +contributions: EdgeSpec,
  +champions: $ReadOnlyArray<URL>,
 |};

--- a/src/plugins/initiatives/initiativesDirectory.js
+++ b/src/plugins/initiatives/initiativesDirectory.js
@ -7,11 +7,11 @@ import {type URL} from "../../core/references";
 import {type NodeAddressT} from "../../core/graph";
 import * as Timestamp from "../../util/timestamp";
 import {compatReader} from "../../backend/compatIO";
+import {normalizeEdgeSpec} from "./edgeSpec";
 import {
  type ReferenceDetector,
  MappedReferenceDetector,
 } from "../../core/references";
-import {type EdgeSpecJson} from "./edgeSpec";
 import {
  type Initiative,
  type InitiativeRepository,
@ -165,9 +165,9 @@ export function _convertToInitiatives(
      id: initiativeFileId(directory, fileName),
      timestampMs,
      champions: champions || [],
-      contributions: _lossyURLFromEdgeSpecJson(contributions),
-      dependencies: _lossyURLFromEdgeSpecJson(dependencies),
-      references: _lossyURLFromEdgeSpecJson(references),
+      contributions: normalizeEdgeSpec(contributions, timestampMs),
+      dependencies: normalizeEdgeSpec(dependencies, timestampMs),
+      references: normalizeEdgeSpec(references, timestampMs),
    };

    initiatives.push(initiative);
@ -175,15 +175,6 @@ export function _convertToInitiatives(
  return initiatives;
 }

-// TODO: this is a temporary function, which reverts an ?EdgeSpecJson to just
-// $ReadOnlyArray<URL>. It only exists to allow the `Initiative` type to add
-// support for EdgeSpec in a separate commit.
-export function _lossyURLFromEdgeSpecJson(
-  json?: EdgeSpecJson
-): $ReadOnlyArray<URL> {
-  return (json || {}).urls || [];
-}
-
 // Creates a reference map using `initiativeFileURL`.
 export function _createReferenceMap(
  initiatives: $ReadOnlyArray<Initiative>
--- a/src/plugins/initiatives/initiativesDirectory.test.js
+++ b/src/plugins/initiatives/initiativesDirectory.test.js
@ -7,6 +7,7 @@ import stringify from "json-stable-stringify";
 import {MappedReferenceDetector} from "../../core/references";
 import * as Timestamp from "../../util/timestamp";
 import {type Initiative, createId, addressFromId} from "./initiative";
+import {normalizeEdgeSpec} from "./edgeSpec";
 import {
  type InitiativesDirectory,
  loadDirectory,
@ -16,7 +17,6 @@ import {
  _validateUrl,
  _convertToInitiatives,
  _createReferenceMap,
-  _lossyURLFromEdgeSpecJson,
 } from "./initiativesDirectory";
 import {type InitiativeFile} from "./initiativeFile";

@ -55,9 +55,9 @@ const exampleInitiative = (remoteUrl: string, fileName: string): Initiative => {
    id: createId("INITIATIVE_FILE", remoteUrl, fileName),
    timestampMs,
    champions: champions || [],
-    contributions: _lossyURLFromEdgeSpecJson(contributions),
-    dependencies: _lossyURLFromEdgeSpecJson(dependencies),
-    references: _lossyURLFromEdgeSpecJson(references),
+    contributions: normalizeEdgeSpec(contributions, timestampMs),
+    dependencies: normalizeEdgeSpec(dependencies, timestampMs),
+    references: normalizeEdgeSpec(references, timestampMs),
  };
 };