Add a configurable feedback URL to prototype (#715)

Summary:
We can now set, at build time, a URL to be displayed at the top of the
prototype, encouraging users to provide feedback. If the URL is not
provided, it defaults to the appropriate topic on the SourceCred
Discourse instance.

The result looks like this:

![Screenshot of the feedback URL in the prototype][screenshot]

[screenshot]: https://user-images.githubusercontent.com/4317806/44814824-a238b380-ab92-11e8-88c8-dfbae27ca496.png

Test Plan:
Unit tests added to `yarn sharness-full` and `yarn unit`.

You can run `yarn start` to see the message with the default URL, or
`SOURCECRED_FEEDBACK_URL=http://example.com/ yarn start` to specify a
custom URL.

wchargin-branch: feedback-url

CHANGELOG.md

@@ -1,6 +1,7 @@
 # Changelog
 
 ## [Unreleased]
+- Add a feedback link to the prototype (#715)
 - Support combining multiple repositories into a single graph (#711)
 - Normalize scores so that 1000 cred is split amongst users (#709)
 - Stop persisting weights in local store (#706)

config/env.js

@@ -116,6 +116,12 @@ function getGitState() /*: GitState */ {
 const SOURCECRED_GIT_STATE = stringify(getGitState());
 process.env.SOURCECRED_GIT_STATE = SOURCECRED_GIT_STATE;
 
+const SOURCECRED_FEEDBACK_URL =
+  process.env.SOURCECRED_FEEDBACK_URL != null
+    ? process.env.SOURCECRED_FEEDBACK_URL
+    : "https://discuss.sourcecred.io/c/cred-feedback/";
+process.env.SOURCECRED_FEEDBACK_URL = SOURCECRED_FEEDBACK_URL;
+
 function getClientEnvironment() {
   const raw = {};
   // Useful for determining whether we’re running in production mode.
@@ -123,6 +129,8 @@ function getClientEnvironment() {
   raw.NODE_ENV = process.env.NODE_ENV || "development";
   // Used by `src/app/version.js`.
   raw.SOURCECRED_GIT_STATE = SOURCECRED_GIT_STATE;
+  // Used by `src/app/credExplorer/App.js`.
+  raw.SOURCECRED_FEEDBACK_URL = SOURCECRED_FEEDBACK_URL;
 
   // Stringify all values so we can feed into Webpack's DefinePlugin.
   const stringified = {"process.env": {}};

scripts/build_static_site.sh

@@ -34,6 +34,7 @@ main() {
 }
 
 parse_args() {
+    unset SOURCECRED_FEEDBACK_URL
     target=
     cname=
     repos=( )
@@ -52,6 +53,17 @@ parse_args() {
                 if [ $# -eq 0 ]; then die 'missing value for --repo'; fi
                 repos+=( "$1" )
                 ;;
+           --feedback-url)
+                shift
+                if [ $# -eq 0 ]; then die 'missing value for --feedback-url'; fi
+                if [ -n "${SOURCECRED_FEEDBACK_URL:-}" ]; then
+                    die '--feedback-url specified multiple times'
+                fi
+                export SOURCECRED_FEEDBACK_URL="$1"
+                if [ -z "${SOURCECRED_FEEDBACK_URL}" ]; then
+                    die 'empty value for --feedback-url'
+                fi
+                ;;
             --cname)
                 shift
                 if [ $# -eq 0 ]; then die 'missing value for --cname'; fi

sharness/test_build_static_site.t

@@ -78,6 +78,25 @@ test_expect_success "should fail with missing repo value" '
     printf "redacted\n" | test_cmp - important_dir/.wallet.dat
 '
 
+test_expect_success "should fail with missing feedback-url value" '
+    test_must_fail run --target putative_output --feedback-url 2>err &&
+    grep -qF -- "missing value for --feedback-url" err &&
+    printf "redacted\n" | test_cmp - important_dir/.wallet.dat
+'
+
+test_expect_success "should fail with empty feedback-url" '
+    test_must_fail run --target putative_output --feedback-url "" 2>err &&
+    grep -qF -- "empty value for --feedback-url" err &&
+    printf "redacted\n" | test_cmp - important_dir/.wallet.dat
+'
+
+test_expect_success "should fail with multiple feedback-url values" '
+    test_must_fail run --target putative_output \
+        --feedback-url a.com --feedback-url b.com 2>err &&
+    grep -qF -- "--feedback-url specified multiple times" err &&
+    printf "redacted\n" | test_cmp - important_dir/.wallet.dat
+'
+
 test_expect_success "should fail with missing cname value" '
     test_must_fail run --target putative_output --cname 2>err &&
     grep -qF -- "missing value for --cname" err &&
@@ -140,6 +159,13 @@ run_build() {
         "${prereq_name}: should have no cache" '
         test_must_fail test_path_is_dir "${api_dir}/cache"
     '
+    test_expect_success "${prereq_name}" \
+        "${prereq_name}: should have a bundle" '
+        _js_bundles=( "${output_dir}"/static/js/main.*.js ) &&
+        [ ${#_js_bundles[@]} -eq 1 ] &&
+        js_bundle_path="${_js_bundles[0]}" &&
+        test_path_is_file "${js_bundle_path}"
+    '
 }
 
 # test_pages PREREQ_NAME
@@ -175,6 +201,7 @@ test_pages() {
 run_build TWO_REPOS \
     "should build the site with two repositories and a CNAME" \
     --cname sourcecred.example.com \
+    --feedback-url http://discuss.example.com/feedback/ \
     --repo sourcecred/example-git \
     --repo sourcecred/example-github \
     ;
@@ -198,12 +225,20 @@ test_expect_success TWO_REPOS \
     done
 '
 
+test_expect_success TWO_REPOS \
+    "TWO_REPOS: should include the feedback URL somewhere in the bundle" '
+    grep -qF http://discuss.example.com/feedback/ "${js_bundle_path}"
+'
+
 test_expect_success TWO_REPOS "TWO_REPOS: should have a correct CNAME record" '
     test_path_is_file "${output_dir}/CNAME" &&
     printf "sourcecred.example.com" | test_cmp - "${output_dir}/CNAME"
 '
 
-run_build NO_REPOS \
+# This feedback URL is "pollution" in the source environment and should
+# _not_ be passed down to the actual application.
+SOURCECRED_FEEDBACK_URL=http://wat.com/wat \
+    run_build NO_REPOS \
     "should build the site with no repositories and no CNAME" \
     # no arguments here
 
@@ -224,6 +259,11 @@ test_expect_success NO_REPOS \
     done
 '
 
+test_expect_success NO_REPOS \
+    "NO_REPOS: should not include a feedback URL from a polluted environment" '
+    test_must_fail grep -qF http://wat.com/wat "${js_bundle_path}"
+'
+
 test_expect_success NO_REPOS "NO_REPOS: should have no CNAME record" '
     test_must_fail test -e "${output_dir}/CNAME"
 '

src/app/credExplorer/App.js

@@ -82,6 +82,14 @@ export function createApp(
       }
       return (
         <div style={{maxWidth: 900, margin: "0 auto", padding: "0 10px"}}>
+          <p>
+            <em>
+              This is a prototype. Please{" "}
+              <a href={process.env.SOURCECRED_FEEDBACK_URL}>
+                let us know what you think
+              </a>.
+            </em>
+          </p>
           <div style={{marginBottom: 10}}>
             <RepositorySelect
               assets={this.props.assets}

src/app/credExplorer/App.test.js

@@ -114,6 +114,15 @@ describe("app/credExplorer/App", () => {
     expect(el.instance().props.localStore).toBe(localStore);
   });
 
+  it("should have a feedback link with a valid URL", () => {
+    const {el} = example();
+    const link = el
+      .find("a")
+      .filterWhere((x) => x.text().includes("let us know what you think"));
+    expect(link).toHaveLength(1);
+    expect(link.prop("href")).toMatch(/https?:\/\//);
+  });
+
   describe("when in state:", () => {
     function testRepositorySelect(stateFn) {
       it("creates a working RepositorySelect", () => {