Post-review changes

5 files changed, 110 insertions, 44 deletions

diff --git a/mod/mod-ci-github-gq.cxx b/mod/mod-ci-github-gq.cxx
index c37c889..d65d701 100644
--- a/mod/mod-ci-github-gq.cxx
+++ b/mod/mod-ci-github-gq.cxx
@@ -321,12 +321,15 @@ namespace brep
   // because GitHub does not allow a check run status of completed without a
   // conclusion.
   //
+  // `du` can be empty for queued but not for the other states.
+  //
   static string
   gq_mutation_create_check_runs (const string& ri, // Repository ID
                                  const string& hs, // Head SHA
+                                 const string& du, // Details URL.
                                  const vector<check_run>& crs,
                                  const string& st, // Check run status.
-                                 const optional<string>& co = nullopt)
+                                 optional<gq_built_result> br = nullopt)
   {
     ostringstream os;
 
@@ -339,22 +342,31 @@ namespace brep
       string al ("cr" + to_string (i)); // Field alias.
 
       os << gq_name (al) << ":createCheckRun(input: {"              << '\n'
-         << "  name: "         << gq_str (crs[i].name) << ','       << '\n'
-         << "  repositoryId: " << gq_str (ri) << ','                << '\n'
-         << "  headSha: "      << gq_str (hs) << ','                << '\n'
+         << "  name: "         << gq_str (crs[i].name)              << '\n'
+         << "  repositoryId: " << gq_str (ri)                       << '\n'
+         << "  headSha: "      << gq_str (hs)                       << '\n'
          << "  status: "       << gq_enum (st);
-      if (co)
+      if (!du.empty ())
+      {
+        os                                                          << '\n';
+        os << "  detailsUrl: " << gq_str (du);
+      }
+      if (br)
       {
-        os << ','                                                   << '\n'
-           << "  conclusion: " << gq_enum (*co);
+        os                                                          << '\n';
+        os << "  conclusion: " << gq_enum (br->conclusion)          << '\n'
+           << "  output: {"                                         << '\n'
+           << "    title: "    << gq_str (br->title)                << '\n'
+           << "    summary: "  << gq_str (br->summary)              << '\n'
+           << "  }";
       }
       os << "})"                                                    << '\n'
         // Specify the selection set (fields to be returned).
         //
          << "{"                                                     << '\n'
          << "  checkRun {"                                          << '\n'
-         << "    id,"                                               << '\n'
-         << "    name,"                                             << '\n'
+         << "    id"                                                << '\n'
+         << "    name"                                              << '\n'
          << "    status"                                            << '\n'
          << "  }"                                                   << '\n'
          << "}"                                                     << '\n';
@@ -374,28 +386,38 @@ namespace brep
   static string
   gq_mutation_update_check_run (const string& ri, // Repository ID.
                                 const string& ni, // Node ID.
+                                const string& du, // Details URL.
                                 const string& st, // Check run status.
-                                const optional<string>& co)
+                                optional<gq_built_result> br)
   {
     ostringstream os;
 
     os << "mutation {"                                            << '\n'
        << "cr0:updateCheckRun(input: {"                           << '\n'
-       << "  checkRunId: "   << gq_str (ni) << ','                << '\n'
-       << "  repositoryId: " << gq_str (ri) << ','                << '\n'
+       << "  checkRunId: "   << gq_str (ni)                       << '\n'
+       << "  repositoryId: " << gq_str (ri)                       << '\n'
        << "  status: "       << gq_enum (st);
-    if (co)
+    if (!du.empty ())
     {
-      os << ','                                                   << '\n'
-         << "  conclusion: " << gq_enum (*co);
+      os                                                          << '\n';
+      os << "  detailsUrl: " << gq_str (du);
+    }
+    if (br)
+    {
+      os                                                          << '\n';
+      os << "  conclusion: " << gq_enum (br->conclusion)          << '\n'
+         << "  output: {"                                         << '\n'
+         << "    title: "    << gq_str (br->title)                << '\n'
+         << "    summary: "  << gq_str (br->summary)              << '\n'
+         << "  }";
     }
     os << "})"                                                    << '\n'
       // Specify the selection set (fields to be returned).
       //
        << "{"                                                     << '\n'
        << "  checkRun {"                                          << '\n'
-       << "    id,"                                               << '\n'
-       << "    name,"                                             << '\n'
+       << "    id"                                                << '\n'
+       << "    name"                                              << '\n'
        << "    status"                                            << '\n'
        << "  }"                                                   << '\n'
        << "}"                                                     << '\n'
@@ -416,8 +438,10 @@ namespace brep
     //
     assert (st != build_state::built);
 
+    // Empty details URL because it's not available until building.
+    //
     string rq (gq_serialize_request (
-        gq_mutation_create_check_runs (rid, hs, crs, gh_to_status (st))));
+        gq_mutation_create_check_runs (rid, hs, "", crs, gh_to_status (st))));
 
     return gq_mutate_check_runs (error, crs, iat, move (rq), st);
   }
@@ -428,22 +452,27 @@ namespace brep
                        const string& iat,
                        const string& rid,
                        const string& hs,
+                       const string& du,
                        build_state st,
-                       optional<result_status> rs,
-                       bool ws)
+                       optional<gq_built_result> br)
   {
     // Must have a result if state is built.
     //
-    assert (st != build_state::built || rs);
+    assert (st != build_state::built || br);
 
-    vector<check_run> crs {move (cr)};
+    // Must have a details URL because `st` should never be queued.
+    //
+    assert (!du.empty ());
 
-    optional<string> co; // Conclusion.
-    if (rs)
-      co = gh_to_conclusion (*rs, ws);
+    vector<check_run> crs {move (cr)};
 
     string rq (gq_serialize_request (
-        gq_mutation_create_check_runs (rid, hs, crs, gh_to_status (st), co)));
+        gq_mutation_create_check_runs (rid,
+                                       hs,
+                                       du,
+                                       crs,
+                                       gh_to_status (st),
+                                       move (br))));
 
     bool r (gq_mutate_check_runs (error, crs, iat, move (rq), st));
 
@@ -458,20 +487,20 @@ namespace brep
                        const string& iat,
                        const string& rid,
                        const string& nid,
+                       const string& du,
                        build_state st,
-                       optional<result_status> rs,
-                       bool ws)
+                       optional<gq_built_result> br)
   {
     // Must have a result if state is built.
     //
-    assert (st != build_state::built || rs);
-
-    optional<string> co; // Conclusion.
-    if (rs)
-      co = gh_to_conclusion (*rs, ws);
-
-    string rq (gq_serialize_request (
-        gq_mutation_update_check_run (rid, nid, gh_to_status (st), co)));
+    assert (st != build_state::built || br);
+
+    string rq (
+        gq_serialize_request (gq_mutation_update_check_run (rid,
+                                                            nid,
+                                                            du,
+                                                            gh_to_status (st),
+                                                            move (br))));
 
     vector<check_run> crs {move (cr)};
 
diff --git a/mod/mod-ci-github-gq.hxx b/mod/mod-ci-github-gq.hxx
index e8bb397..5ad77e1 100644
--- a/mod/mod-ci-github-gq.hxx
+++ b/mod/mod-ci-github-gq.hxx
@@ -26,6 +26,10 @@ namespace brep
   // Note: no details_url yet since there will be no entry in the build result
   // search page until the task starts building.
   //
+  // @@ TMP We only create multiple check runs in build_queued() so
+  //        build_state is redundant. Maybe we should rename this
+  //        gq_queue_check_runs()?
+  //
   bool
   gq_create_check_runs (const basic_mark& error,
                         vector<check_run>& check_runs,
@@ -38,14 +42,15 @@ namespace brep
   // state and the node ID. Return false and issue diagnostics if the request
   // failed.
   //
-  // The result_status is required if the build_state is built because GitHub
-  // does not allow a check run status of `completed` without a conclusion. @@
+  // The gq_built_result is required if the build_state is built because
+  // GitHub does not allow a check run status of `completed` without at least
+  // a conclusion.
   //
   struct gq_built_result
   {
     string conclusion;
     string title;
-    string summmary;
+    string summary;
   };
 
   bool
@@ -64,8 +69,9 @@ namespace brep
   // with the new state. Return false and issue diagnostics if the request
   // failed.
   //
-  // The result_status is required if the build_state is built because GitHub
-  // does not allow a check run status of `completed` without a conclusion. @@
+  // The gq_built_result is required if the build_state is built because
+  // GitHub does not allow a check run status of `completed` without at least
+  // a conclusion.
   //
   bool
   gq_update_check_run (const basic_mark& error,
diff --git a/mod/mod-ci-github.cxx b/mod/mod-ci-github.cxx
index 29097be..52431d2 100644
--- a/mod/mod-ci-github.cxx
+++ b/mod/mod-ci-github.cxx
@@ -5,6 +5,8 @@
 
 #include <libbutl/json/parser.hxx>
 
+#include <web/server/mime-url-encoding.hxx> // mime_url_encode()
+
 #include <mod/jwt.hxx>
 #include <mod/hmac.hxx>
 #include <mod/module-options.hxx>
@@ -711,6 +713,7 @@ namespace brep
                                iat->token,
                                sd.repository_id,
                                *cr->node_id,
+                               details_url (b),
                                build_state::building))
       {
         // Do nothing further if the state was already built on GitHub (note
@@ -851,6 +854,12 @@ namespace brep
       //    instead of passing it to gq_*() functions? Let's see how we handle
       //    the report.
 
+      // @@ TODO Summary
+      //
+      gq_built_result br (gh_to_conclusion (*b.status, sd.warning_success),
+                          ucase (to_string (*b.status)),
+                          "SUMMARY");
+
       if (cr.node_id)
       {
         // Update existing check run to built.
@@ -860,8 +869,9 @@ namespace brep
                                  iat->token,
                                  sd.repository_id,
                                  *cr.node_id,
+                                 "", // Don't update details_url again.
                                  build_state::built,
-                                 *b.status, sd.warning_success))
+                                 move (br)))
         {
           assert (cr.state == build_state::built);
 
@@ -883,8 +893,9 @@ namespace brep
                                  iat->token,
                                  sd.repository_id,
                                  sd.head_sha,
+                                 details_url (b),
                                  build_state::built,
-                                 *b.status, sd.warning_success))
+                                 move (br)))
         {
           assert (cr.state == build_state::built);
 
@@ -938,6 +949,20 @@ namespace brep
     };
   }
 
+  string ci_github::
+  details_url (const build& b) const
+  {
+    return options_->host ()                                          +
+      "/@" + b.tenant                                                 +
+      "?builds=" + mime_url_encode (b.package_name.string ())         +
+      "&pv=" + b.package_version.string ()                            +
+      "&tg=" + mime_url_encode (b.target.string ())                   +
+      "&tc=" + mime_url_encode (b.target_config_name)                 +
+      "&pc=" + mime_url_encode (b.package_config_name)                +
+      "&th=" + mime_url_encode (b.toolchain_version.string ())        +
+      "&rs=*";
+  }
+
   optional<string> ci_github::
   generate_jwt (const basic_mark& trace,
                 const basic_mark& error) const
diff --git a/mod/mod-ci-github.hxx b/mod/mod-ci-github.hxx
index b16085e..6bb01e3 100644
--- a/mod/mod-ci-github.hxx
+++ b/mod/mod-ci-github.hxx
@@ -66,6 +66,11 @@ namespace brep
     bool
     handle_check_suite_request (gh_check_suite_event, bool warning_success);
 
+    // Build a check run details_url for a build.
+    //
+    string
+    details_url (const build&) const;
+
     optional<string>
     generate_jwt (const basic_mark& trace, const basic_mark& error) const;
 
diff --git a/mod/module.cli b/mod/module.cli
index bec4c2d..e8ebbb0 100644
--- a/mod/module.cli
+++ b/mod/module.cli
@@ -818,7 +818,8 @@ namespace brep
     // @@ TODO Is etc/brep-module.conf updated manually? Yes, will need to
     //    replicate there eventually.
     //
-    class ci_github: ci_start, ci_cancel,
+    class ci_github: repository_url,
+                     ci_start, ci_cancel,
                      build, build_db,
                      handler,
                      openssl_options