Currently, the cred explorer is a submodule of `app`. This is somewhat
confusing, as `app` is essentially our homepage, and the explorer is a
standalone React application which happens to get embedded in our
homepage. This commit pulls the explorer from `app/credExplorer/` into
`explorer/`, which is a better organization.
The `app/adapters` were actually only used by the cred explorer, so
those files have been moved to `explorer/adapters`. We should rename
them from "App Adapters" to "Explorer Adapters", but I didn't do that in
this commit so as to minimize the (already substantial) size of the
change.
Also, we should rename `app/` to `homepage/` in a subsequent commit.
I encountered a nasty Flow bug, which I fixed with help from @wchargin.
The result is extra annotations on the demo and fallback dynamic
adapters (so that the `static()` method is type annotated).
Test plan: This change is massive, but it's just a rename. `yarn test`
suffices.
I'm planning to pull `credExplorer` out of `app` and into its own
top-level module. This is a bit awkward, as `credExplorer` depends on
a lot of little modules that are currently collected in `app/`.
To resolve this, I pull all of these little utility modules into
`webutil/`. It's not a totally principled grouping, but it's quite
convenient and keeps these rarely changing modules out of the way.
Test plan: It's a file move, `yarn test` suffices.
The logic for converting weights into an edge evaluator should not be
coupled to the frontend application.
Progress towards #967.
Test plan: Very straightforward rename; `yarn test` suffices.
Now that the `analysis` module owns the Node and Edge types, it should
own the "fallback plugin" too. (Note that it's not actually a plugin,
though it somewhat acts like one.)
We now declare the fallback type in `analysis`, along with a fallback
analysis adapter. `app/adapters` then declares a fallback app adapter.
Test plan: `yarn test`
Progress towards #967.
There's a folder called `app/credExplorer/weights` which contains the
type specification for weights (for PageRank configurability), and also
contains frontend code for specifying those weights. This commit creates
a `weights` module under `analysis` which will contain just the logic
for specifying and using the weights, without any frontend
consideration.
It's mostly a port of the existing logic in `credExplorer/weights`, with
the caveat that app adapter related concepts have been removed, in favor
of referencing the declaration instead.
We then remove the duplicated logic and re-route imports.
Test plan: `yarn test`
* Add the demo plugin
This ports the ad-hoc demo adapter defined in
`src/app/adapters/demoAdapters.js` into its own demo plugin.
This has the benefit that the demo plugin can now be depended on outside
the app module, e.g. for the analysis module as well. Correspondingly,
I've added a demo analysis adapter.
Test plan: `yarn test`. Note that no unit tests were added, as the demo
plugin is trivial.
* Delete `src/app/adapters/demoAdapters.js`
Now that we have an explicit demo plugin at `src/plugins/demo/`, we can
remove the legacy declaration of that plugin within the `app` module.
This commit deletes the old version, and re-writes all references to
point to the standalone plugin.
Test plan: `yarn test`
Summary:
It is time. (Replaced with #622.)
Test Plan:
Running `yarn flow` suffices. Running `yarn test --full` also passes.
wchargin-branch: remove-legacy-graphql
Summary:
This test has data in the old format, and uses the `RelationalView`
method that automatically translates it. As we prepare to delete that
code, we upgrade the underlying format of this test data. The end code
is nicer to read, too (e.g., we don’t need the `connection` helper
function).
Recommend reviewing with `git show -b`.
Test Plan:
Running `yarn test` suffices.
wchargin-branch: mentionsAuthorReference-remove-legacy-graphql
Summary:
A number of modules depended on the legacy `github/graphql.js` module
solely to get at the `Reactions` enum object. As of #961, that object is
exposed from the much lighter-weight `graphqlTypes.js`. This patch
switches over the relevant imports, reducing our dependencies on this
legacy module and its large bundle size.
Test Plan:
It suffices to run `yarn flow` and verify that the two values being
imported are identical.
wchargin-branch: github-use-generated-enums
Summary:
We have a `const Reactions` convenience enum in `github/graphql.js`.
That value is useful, but that module is slated to die. This commit
extends our Flow type generation script to include these values.
Test Plan:
Existing unit tests suffice.
wchargin-branch: schema-generate-enums
For #704, we're adding plugin adapters that are specific to the needs of
the analysis module. They have a simple scope: they just provide a way
to get the declaration, and to load the corresponding graph.
Adapters for the `git` and `github` plugins have been implemented, along
with unit tests.
Test plan: `yarn test` suffices.
Summary:
Fixes#953. See that issue for context.
Test Plan:
Unit tests updated. To see the change in action, load the SourceCred
data and expand @decentralion’s commits-authored to find commits that
were merged into non-`master` branches. Note that these commits are
rendered correctly (in the same way that they were before this patch),
and that there is no console error (new as of this patch).
![Screenshot](https://user-images.githubusercontent.com/4317806/47805669-1f98b580-dcf5-11e8-8683-8ee91f7f478a.png)
wchargin-branch: git-no-warn-on-unknown-commit
Test Plan:
Remove the SourceCred output directory, run `yarn backend`, and load
data for `sourcecred/example-github` and `sourcecred/sourcecred`. Then,
run `yarn start` and note that the cred explorer still works. Finally,
note that `yarn test --full` passes.
wchargin-branch: release-v0.2.0
Now that we're planning to add adapters for the `analysis` module, we
should rename the `PluginAdapter` to make it clear that it's scoped for
`app`.
Test plan: `yarn test` suffices
The plugin adapters are specific to `app/` and have logic for fetching
data from the backend, producing React nodes for descriptions, et
cetera.
However, they also have information that is generic to the plugin
itself: its name, its node/edge prefixes, and its types.
This method factors out the generic info into a `PluginDeclaration`,
which is a type (rather than an interface). Then, the plugin adapter has
a `declaration` method which returns the declaration.
Current users of the plugin adapters get additional mechanical
complexity because they need to call `.declaration().property` rather
than `.property()`, but this is not too significant.
The main benefit is that #704 is unblocked, as the cli `analyze` command
will be able to get necessary information from the declaration. As an
added benefit, the organization of plugin code is somewhat improved.
Test plan: `yarn test` sufficies, but I also ran `yarn start` and
checked the UI behavior to be safe.
The `core/attribution/` folder has some code that really is "core" in
that it deals with very basic concepts around converting graphs to
markov chains and running PageRank on them, and some code that is less
"core", like for normalizing scores and doing analysis on them.
To make progresson #704, we need an intermediary directory that has
analysis-related code that is e.g. aware of Node and Edge types, and
weights on those types, and can use them to run weight-informed
PageRank. That code shouldn't live in the app directory (since it is not
coupled to the frontend rendering), but also shouldn't live in core
(since "core" is basically finalized code with fully baked abstractions,
and per #710, this is not true of the node/edge type system).
Thus, I've decided to create the `analysis` directory. To get that
directory started, I've moved the non-core code in `core/attribution/`
to `analysis/`.
Test plan: `yarn test` passes, which is all we need, since this is a
straightforward file rename.
The `analyze` command is the first step towards #704 and #703. When
fully implemented, it will run PageRank for a loaded repository,
generating a complete graph and cred attribution.
For now, this just adds a scaffold. It does basic argument parsing, and
has help text, but the actual command is not yet implemented.
Test plan:
Unit tests verify that the analyze command is hooked into `sourcecred`
and `sourcecred help`, and that it responds to the `--help` command and
parses its arguments appropriately.
Summary:
As of this commit, `node ./bin/sourcecred.js load` uses the Mirror code,
and the legacy continuation-fetching code is not included in the
`sourcecred.js` bundle.
We do not yet perform the commit prefetching described in #923. The code
should be plenty fast for repositories that merge pull requests at least
occasionally.
Test Plan:
Running `yarn test --full` passes. Loading `sourcecred/sourcecred` works
and generates a reasonable credit attribution. Loading it again
completes immediately.
wchargin-branch: fetchGithubRepo-mirror
Summary:
This makes significant progress toward #923. As of this commit, it is
possible to use the Mirror module for the whole loading pipeline. This
process may be slow for repositories that do not use pull requests at
all (more precisely, that have large connected commit subgraphs none of
whose nodes is the merge commit of a pull request; see #920 for details)
so it is not yet the default codepath.
Test Plan:
Existing unit tests should suffice. For extra testing, I’ve added a
script that fetches a repository both via the old continuations logic
and the new Mirror logic, then constructs relational views and checks
whether the data is the same. For `example-github`, the views are
identical. For `sourcecred`, they are not: the old continuations logic
erroneously omits two commits, which the Mirror logic includes.
You can run the test like this:
```
$ node ./bin/testContinuations.js \
> sourcecred sourcecred MDEwOlJlcG9zaXRvcnkxMjAxNDU1NzA= \
> /tmp/continuations.json /tmp/mirror.json \
> 2> >(jq . >&2)
{
"child": "0d38dde23a6de831315f3643a7d2bc15e8df7678",
"parent": "cb8ba0eaa1abc1f921e7165bb19e29b40723ce65",
"type": "UNKNOWN_PARENT_OID"
}
{
"child": "d152f48ce4c2ed1d046bf6ed4f139e7e393ea660",
"parent": "de7a8723963d9cd0437ef34f5942a071b850c0e7",
"type": "UNKNOWN_PARENT_OID"
}
Different. Saving to disk...
```
Use `diff -u <(jq . /tmp/continuations.json) <(jq . /tmp/mirror.json)`
to inspect the differences, and note that exactly the two missing
commits have been added and that there are no other changes. (The diff
is small: just 51 lines of nicely formatted JSON.) The full log is here:
<https://gist.github.com/wchargin/e159cac9dcf3cc3b1efbd54f59e24e0b>
I also generated the `sourcecred/sourcecred` cred attribution and viewed
it with `yarn start`, which seems to work fine.
wchargin-branch: relationalview-new-data-format
Summary:
An upcoming commit will happen to change the order in which commits are
ingested. This is not an observable change, and should not cause a
snapshot failure.
Test Plan:
Inspection.
wchargin-branch: relationalview-snapshots-order-invariant
Summary:
This implements the translation module described in #923. See that issue
for context.
Test Plan:
This is a mostly straightforward translation from one strongly typed
data structure to another, so Flow handles most of it.
As a check on the snapshot, run:
```
$ grep -e oid -e target -e mergeCommit \
> src/plugins/github/__snapshots__/translateContinuations.test.js.snap
"target": Object {
"oid": "6bd1b4c0b719c22c688a74863be07a699b7b9b34",
"oid": "c430bd74455105f77215ece51945094ceeee6c86",
"oid": "6d5b3aa31ebb68a06ceb46bbd6cf49b6ccd6f5e6",
"oid": "0a223346b4e6dec0127b1e6aa892c4ee0424b66a",
"oid": "ec91adb718a6045b492303f00d8e8beb957dc780",
"oid": "ecc889dc94cf6da17ae6eab5bb7b7155f577519d",
"oid": "ec91adb718a6045b492303f00d8e8beb957dc780",
"mergeCommit": Object {
"oid": "0a223346b4e6dec0127b1e6aa892c4ee0424b66a",
"oid": "ec91adb718a6045b492303f00d8e8beb957dc780",
"oid": "ecc889dc94cf6da17ae6eab5bb7b7155f577519d",
"oid": "ec91adb718a6045b492303f00d8e8beb957dc780",
"mergeCommit": Object {
"oid": "6d5b3aa31ebb68a06ceb46bbd6cf49b6ccd6f5e6",
"oid": "0a223346b4e6dec0127b1e6aa892c4ee0424b66a",
"oid": "ec91adb718a6045b492303f00d8e8beb957dc780",
"oid": "ecc889dc94cf6da17ae6eab5bb7b7155f577519d",
"oid": "ec91adb718a6045b492303f00d8e8beb957dc780",
"mergeCommit": null,
```
Cross-check this against [the example-github commits][commits] thus:
- Note that commit `6bd1b4c` is the head commit, and is thus the root
commit of the `target` chain.
- Note that commits `0a22334` and `6d5b3aa`, which were merged via
pull request, appear twice each: once in the history from head, and
once as the merge commit of a pull request.
- Note that commit `0a22334` has two parents at each occurrence.
- Note that the unmerged pull request’s merge commit is `null`.
[commits]: https://github.com/sourcecred/example-github/commits/master
To run this on real-world data, apply the following patch:
```diff
diff --git a/src/plugins/github/fetchGithubRepo.js b/src/plugins/github/fetchGithubRepo.js
index 6ac201af..b14ca760 100644
--- a/src/plugins/github/fetchGithubRepo.js
+++ b/src/plugins/github/fetchGithubRepo.js
@@ -11,6 +11,7 @@ import {stringify, inlineLayout, type Body} from "../../graphql/queries";
import {createQuery, createVariables, postQueryExhaustive} from "./graphql";
import type {GithubResponseJSON} from "./graphql";
import type {RepoId} from "../../core/repoId";
+import translateContinuations from "./translateContinuations";
/**
* Scrape data from a GitHub repo using the GitHub API.
@@ -44,6 +45,11 @@ export default function fetchGithubRepo(
payload
).then((x: GithubResponseJSON) => {
ensureNoMorePages(x);
+ console.warn("Translating continuations...");
+ for (const w of translateContinuations(x).warnings) {
+ console.warn(w);
+ }
+ console.warn("Done.");
return x;
});
}
```
Then run:
```
$ yarn backend >/dev/null 2>/dev/null; echo $?
0
$ node ./bin/sourcecred.js load sourcecred/sourcecred --plugin github 2>&1 |
> ts -s '%.s'
55.015740 Translating continuations...
55.037217 { type: 'UNKNOWN_PARENT_OID',
55.037273 child: '0d38dde23a6de831315f3643a7d2bc15e8df7678',
55.037290 parent: 'cb8ba0eaa1abc1f921e7165bb19e29b40723ce65' }
55.037309 { type: 'UNKNOWN_PARENT_OID',
55.037336 child: 'd152f48ce4c2ed1d046bf6ed4f139e7e393ea660',
55.037359 parent: 'de7a8723963d9cd0437ef34f5942a071b850c0e7' }
55.037383 Done.
```
Note that the two commits in question were each merged into a non-master
branch, in #28 and #329 respectively. Note also that translating these
continuations took just 22 milliseconds.
wchargin-branch: github-translate-continuations
Summary:
This fixes the following issues:
- Pull request reviews actually do not have reactions.
- We must fetch the `id` of a `Ref`.
- We must fetch the `id` of a `Commit`, `Tree`, `Blob`, or `Tag`, and
should also fetch its `oid`.
- Repository owners cannot be bots.
- Commit and reaction authors cannot be bots, organizations, or
`undefined`.
Test Plan:
Running `yarn test --full` passes, and the snapshot diff is clearly
correct.
wchargin-branch: github-fix-up-continuations
Summary:
The included schema is forked from the one in `graphql/demo.js`.
Primitive types have been added, and the `parents` connection has been
added to commit objects per #920. (We do not include this in the demo
script because without prefetching it would take a long time to load.)
Test Plan:
Unit tests added; run `yarn unit`. Then run `yarn backend` and verify
that `node ./bin/generateGithubGraphqlFlowTypes.js` generates exactly
the same output as in the types file:
```
$ node ./bin/generateGithubGraphqlFlowTypes.js |
> diff -u - ./src/plugins/github/graphqlTypes.js
$ echo $?
0
```
Change the `graphqlTypes.js` file and verify that `yarn unit` fails.
As the build config has been changed, a `yarn test --full` is warranted.
It passes.
Finally, I have manually verified that the schema is consistent with the
documentation at <https://developer.github.com/v4/object/repository/>
and related pages.
wchargin-branch: github-schema-flow-types
Summary:
Generating Flow types from a structured schema is both straightforward
and terribly useful. This commit implements it.
Test Plan:
Inspect the snapshot for correctness manually. Then, copy it into a new
file, remove backslash-escapes, and verify that it passes Flow.
A subsequent commit will generate types for the actual GitHub schema.
wchargin-branch: graphql-generate-flow-types
Summary:
Prior to this change, primitive fields were un\[i\]typed. This commit
allows adding type annotations. Such annotations do not change the
semantics at all, but we will be able to use them to generate Flow types
corresponding to a schema.
This commit also strengthens the validation on schemata to catch some
errors that would previously have gone unnoticed at schema construction
time: e.g., a node reference to a type that does not exist.
Test Plan:
Unit tests updated, retaining full coverage. The demo script continues
to work when loading `example-github` or `sourcecred`.
wchargin-branch: schema-annotated-primitives
Summary:
To ease the transition from manual continuation resolution to the Mirror
API, we update the old system to fetch commit parent OIDs, as described
in #923.
Test Plan:
To check that the continuations are wired correctly, apply the following
patch to force continuations to be followed at every step:
```diff
diff --git a/src/plugins/github/graphql.js b/src/plugins/github/graphql.js
index 05761ca..a21a364 100644
--- a/src/plugins/github/graphql.js
+++ b/src/plugins/github/graphql.js
@@ -53,7 +53,7 @@ const PAGE_SIZE_COMMENTS = 20;
const PAGE_SIZE_REVIEWS = 5;
const PAGE_SIZE_REVIEW_COMMENTS = 10;
const PAGE_SIZE_COMMIT_HISTORY = 100;
-const PAGE_SIZE_COMMIT_PARENTS = 5;
+const PAGE_SIZE_COMMIT_PARENTS = 0;
const PAGE_SIZE_REACTIONS = 5;
/**
@@ -358,6 +358,7 @@ function* continuationsFromCommit(
) {
const b = build;
if (result.parents && result.parents.pageInfo.hasNextPage) {
+ console.warn(result.parents.pageInfo);
yield {
enclosingNodeType: "COMMIT",
enclosingNodeId: nodeId,
@@ -366,7 +367,7 @@ function* continuationsFromCommit(
b.field(
"parents",
{
- first: b.literal(PAGE_LIMIT),
+ first: b.literal(1),
after: b.literal(result.parents.pageInfo.endCursor),
},
[b.fragmentSpread("commitParents")]
```
(It is important that the initial page limit be `0` and not (say) `1`,
because the `defaultBranchRef` is likely to have just one parent; by
using `0`, we test its continuations as long as it has at least one
parent.)
Then run `./src/plugins/github/fetchGithubRepoTest.sh` and note that the
test passes and the output is
```
{ hasNextPage: true, endCursor: null }
{ hasNextPage: true, endCursor: null }
{ hasNextPage: true, endCursor: null }
{ hasNextPage: true, endCursor: null }
{ hasNextPage: true, endCursor: null }
{ hasNextPage: true, endCursor: null }
{ hasNextPage: true, endCursor: null }
{ hasNextPage: true, endCursor: 'MQ==' }
{ hasNextPage: true, endCursor: 'MQ==' }
```
Note that the test output (as updated in this commit) includes commits
with a unique parent, a commit with no parents, and a commit with two
parents.
I also ran `node ./bin/sourcecred.js load` on sourcecred/sourcecred and
ipfs/js-ipfs. Each worked, and the resulting credit attributions loaded
fine.
wchargin-branch: github-commit-parent-oids
Summary:
This has two benefits:
- The dates on commits are data that we will probably want when we add
timestamps to authorship edges to accommodate time-weighted cred.
- Once the mirror module is integrated with the GitHub plugin, we’ll
want to fetch dates on commits, because this is the only real-world
test case for a nested field that contains a primitive field (as
opposed to a node reference), so it’ll be nice to be continually
exercising that somewhat-edge case.
Date strings are in commit-local time and do not depend on the time zone
of the requester (in contrast to [cursors]). For example, on SourceCred:
```shell
$ time node ./bin/fetchAndPrintGithubRepo.js \
> sourcecred sourcecred "${GITHUB_TOKEN}" |
> jq -rc '
> .repository.defaultBranchRef.target.history.nodes[]
> .author?.date[-6:]
> ' | sort | uniq -c
1 +03:00
6 -04:00
717 -07:00
58 -08:00
```
[cursors]: <https://github.com/sourcecred/sourcecred/pull/129#issuecomment-382970474>
Test Plan:
The snapshot contains 8 instances of `oid` and 8 instances of `date`,
which is good (each of these properties appears exactly once on each
commit, and nowhere else). Running `yarn test --full` passes.
wchargin-branch: github-commit-dates
Summary:
This commit follows up on the previous two pull requests by drawing the
rest of the owl.
Resolves#915.
Test Plan:
Unit tests included.
To verify the snapshot change, open the snapshot file, and copy
everything from `query TestUpdate {` through the matching `}`, not
including the enclosing quotes. Strip all backslashes (Jest adds them).
Post the resulting query to GitHub and verify that it completes
successfully and that the result contains a commit with an `author`.
In other words, `xsel -ob | tr -d '\\' | ghquery | jq .` with [ghquery].
[ghquery]: https://gist.github.com/wchargin/630e03e66fa084b7b2297189615326d1
The demo entry point has also been updated. For an end-to-end test, you
can run the following command to see a commit with a `null` author (with
the current state of the repository) and a commit with a non-`null`
author:
```
$ node bin/mirror.js /tmp/mirror-example.db \
> Repository MDEwOlJlcG9zaXRvcnkxMjMyNTUwMDY= \
> 3600 2>/dev/null |
> jq '(.defaultBranchRef.target, .pullRequests[0].mergeCommit) | {url, author}'
{
"url": "6bd1b4c0b7",
"author": {
"date": "2018-09-12T19:48:21-07:00",
"user": null
}
}
{
"url": "0a223346b4",
"author": {
"date": "2018-02-28T00:43:47-08:00",
"user": {
"id": "MDQ6VXNlcjE0MDAwMjM=",
"__typename": "User",
"url": "https://github.com/decentralion",
"login": "decentralion"
}
}
}
```
You can also check that it is possible to fetch the whole SourceCred
repository (ID: `MDEwOlJlcG9zaXRvcnkxMjAxNDU1NzA=`).
wchargin-branch: mirror-shallow
Summary:
See #915 for context. This adds nested field data to the “useful info”
data structure added in #857.
Test Plan:
Unit tests for `_buildSchemaInfo` updated.
wchargin-branch: mirror-schemainfo-shallow
Summary:
See #915 for context. This commit changes the `schema` module only.
I had a hard time picking names that clearly distinguish the top-level
field on the object and the subfields that it contains. @decentralion
and I independently came up with “nest” and “egg”. It’s a bit colorful,
but it’s certainly easy to remember which one is which, and it doesn’t
conflict with existing notions like “parent”/“child”.
Test Plan:
Unit tests expanded slightly, retaining full coverage.
wchargin-branch: schema-shallow
Summary:
By using `<a {...this.props}>{children}</a>`, we were forwarding the
Aphrodite selectors as `styles`. This caused the static HTML for the
page to include `<a styles="[object Object]">`, which is annoying.
Test Plan:
Unit tests extended: they fail before this change and pass after it.
Also clicked a router link and an external link in the application.
wchargin-branch: link-child-styles
Summary:
This completes the minimum viable public API for the `Mirror` class. As
described on the docstring, the typical workflow for a client is:
- Invoke the constructor to acquire a `Mirror` instance.
- Invoke `registerObject` to register a root object of interest.
- Invoke `update` to update all transitive dependencies.
- Invoke `extract` to retrieve the data in structured form.
It is the third step that is added in this commit.
In this commit, we also include a command-line demo of the Mirror
module, which exercises exactly the workflow above with a hard-coded
GitHub schema. This can be used to test the module’s behavior with
real-world data. I plan to remove this entry point once we integrate
this module into SourceCred.
This commit makes progress toward #622.
Test Plan:
Unit tests included for the core functionality. The imperative shell
does not have automated tests. You can test it as follows.
First, run `yarn backend` to build `bin/mirror.js`. Then, run:
```shell
$ node bin/mirror.js /tmp/mirror-demo.db \
> Repository MDEwOlJlcG9zaXRvcnkxMjMyNTUwMDY= \
> 60
```
Here, the big base64 string is the ID for the sourcecred/example-github
repository. (This is listed in `graphql/demo.js`, alongside the ID for
the SourceCred repository itself.) The value 60 is a TTL in seconds. The
database filename is arbitrary.
This will print out a ton of output to stderr (all intermediate queries
and responses, for debugging purposes), and then the full contents of
the example repository to stdout.
Run the command again, and it should finish instantly. On my machine,
the main function runs faster than the Node startup time (50ms vs 60ms).
Then, re-run the command, changing the TTL from `60` to `1`. Note that
it sends off some queries and then prints the same repository.
It is safe to kill the program at any time, either while waiting for a
response from GitHub or while processing the results, because the mirror
module takes advantage of SQLite’s transaction-safety. Intermediate
updates will be persisted, so you’ll lose just a few seconds of
progress.
You can also of course dive into the generated database yourself to
explore the data. It’s good fun.
wchargin-branch: mirror-e2e-update
Summary:
GitHub has an undocumented node limit on the number of IDs that can be
provided to the top-level `nodes` connection. This is silly, because we
can just spread the IDs over multiple identical connections. This commit
implements the logic to do so.
Test Plan:
Create some queries that use `nodes(ids: ...)` to fetch varying numbers
of objects:
```shell
id="MDEwOlJlcG9zaXRvcnkxMjAxNDU1NzA="
nodes() {
n="$1"
ids="$(yes "$id" | head -n "$n" | jq -R . | jq -sc .)"
printf 'nodes(ids: %s) { __typename }' "$ids"
}
query() {
printf '{ '
for num; do
printf 'nodes_%s: %s ' "$num" "$(nodes "$num")"
done
printf '}'
}
```
Note that the query given by `query 101` results in an error…
```json
{
"data": null,
"errors": [
{
"message": "You may not provide more than 100 node ids; you provided 101.",
"type": "ARGUMENT_LIMIT",
"path": [
"nodes_101"
],
"locations": [
{
"line": 1,
"column": 3
}
]
}
]
}
```
…but the query given by `query 98 99` happily returns 197 node entries.
wchargin-branch: mirror-paginate-own-data
Summary:
Fixes#903. We already ignore Markdown code syntax (backticks), but
prior to this commit we treated the contents of all HTML elements,
including `<code>`, as normal text. As of this commit, `<code>` elements
are stripped entirely. Other HTML elements, like `<em>`, are unaffected.
Test Plan:
Unit tests added. Also, load data for `ipfs/js-ipfs-block-service`, and
observe in the UI that PR `#36` (Update aegir to version 9.0.0) no
longer has any outward references.
wchargin-branch: markdown-html-code
This modifies the `nodeDescription` code for the Git plugin so that when
given a Git commit, it will hyperlink to that commit on GitHub. It does
this by looking up the corresponding `RepoId`s from the newly-added
`commitToRepoId` field in the `Repository` (#884).
Per a [suggestion in review], rather than hardcoding the GitHub url
logic in the Git plugin, we provide them via a `GitGateway`.
[suggestion in review]: https://github.com/sourcecred/sourcecred/pull/887#issuecomment-424059649
When no `RepoId` is found, it errors to console and does not include a
hyperlink. When multiple `RepoId`s are available, it chooses to link to
one arbitrarily. (In the future, we could amend this behavior to add
links to every valid repo). This behavior is tested.
Test plan:
I ran the application on newly-generated data and verified that it sets
up commit hyperlinks appropriately. Also, see unit tests.
Summary:
An update step is now roughly as simple as:
_updateData(postQuery(_queryFromPlan(_findOutdated())))
(with some option/config parameters thrown in).
This makes progress toward #622.
Test Plan:
Unit tests included. They are light, because these functions are light.
They still retain full coverage.
wchargin-branch: mirror-full-pipeline
Summary:
These typenames are often superfluous, but sometimes they are useful.
For instance, we might want to fetch the same data for `User`s, `Bot`s,
and `Organization`s, but still differentiate which kind of node we
fetched from an `Actor` union reference. Similarly, many timeline events
may have similar signatures (like, “issue closed” vs. “issue reopened”).
Test Plan:
Existing unit tests have been updated; run `yarn unit`.
wchargin-branch: mirror-extract-typenames
Summary:
The `extract` method lets you get data out of a mirror in a structured
format.
The mirror module now contains all the plumbing needed to provide
meaningful value. Remaining to be implemented are some internal
porcelain and a public method to perform an update step.
This makes progress toward #622.
Test Plan:
Comprehensive unit tests included, with full coverage; run `yarn unit`.
wchargin-branch: mirror-extract
Test Plan:
Note that the nav links are now a lighter color, except when `:active`
(e.g., when you’re holding down the mouse button).
wchargin-branch: remove-visited
Summary:
This reestablishes harmony in light of #882.
Test Plan:
Existing unit tests suffice; run `yarn unit`.
wchargin-branch: mirror-query-plan-connection-object-typename
Summary:
This commit adds internal functions to (a) emit a GraphQL query to fetch
data for own-data of an object, and (b) ingest the results of said query
back into the database.
The API and implementation differ from the connection-updating analogues
introduced in #878 in that the query for own data is independent of an
object’s ID: it depends only on the object’s type. This affords us more
flexibility in composing queries.
As described in a internal documentation comment, values are stored in
the database in JSON-stringified form: we cannot use the obvious raw SQL
values, because there is no native encoding of booleans (`0`/`1` is
conventional), and we need to distinguish them from other data types.
There are other ways to solve this problem. Notably:
1. We could take inspiration from OCaml: encode stronger static types
and a simpler runtime representation. That is, we could change the
schema field types from simply “primitive” to the various specific
primitive types. Then, when reading data out from the database, we
could reinterpret the values appropriately.
2. We could take advantage of the fact that we are not using all of
SQLite’s data types. In particular, we do not store anything as a
binary blob, so we could encode `false` as a length-0 zeroblob and
`true` as a length-1 zeroblob, for instance. Again, when reading
data out from the database, we would reinterpret the values—but in
this approach we would not need an explicit schema from the user.
For now, we take the easiest and simplest approach just to get ourselves
off the ground. We can easily move to the second option described above
later.
This commit makes progress toward #622.
Test Plan:
Unit tests included, with full coverage. While these tests check that
the GraphQL queries are as expected, they cannot check that they are
actually valid in production. To check this, follow the instructions in
the added snapshot test.
wchargin-branch: mirror-own-data-updates
Summary:
This will enable us to style links consistently across our application.
Our previous link colors for base and `:visited` were so similar that I
didn’t actually realize that they were different. In this change, I’ve
kept the same base color, and selected a more contrasting `:visited`
color. I also added an `:active` color, which is good for usability
(color chosen via <http://paletton.com/>’s “triad” suggestion).
Example screenshot, with active, visited, and base links:
![Screenshot as in this commit][img-underline]
I also considered implementing the link underlines with `border-bottom`
instead of `text-decoration` (an oft-touted suggestion that has always
smelled fishy to me, but [the W3 does it][w3], so I guess it’s okay).
That would look like this:
![Screenshot with `border-bottom` underlines][img-border]
…but I did not do so ([rationale in a comment on #890][rationale]).
[w3]: https://www.w3.org/TR/WCAG20-TECHS/F73.html
[img-underline]: https://user-images.githubusercontent.com/4317806/45987381-f154f580-c025-11e8-8b0f-63c1e1ddce02.png
[img-border]: https://user-images.githubusercontent.com/4317806/45926176-a081b780-bed4-11e8-96f2-d0d24d11c8f7.png
[rationale]: https://github.com/sourcecred/sourcecred/pull/890#issuecomment-424146773
We can certainly change these decisions later—that’s one of the purposes
of having this abstraction—so I’m not inclined to bikeshed on them too
much in this commit.
Implementation adapted from:
<80263b190e/src/components/Link.js>
Test Plan:
Check that `<a>` elements and React Router link elements are used only
in `Link` and snapshots:
```
$ git grep --name-only -Fw '<a'
src/app/Link.js
src/assets/logo/discourse_512.png
src/plugins/github/__snapshots__/render.test.js.snap
$ git grep '"react-router"' | grep 'Link'
src/app/Link.js:import {Link as RouterLink} from "react-router";
src/app/Link.test.js:import {Link as RouterLink} from "react-router";
src/app/createRelativeHistory.test.js:import {Router, Route, Link} from "react-router";
src/app/withAssets.test.js:import {IndexRoute, Link, Router, Route} from "react-router";
```
Check that the primary color now appears in just one spot:
```
$ git grep -i 0872a2
src/app/Link.js: ...colorAttributes("#0872A2"),
```
Then, run `yarn start` and click all the links. Note in particular that
the SVG icons in the header have the correct colors in the active state
as well as the base state.
wchargin-branch: app-link
Summary:
As <https://github.com/sourcecred/sourcecred/pull/883/files#r219648511>.
It is somewhat unfortunate that this mixes a command with a query, but
the concession is acceptable in this instance, I think.
Test Plan:
Existing unit tests suffice, retaining full coverage.
wchargin-branch: mirror-register-node-field-results
Summary:
This helpful utility is already used in some test code, and will shortly
be used in main code. @decentralion suggested factoring it out in
<https://github.com/sourcecred/sourcecred/pull/883#discussion_r219647781>.
Test Plan:
Unit tests included, with full coverage; run `yarn unit`.
wchargin-branch: mirror-make-update-helper
This modifies the Git `Repository` data structure so that for every
commit, we track the `RepoId`s of repos containing that commit. This way
we will be able to do things like hyperlink to the right url for that
commit.
`loadRepository` has been modified to set the initial `repoId`.
`mergeRepository` has been updated to ensure that it concatenates the
`repoId`s properly.
Tests were added for both cases.
The example-git snapshot has been updated accordingly.
Test plan: `yarn test --full`
This modifies `core/repoId` so that `repoIdToString` returns not a
`string`, but an opaque subtype of `string` called `RepoIdString`.
This allows us to store stringified `RepoId`s (which is useful whenever
we want value-not-reference semantics, like for use as a map key)
while still maintaining a type assertion that the strings, in fact,
represent valid `RepoId`s.
Test plan: A unit test verifies that it's a flow error to cast a string
to a `RepoIdString`.