Radish alpha
r
Radicle web interface
Radicle
Git (anonymous pull)
Log in to clone via SSH
Use structured routing for patch views
Thomas Scholtes committed 2 years ago
commit aae50a2fda2469b479d57dce9063ac3bde95fcd0
parent fdd074fd13feaf3ed7c805f4ceb4372fcf91eda1
5 files changed +368 -202
modified src/views/projects/Cob/Revision.svelte
@@ -215,7 +215,11 @@
              project: projectId,
              seed: baseUrl,
              patch: patchId,
-
              search: `diff=${previousRevOid}..${revisionOid}`,
+
              view: {
+
                name: "diff",
+
                fromCommit: previousRevOid,
+
                toCommit: revisionOid,
+
              },
            }}>
            <Icon name="diff" />
          </Link>
@@ -237,7 +241,11 @@
                    project: projectId,
                    seed: baseUrl,
                    patch: patchId,
-
                    search: `diff=${item}..${revisionOid}`,
+
                    view: {
+
                      name: "diff",
+
                      fromCommit: item,
+
                      toCommit: revisionOid,
+
                    },
                  }}>
                  {#if item === projectHead}
                    <DropdownItem selected={false} size="small">
modified src/views/projects/Patch.svelte
@@ -30,6 +30,8 @@
<script lang="ts">
  import type { BaseUrl, Patch } from "@httpd-client";
  import type { Variant } from "@app/components/Badge.svelte";
+
  import { type Route } from "@app/lib/router";
+
  import type { PatchView } from "./router";

  import * as utils from "@app/lib/utils";
  import { capitalize } from "lodash";
@@ -43,7 +45,6 @@
  import CommitTeaser from "@app/views/projects/Commit/CommitTeaser.svelte";
  import Dropdown from "@app/components/Dropdown.svelte";
  import DropdownItem from "@app/components/Dropdown/DropdownItem.svelte";
-
  import ErrorMessage from "@app/components/ErrorMessage.svelte";
  import Floating, { closeFocused } from "@app/components/Floating.svelte";
  import Icon from "@app/components/Icon.svelte";
  import Link from "@app/components/Link.svelte";
@@ -53,18 +54,12 @@
  import SquareButton from "@app/components/SquareButton.svelte";
  import TagInput from "@app/views/projects/Cob/TagInput.svelte";

-
  export let search: string | undefined = undefined;
-
  export let projectId: string;
  export let baseUrl: BaseUrl;
  export let patch: Patch;
+
  export let projectId: string;
  export let projectHead: string;
  export let projectDefaultBranch: string;
-
  export let revision: string | undefined = undefined;
-

-
  $: searchParams = new URLSearchParams(search || "");
-
  $: currentTab =
-
    (searchParams.get("tab") as "activity" | "commits" | "files") || "activity";
-
  $: diff = searchParams.get("diff") || undefined;
+
  export let view: PatchView;

  const api = new HttpdClient(baseUrl);

@@ -77,7 +72,7 @@
        patch.id,
        {
          type: "thread",
-
          revision: currentRevision.id,
+
          revision: revisionId,
          action: {
            type: "comment",
            body: reply.body,
@@ -109,9 +104,16 @@
      if (add.length === 0 && remove.length === 0) {
        return;
      }
+

+
      let revision;
+
      if (view.view.name === "diff") {
+
        revision = patch.revisions[patch.revisions.length - 1].id;
+
      } else {
+
        revision = view.view.revision;
+
      }
      await api.project.updatePatch(
        projectId,
-
        currentRevision.id,
+
        revision,
        { type: "tag", add, remove },
        $httpdStore.session.id,
      );
@@ -123,11 +125,34 @@
    $httpdStore.state === "authenticated" && utils.isLocal(baseUrl.hostname)
      ? "edit"
      : "view";
-
  const options = ["activity", "commits", "files"].map(o => ({
-
    value: o,
-
    title: capitalize(o),
-
    disabled: false,
-
  }));
+

+
  let tabs: Record<string, Route>;
+
  $: {
+
    const baseRoute = {
+
      resource: "project.patch",
+
      project: projectId,
+
      seed: baseUrl,
+
      patch: patch.id,
+
    } as const;
+
    // For cleaner URLs, we omit the the revision part when we link to the
+
    // latest revision.
+
    const latestRevisionId = patch.revisions[patch.revisions.length - 1].id;
+
    const revision = latestRevisionId === revisionId ? undefined : revisionId;
+
    tabs = {
+
      activity: {
+
        ...baseRoute,
+
        view: { name: "activity" },
+
      },
+
      commits: {
+
        ...baseRoute,
+
        view: { name: "commits", revision },
+
      },
+
      files: {
+
        ...baseRoute,
+
        view: { name: "files", revision },
+
      },
+
    };
+
  }

  function computeReviews(patch: Patch) {
    const patchReviews: Record<string, { latest: boolean; review: Review }> =
@@ -143,10 +168,14 @@
    return patchReviews;
  }

+
  let revisionId: string;
+
  $: if (view.view.name === "diff") {
+
    revisionId = patch.revisions[patch.revisions.length - 1].id;
+
  } else {
+
    revisionId = view.view.revision;
+
  }
+

  $: patchReviews = computeReviews(patch);
-
  $: currentRevision =
-
    (revision && patch.revisions.find(r => r.id === revision)) ||
-
    patch.revisions[patch.revisions.length - 1];
  $: timelineTuple = patch.revisions.map<
    [
      {
@@ -307,7 +336,7 @@
            rawPath={utils.getRawBasePath(
              projectId,
              baseUrl,
-
              currentRevision.oid,
+
              patch.revisions[0].id,
            )} />
        {:else}
          <span class="txt-missing">No description available</span>
@@ -323,59 +352,44 @@

    <div class="tab-line">
      <div style="display: flex; gap: 0.5rem;">
-
        {#each options as option}
-
          {#if !option.disabled}
-
            <Link
-
              route={{
-
                resource: "project.patch",
-
                project: projectId,
-
                seed: baseUrl,
-
                patch: patch.id,
-
                revision,
-
                search: `tab=${option.value}`,
-
              }}>
-
              <SquareButton
-
                size="small"
-
                clickable={option.disabled}
-
                active={option.value === currentTab && !diff}
-
                disabled={option.disabled}>
-
                {option.title}
-
              </SquareButton>
-
            </Link>
-
          {:else}
-
            <SquareButton
-
              size="small"
-
              clickable={option.disabled}
-
              active={option.value === currentTab}
-
              disabled={option.disabled}>
-
              {option.title}
+
        {#each Object.entries(tabs) as [name, route]}
+
          <Link {route}>
+
            <SquareButton size="small" active={name === view.view.name}>
+
              {capitalize(name)}
            </SquareButton>
-
          {/if}
+
          </Link>
        {/each}
-
        {#if diff}
+
        {#if view.view.name === "diff"}
          <Link
            route={{
              resource: "project.patch",
              project: projectId,
              seed: baseUrl,
              patch: patch.id,
-
              search: `diff=${diff}`,
+
              view: {
+
                name: "diff",
+
                fromCommit: view.view.fromCommit,
+
                toCommit: view.view.toCommit,
+
              },
            }}>
            <SquareButton size="small" active={true}>
-
              Diff {diff.substr(0, 6)}..{diff.split("..")[1].substr(0, 6)}
+
              Diff {view.view.fromCommit.substring(
+
                0,
+
                6,
+
              )}..{view.view.toCommit.substring(0, 6)}
            </SquareButton>
          </Link>
        {/if}
      </div>

-
      {#if currentTab !== "activity"}
+
      {#if view.view.name === "commits" || view.view.name === "files"}
        <Floating disabled={patch.revisions.length === 1}>
          <svelte:fragment slot="toggle">
            <SquareButton
              size="small"
              clickable={patch.revisions.length > 1}
              disabled={patch.revisions.length === 1}>
-
              Revision {utils.formatObjectId(currentRevision.id)}
+
              Revision {utils.formatObjectId(view.view.revision)}
            </SquareButton>
          </svelte:fragment>
          <svelte:fragment slot="modal">
@@ -388,11 +402,13 @@
                    project: projectId,
                    seed: baseUrl,
                    patch: patch.id,
-
                    revision: item.id,
-
                    search: `tab=${currentTab}`,
+
                    view: {
+
                      name: view.view.name,
+
                      revision: item.id,
+
                    },
                  }}>
                  <DropdownItem
-
                    selected={item.id === currentRevision.id}
+
                    selected={item.id === view.view.revision}
                    size="tiny">
                    Revision {utils.formatObjectId(item.id)}
                  </DropdownItem>
@@ -403,66 +419,56 @@
        </Floating>
      {/if}
    </div>
-
    {#if currentTab === "activity"}
-
      {#if diff}
-
        {@const [base, revision] = diff.split("..")}
-
        {#await api.project.getDiff(projectId, base, revision) then { diff }}
-
          <div style:margin-top="1rem">
-
            <Changeset {projectId} {baseUrl} {revision} {diff} />
-
          </div>
-
        {:catch e}
-
          <ErrorMessage
-
            message="Not able to load revision diff."
-
            stackTrace={e} />
-
        {/await}
+
    {#if view.view.name === "diff"}
+
      <div style:margin-top="1rem">
+
        <Changeset
+
          {projectId}
+
          {baseUrl}
+
          revision={view.view.toCommit}
+
          diff={view.view.diff} />
+
      </div>
+
    {:else if view.view.name === "activity"}
+
      {#each timelineTuple as [revision, timelines], index}
+
        {@const previousRevision =
+
          index > 0 ? patch.revisions[index - 1] : undefined}
+
        <RevisionComponent
+
          {baseUrl}
+
          {projectId}
+
          {timelines}
+
          {projectDefaultBranch}
+
          {projectHead}
+
          {...revision}
+
          first={index === 0}
+
          on:reply={createReply}
+
          patchId={patch.id}
+
          expanded={index === patch.revisions.length - 1}
+
          previousRevId={previousRevision?.id}
+
          previousRevOid={previousRevision?.oid} />
      {:else}
-
        {#each timelineTuple as [revision, timelines], index}
-
          {@const previousRevision =
-
            index > 0 ? patch.revisions[index - 1] : undefined}
-
          <RevisionComponent
-
            {baseUrl}
-
            {projectId}
-
            {timelines}
-
            {projectDefaultBranch}
-
            {projectHead}
-
            {...revision}
-
            first={index === 0}
-
            on:reply={createReply}
-
            patchId={patch.id}
-
            expanded={index === patch.revisions.length - 1}
-
            previousRevId={previousRevision?.id}
-
            previousRevOid={previousRevision?.oid} />
-
        {:else}
-
          <Placeholder emoji="🍂">
-
            <div slot="title">No activity</div>
-
            <div slot="body">No activity on this patch yet</div>
-
          </Placeholder>
+
        <Placeholder emoji="🍂">
+
          <div slot="title">No activity</div>
+
          <div slot="body">No activity on this patch yet</div>
+
        </Placeholder>
+
      {/each}
+
    {:else if view.view.name === "commits"}
+
      <div class="commit-list">
+
        {#each view.view.commits as commit}
+
          <CommitTeaser {projectId} {baseUrl} {commit} />
        {/each}
-
      {/if}
-
    {:else if currentTab === "commits"}
-
      {#await api.project.getDiff(projectId, currentRevision.base, currentRevision.oid) then diff}
-
        <div class="commit-list">
-
          {#each diff.commits as commit}
-
            <CommitTeaser {projectId} {baseUrl} {commit} />
-
          {/each}
-
        </div>
-
      {:catch e}
-
        <ErrorMessage message="Not able to load commits." stackTrace={e} />
-
      {/await}
-
    {:else if currentTab === "files"}
-
      {#await api.project.getDiff(projectId, currentRevision.base, currentRevision.oid) then diff}
-
        <div style:margin-top="1rem">
-
          <Changeset
-
            {projectId}
-
            {baseUrl}
-
            revision={currentRevision.oid}
-
            diff={diff.diff} />
-
        </div>
-
      {:catch e}
-
        <ErrorMessage message="Not able to load files diff." stackTrace={e} />
-
      {/await}
+
      </div>
+
    {:else if view.view.name === "files"}
+
      <div style:margin-top="1rem">
+
        <Changeset
+
          {projectId}
+
          {baseUrl}
+
          revision={view.view.revision}
+
          diff={view.view.diff} />
+
      </div>
+
    {:else}
+
      {utils.unreachable(view.view.name)}
    {/if}
  </div>
+

  <div class="metadata">
    <div>
      <div class="metadata-section-header">Reviews</div>
modified src/views/projects/View.svelte
@@ -126,8 +126,7 @@
      projectId={id}
      projectDefaultBranch={project.defaultBranch}
      projectHead={project.head}
-
      revision={view.revision}
-
      search={view.search} />
+
      {view} />
  {:else}
    {unreachable(view)}
  {/if}
modified src/views/projects/router.ts
@@ -4,6 +4,7 @@ import type {
  Blob,
  Commit,
  CommitHeader,
+
  Diff,
  Issue,
  Patch,
  Project,
@@ -40,20 +41,8 @@ export type ProjectRoute =
      project: string;
      issue: string;
    }
-
  | {
-
      resource: "project.patches";
-
      seed: BaseUrl;
-
      project: string;
-
      search?: string;
-
    }
-
  | {
-
      resource: "project.patch";
-
      seed: BaseUrl;
-
      project: string;
-
      patch: string;
-
      revision?: string;
-
      search?: string;
-
    };
+
  | ProjectPatchesRoute
+
  | ProjectPatchRoute;

interface ProjectTreeRoute {
  resource: "project.tree";
@@ -73,6 +62,33 @@ interface ProjectHistoryRoute {
  revision?: string;
}

+
interface ProjectPatchRoute {
+
  resource: "project.patch";
+
  seed: BaseUrl;
+
  project: string;
+
  patch: string;
+
  view?:
+
    | {
+
        name: "activity";
+
      }
+
    | {
+
        name: "commits" | "files";
+
        revision?: string;
+
      }
+
    | {
+
        name: "diff";
+
        fromCommit: string;
+
        toCommit: string;
+
      };
+
}
+

+
interface ProjectPatchesRoute {
+
  resource: "project.patches";
+
  seed: BaseUrl;
+
  project: string;
+
  search?: string;
+
}
+

export interface ProjectLoadedRoute {
  resource: "projects";
  params: ProjectLoadedParams;
@@ -119,17 +135,34 @@ export type ProjectLoadedView =
  | { resource: "issues"; search: string }
  | { resource: "newIssue" }
  | { resource: "patches"; search: string }
-
  | {
-
      resource: "patch";
-
      patch: Patch;
-
      revision?: string;
-
      search: string;
-
    };
+
  | PatchView;

export type BlobResult =
  | { ok: true; blob: Blob; highlighted: Syntax.Root | undefined }
  | { ok: false; error: { message: string; path: string } };

+
export interface PatchView {
+
  resource: "patch";
+
  patch: Patch;
+
  view:
+
    | {
+
        name: "activity";
+
        revision: string;
+
      }
+
    | {
+
        name: "commits" | "files";
+
        revision: string;
+
        diff: Diff;
+
        commits: CommitHeader[];
+
      }
+
    | {
+
        name: "diff";
+
        diff: Diff;
+
        fromCommit: string;
+
        toCommit: string;
+
      };
+
}
+

// Check whether the input is a SHA1 commit.
function isOid(input: string): boolean {
  return /^[a-fA-F0-9]{40}$/.test(input);
@@ -232,40 +265,7 @@ export async function loadProjectRoute(
        };
      }
    } else if (route.resource === "project.patch") {
-
      try {
-
        const projectPromise = api.project.getById(route.project);
-
        const patchPromise = api.project.getPatchById(
-
          route.project,
-
          route.patch,
-
        );
-
        const [project, patch] = await Promise.all([
-
          projectPromise,
-
          patchPromise,
-
        ]);
-
        return {
-
          resource: "projects",
-
          params: {
-
            id: route.project,
-
            baseUrl: route.seed,
-
            project,
-
            view: {
-
              resource: "patch",
-
              patch,
-
              revision: route.revision,
-
              search: route.search || "",
-
            },
-
          },
-
        };
-
      } catch (error: any) {
-
        return {
-
          resource: "loadError",
-
          params: {
-
            title: route.patch,
-
            errorMessage: "Not able to load this patch.",
-
            stackTrace: error.stack,
-
          },
-
        };
-
      }
+
      return loadPatchView(route);
    } else if (route.resource === "project.issues") {
      const project = await api.project.getById(route.project);
      return {
@@ -473,6 +473,73 @@ async function loadHistoryView(
  };
}

+
async function loadPatchView(
+
  route: ProjectPatchRoute,
+
): Promise<ProjectLoadedRoute> {
+
  const api = new HttpdClient(route.seed);
+
  const [project, patch] = await Promise.all([
+
    api.project.getById(route.project),
+
    api.project.getPatchById(route.project, route.patch),
+
  ]);
+
  const latestRevision = patch.revisions[patch.revisions.length - 1];
+

+
  let view: PatchView["view"];
+
  switch (route.view?.name) {
+
    case "activity":
+
    case undefined: {
+
      view = { name: "activity", revision: latestRevision.id };
+
      break;
+
    }
+
    case "commits":
+
    case "files": {
+
      const revisionId = route.view.revision;
+
      const revision =
+
        patch.revisions.find(r => r.id === revisionId) || latestRevision;
+
      if (!revision) {
+
        throw new Error(
+
          `revision ${revisionId} of patch ${route.patch} not found`,
+
        );
+
      }
+
      const { diff, commits } = await api.project.getDiff(
+
        route.project,
+
        revision.base,
+
        revision.oid,
+
      );
+
      view = {
+
        name: route.view?.name,
+
        revision: revision.id,
+
        diff,
+
        commits,
+
      };
+
      break;
+
    }
+
    case "diff": {
+
      const { fromCommit, toCommit } = route.view;
+
      const { diff } = await api.project.getDiff(
+
        route.project,
+
        fromCommit,
+
        toCommit,
+
      );
+

+
      view = { name: "diff", fromCommit, toCommit, diff };
+
      break;
+
    }
+
  }
+
  return {
+
    resource: "projects",
+
    params: {
+
      id: route.project,
+
      baseUrl: route.seed,
+
      project,
+
      view: {
+
        resource: "patch",
+
        patch,
+
        view,
+
      },
+
    },
+
  };
+
}
+

async function getPeerBranches(
  api: HttpdClient,
  project: string,
@@ -580,27 +647,60 @@ export function resolveProjectRoute(
      };
    }
  } else if (content === "patches") {
-
    const patch = segments.shift();
-
    const revision = segments.shift();
-
    if (patch) {
+
    return resolvePatchesRoute(seed, project, segments, urlSearch);
+
  } else {
+
    return null;
+
  }
+
}
+

+
function resolvePatchesRoute(
+
  seed: BaseUrl,
+
  project: string,
+
  segments: string[],
+
  urlSearch: string,
+
): ProjectPatchRoute | ProjectPatchesRoute {
+
  const patch = segments.shift();
+
  const revision = segments.shift();
+
  if (patch) {
+
    const searchParams = new URLSearchParams(sanitizeQueryString(urlSearch));
+
    const tab = searchParams.get("tab");
+
    const base = {
+
      resource: "project.patch",
+
      seed,
+
      project,
+
      patch,
+
    } as const;
+
    const diff = searchParams.get("diff");
+
    if (diff) {
+
      const [fromCommit, toCommit] = diff.split("..");
+
      if (isOid(fromCommit) && isOid(toCommit)) {
+
        return {
+
          ...base,
+
          view: { name: "diff", fromCommit, toCommit },
+
        };
+
      }
+
    }
+

+
    if (tab === "commits" || tab === "files") {
      return {
-
        resource: "project.patch",
-
        seed,
-
        project,
-
        patch,
-
        revision,
-
        search: sanitizeQueryString(urlSearch),
+
        ...base,
+
        view: { name: tab, revision },
      };
-
    } else {
+
    } else if (tab === "activity") {
      return {
-
        resource: "project.patches",
-
        seed,
-
        project,
-
        search: sanitizeQueryString(urlSearch),
+
        ...base,
+
        view: { name: tab },
      };
+
    } else {
+
      return base;
    }
  } else {
-
    return null;
+
    return {
+
      resource: "project.patches",
+
      seed,
+
      project,
+
      search: sanitizeQueryString(urlSearch),
+
    };
  }
}

@@ -666,18 +766,40 @@ export function projectRouteToPath(route: ProjectRoute): string {
    }
    return url;
  } else if (route.resource === "project.patch") {
-
    pathSegments.push("patches", route.patch);
-
    if (route.revision) {
-
      pathSegments.push(route.revision);
-
    }
+
    return patchRouteToPath(route);
+
  } else {
+
    return unreachable(route);
+
  }
+
}

-
    let url = pathSegments.join("/");
-
    if (route.search) {
-
      url += `?${route.search}`;
+
function patchRouteToPath(route: ProjectPatchRoute): string {
+
  const seed = seedPath(route.seed);
+

+
  const pathSegments = [seed, route.project];
+

+
  pathSegments.push("patches", route.patch);
+
  if (route.view?.name === "commits" || route.view?.name === "files") {
+
    if (route.view.revision) {
+
      pathSegments.push(route.view.revision);
    }
+
  }
+

+
  let url = pathSegments.join("/");
+
  if (!route.view) {
    return url;
  } else {
-
    return unreachable(route);
+
    const searchParams = new URLSearchParams();
+

+
    if (route.view.name === "diff") {
+
      searchParams.set(
+
        "diff",
+
        `${route.view.fromCommit}..${route.view.toCommit}`,
+
      );
+
    } else {
+
      searchParams.set("tab", route.view.name);
+
    }
+
    url += `?${searchParams.toString()}`;
+
    return url;
  }
}

modified tests/unit/router.test.ts
@@ -158,45 +158,76 @@ describe("route invariant when parsed", () => {
    });
  });

-
  test("projects.patch", () => {
+
  test("projects.patch default view", () => {
    expectParsingInvariant({
      resource: "project.patch",
      seed,
      project: "PROJECT",
      patch: "PATCH",
-
      search: "",
    });
  });

-
  test("projects.patch with revision", () => {
+
  test("projects.patch activity", () => {
    expectParsingInvariant({
      resource: "project.patch",
      seed,
      project: "PROJECT",
      patch: "PATCH",
-
      search: "",
-
      revision: "REVISION",
+
      view: { name: "activity" },
    });
  });

-
  test("projects.patch with search", () => {
+
  test("projects.patch commits", () => {
    expectParsingInvariant({
      resource: "project.patch",
      seed,
      project: "PROJECT",
      patch: "PATCH",
-
      search: "SEARCH",
+
      view: { name: "commits" },
    });
  });

-
  test("projects.patch with revision and search", () => {
+
  test("projects.patch commits with revision", () => {
    expectParsingInvariant({
      resource: "project.patch",
      seed,
      project: "PROJECT",
      patch: "PATCH",
-
      revision: "REVISION",
-
      search: "SEARCH",
+
      view: { name: "commits", revision: "REVISION" },
+
    });
+
  });
+

+
  test("projects.patch files", () => {
+
    expectParsingInvariant({
+
      resource: "project.patch",
+
      seed,
+
      project: "PROJECT",
+
      patch: "PATCH",
+
      view: { name: "files" },
+
    });
+
  });
+

+
  test("projects.patch files with revision", () => {
+
    expectParsingInvariant({
+
      resource: "project.patch",
+
      seed,
+
      project: "PROJECT",
+
      patch: "PATCH",
+
      view: { name: "files", revision: "REVISION" },
+
    });
+
  });
+

+
  test("projects.patch diff", () => {
+
    expectParsingInvariant({
+
      resource: "project.patch",
+
      seed,
+
      project: "PROJECT",
+
      patch: "PATCH",
+
      view: {
+
        name: "diff",
+
        fromCommit: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
+
        toCommit: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb",
+
      },
    });
  });