fix(sync): Better error message on Workspace Add and Commit

packages/engine-server/src/workspace/utils.ts

@@ -32,6 +32,10 @@ import fs from "fs-extra";
 import _ from "lodash";
 import path from "path";
 import { URI } from "vscode-uri";
+import {
+  SyncActionResult,
+  SyncActionStatus,
+} from "./workspaceServiceInterface";
 
 export class WorkspaceUtils {
   static isWorkspaceConfig(val: any): val is WorkspaceSettings {
@@ -317,4 +321,40 @@ export class WorkspaceUtils {
     }
     return link;
   }
+
+  /**
+   * @param results
+   * @returns number of repos that has Sync Action Status done.
+   */
+  static getCountForStatusDone(results: SyncActionResult[]): number {
+    return this.count(results, SyncActionStatus.DONE);
+  }
+
+  static count(results: SyncActionResult[], status: SyncActionStatus) {
+    return results.filter((result) => result.status === status).length;
+  }
+
+  /**
+   *
+   * @param results
+   * @param status
+   * @returns name of all the repos with status same as @param status.
+   */
+  static getFilteredRepoNames(
+    results: SyncActionResult[],
+    status: SyncActionStatus
+  ): string[] {
+    const matchingResults = results.filter(
+      (result) => result.status === status
+    );
+    if (matchingResults.length === 0) return [];
+    return matchingResults.map((result) => {
+      // Display the vault names for info/error messages
+      if (result.vaults.length === 1) {
+        return VaultUtils.getName(result.vaults[0]);
+      }
+      // But if there's more than one vault in the repo, then use the repo path which is easier to interpret
+      return result.repo;
+    });
+  }
 }

packages/plugin-core/src/commands/AddAndCommit.ts

@@ -1,38 +1,112 @@
+import { DendronError, ERROR_SEVERITY } from "@dendronhq/common-all";
+import {
+  SyncActionResult,
+  SyncActionStatus,
+  WorkspaceUtils,
+} from "@dendronhq/engine-server";
 import _ from "lodash";
-import path from "path";
-import { window } from "vscode";
+import { ProgressLocation, window } from "vscode";
 import { DENDRON_COMMANDS } from "../constants";
 import { ExtensionProvider } from "../ExtensionProvider";
 import { Logger } from "../logger";
+import { MessageSeverity, VSCodeUtils } from "../vsCodeUtils";
 import { BasicCommand } from "./base";
 
 const L = Logger;
 
 type CommandOpts = {};
+type CommandReturns =
+  | {
+      finalMessage: string;
+      committed: SyncActionResult[];
+    }
+  | undefined;
 
-export class AddAndCommit extends BasicCommand<CommandOpts, void> {
+export class AddAndCommit extends BasicCommand<CommandOpts, CommandReturns> {
   key = DENDRON_COMMANDS.ADD_AND_COMMIT.key;
 
+  private static generateReportMessage({
+    committed,
+  }: {
+    committed: SyncActionResult[];
+  }) {
+    const message = ["Finished Commit."];
+    // Report anything unusual the user probably should know about
+    let maxMessageSeverity: MessageSeverity = MessageSeverity.INFO;
+
+    const makeMessage = (
+      status: SyncActionStatus,
+      results: SyncActionResult[][],
+      fn: (repos: string) => {
+        msg: string;
+        severity: MessageSeverity;
+      }
+    ) => {
+      const uniqResults = _.uniq(_.flattenDeep(results));
+      const repos = WorkspaceUtils.getFilteredRepoNames(uniqResults, status);
+      if (repos.length === 0) return;
+      const { msg, severity } = fn(repos.join(", "));
+      message.push(msg);
+      if (severity > maxMessageSeverity) maxMessageSeverity = severity;
+    };
+
+    // Warnings, need user interaction to continue commit
+    makeMessage(SyncActionStatus.MERGE_CONFLICT, [committed], (repos) => {
+      return {
+        msg: `Skipped ${repos} because they have merge conflicts that must be resolved manually.`,
+        severity: MessageSeverity.WARN,
+      };
+    });
+
+    makeMessage(SyncActionStatus.NO_CHANGES, [committed], (repos) => {
+      return {
+        msg: `Skipped ${repos} because it has no new changes.`,
+        severity: MessageSeverity.INFO,
+      };
+    });
+
+    makeMessage(SyncActionStatus.REBASE_IN_PROGRESS, [committed], (repos) => {
+      return {
+        msg: `Skipped ${repos} because there's a rebase in progress that must be resolved.`,
+        severity: MessageSeverity.WARN,
+      };
+    });
+    return { message, maxMessageSeverity };
+  }
+
   async execute(opts?: CommandOpts) {
     const ctx = "execute";
     L.info({ ctx, opts });
     const engine = ExtensionProvider.getEngine();
     const workspaceService = ExtensionProvider.getExtension().workspaceService;
-    const resp = await workspaceService!.commitAndAddAll({
-      engine,
-    });
-    if (_.isEmpty(resp)) {
-      window.showInformationMessage(`no files to add or commit`);
-      return;
-    }
-    const respString = _.map(resp, (ent: string) => {
-      return path.basename(ent);
-    })
-      .filter((ent) => !_.isUndefined(ent))
-      .join(", ");
-    window.showInformationMessage(
-      `add and commit files in the following vaults: ${respString}`
+    if (_.isUndefined(workspaceService))
+      throw new DendronError({
+        message: "Workspace is not initialized",
+        severity: ERROR_SEVERITY.FATAL,
+      });
+    const committed = await window.withProgress(
+      {
+        location: ProgressLocation.Notification,
+        title: "Workspace Add and Commit",
+        cancellable: false,
+      },
+      async (progress) => {
+        progress.report({ message: "staging changes" });
+        const committed = await workspaceService.commitAndAddAll({
+          engine,
+        });
+        L.info(committed);
+        return committed;
+      }
     );
-    return;
+    const { message, maxMessageSeverity } = AddAndCommit.generateReportMessage({
+      committed,
+    });
+    const committedDone = WorkspaceUtils.getCountForStatusDone(committed);
+    const repos = (count: number) => (count <= 1 ? "repo" : "repos");
+    message.push(`Committed ${committedDone} ${repos(committedDone)}`);
+    const finalMessage = message.join(" ");
+    VSCodeUtils.showMessage(maxMessageSeverity, finalMessage, {});
+    return { committed, finalMessage };
   }
 }

packages/plugin-core/src/test/suite-integ/AddAndCommitCommand.test.ts

@@ -0,0 +1,215 @@
+import { ConfigUtils, NoteUtils } from "@dendronhq/common-all";
+import { tmpDir } from "@dendronhq/common-server";
+import {
+  Git,
+  SyncActionStatus,
+  WorkspaceUtils,
+} from "@dendronhq/engine-server";
+import { GitTestUtils } from "@dendronhq/engine-test-utils";
+import _ from "lodash";
+import { expect } from "../testUtilsv2";
+import { describeSingleWS } from "../testUtilsV3";
+import fs from "fs-extra";
+import { ExtensionProvider } from "../../ExtensionProvider";
+import { AddAndCommit } from "../../commands/AddAndCommit";
+import { NoteTestUtilsV4 } from "@dendronhq/common-test-utils";
+
+const TIMEOUT = 60 * 1000 * 5;
+
+suite("GIVEN Workspace Add And Commit command is run", function () {
+  this.timeout(TIMEOUT);
+  describeSingleWS(
+    "WHEN there are no changes",
+    {
+      modConfigCb: (config) => {
+        ConfigUtils.setWorkspaceProp(config, "workspaceVaultSyncMode", "sync");
+        return config;
+      },
+    },
+    () => {
+      test("THEN Dendron skips committing", async () => {
+        const { wsRoot } = ExtensionProvider.getDWorkspace();
+        const remoteDir = tmpDir().name;
+        await GitTestUtils.createRepoForRemoteWorkspace(wsRoot, remoteDir);
+        // Add everything and push, so that there's no changes
+        const git = new Git({ localUrl: wsRoot });
+        await git.addAll();
+        await git.commit({ msg: "add all and commit" });
+        const out = await new AddAndCommit().execute();
+        const { committed, finalMessage } = out;
+        expect(WorkspaceUtils.getCountForStatusDone(committed)).toEqual(0);
+        expect(committed[0].status).toEqual(SyncActionStatus.NO_CHANGES);
+        expect(finalMessage).toEqual(
+          "Finished Commit. Skipped vault because it has no new changes. Committed 0 repo"
+        );
+      });
+    }
+  );
+
+  describeSingleWS(
+    "WHEN there is a merge conflict",
+    {
+      modConfigCb: (config) => {
+        ConfigUtils.setWorkspaceProp(config, "workspaceVaultSyncMode", "sync");
+        return config;
+      },
+    },
+    () => {
+      test("THEN Dendron skips committing files", async () => {
+        const { vaults, wsRoot, engine } = ExtensionProvider.getDWorkspace();
+        const remoteDir = tmpDir().name;
+        await GitTestUtils.createRepoForRemoteWorkspace(wsRoot, remoteDir);
+        const rootNote = NoteUtils.getNoteByFnameFromEngine({
+          fname: "root",
+          vault: vaults[0],
+          engine,
+        })!;
+        // Add everything and push, so that there's no untracked changes
+        const git = new Git({ localUrl: wsRoot, remoteUrl: remoteDir });
+        await git.addAll();
+        await git.commit({ msg: "first commit" });
+        await git.push();
+        // Update root note and add a commit that's not in remote, so there'll be something to rebase
+        const fpath = NoteUtils.getFullPath({
+          note: rootNote,
+          wsRoot,
+        });
+        await fs.appendFile(fpath, "Deserunt culpa in expedita\n");
+        await git.addAll();
+        await git.commit({ msg: "second commit" });
+
+        // Clone to a second location, then push a change through that
+        const secondaryDir = tmpDir().name;
+        const secondaryGit = new Git({
+          localUrl: secondaryDir,
+          remoteUrl: remoteDir,
+        });
+        await secondaryGit.clone(".");
+        const secondaryFpath = NoteUtils.getFullPath({
+          note: rootNote,
+          wsRoot: secondaryDir,
+        });
+        await fs.appendFile(secondaryFpath, "Aut ut nisi dolores quae et\n");
+        await secondaryGit.addAll();
+        await secondaryGit.commit({ msg: "secondary" });
+        await secondaryGit.push();
+
+        // Cause an ongoing rebase
+        try {
+          await git.pull();
+        } catch {
+          // deliberately ignored
+        }
+
+        const out = await new AddAndCommit().execute();
+        const { committed, finalMessage } = out;
+        expect(WorkspaceUtils.getCountForStatusDone(committed)).toEqual(0);
+        expect(committed[0].status).toEqual(SyncActionStatus.MERGE_CONFLICT);
+        expect(finalMessage).toEqual(
+          "Finished Commit. Skipped vault because they have merge conflicts that must be resolved manually. Committed 0 repo"
+        );
+      });
+    }
+  );
+
+  describeSingleWS(
+    "WHEN there is a rebase in progress",
+    {
+      modConfigCb: (config) => {
+        ConfigUtils.setWorkspaceProp(config, "workspaceVaultSyncMode", "sync");
+        return config;
+      },
+    },
+    () => {
+      test("THEN Dendron skips committing files", async () => {
+        const { vaults, wsRoot, engine } = ExtensionProvider.getDWorkspace();
+        const remoteDir = tmpDir().name;
+        await GitTestUtils.createRepoForRemoteWorkspace(wsRoot, remoteDir);
+        const rootNote = NoteUtils.getNoteByFnameFromEngine({
+          fname: "root",
+          vault: vaults[0],
+          engine,
+        })!;
+        // Add everything and push, so that there's no untracked changes
+        const git = new Git({ localUrl: wsRoot, remoteUrl: remoteDir });
+        await git.addAll();
+        await git.commit({ msg: "first commit" });
+        await git.push();
+        // Update root note and add a commit that's not in remote, so there'll be something to rebase
+        const fpath = NoteUtils.getFullPath({
+          note: rootNote,
+          wsRoot,
+        });
+        await fs.appendFile(fpath, "Deserunt culpa in expedita\n");
+        await git.addAll();
+        await git.commit({ msg: "second commit" });
+
+        // Clone to a second location, then push a change through that
+        const secondaryDir = tmpDir().name;
+        const secondaryGit = new Git({
+          localUrl: secondaryDir,
+          remoteUrl: remoteDir,
+        });
+        await secondaryGit.clone(".");
+        const secondaryFpath = NoteUtils.getFullPath({
+          note: rootNote,
+          wsRoot: secondaryDir,
+        });
+        await fs.appendFile(secondaryFpath, "Aut ut nisi dolores quae et\n");
+        await secondaryGit.addAll();
+        await secondaryGit.commit({ msg: "secondary" });
+        await secondaryGit.push();
+
+        // Cause an ongoing rebase
+        try {
+          await git.pull();
+        } catch {
+          // deliberately ignored
+        }
+        // Mark the conflict as resolved
+        await git.add(fpath);
+
+        const out = await new AddAndCommit().execute();
+        const { committed, finalMessage } = out;
+        // Should skip everything since there's an ongoing rebase the user needs to resolve
+        expect(WorkspaceUtils.getCountForStatusDone(committed)).toEqual(0);
+        expect(committed[0].status).toEqual(
+          SyncActionStatus.REBASE_IN_PROGRESS
+        );
+        expect(finalMessage).toEqual(
+          "Finished Commit. Skipped vault because there's a rebase in progress that must be resolved. Committed 0 repo"
+        );
+      });
+    }
+  );
+
+  describeSingleWS(
+    "WHEN there are unstaged changes",
+    {
+      modConfigCb: (config) => {
+        ConfigUtils.setWorkspaceProp(config, "workspaceVaultSyncMode", "sync");
+        return config;
+      },
+    },
+    () => {
+      test("THEN Dendron commit files successfully", async () => {
+        const { vaults, wsRoot } = ExtensionProvider.getDWorkspace();
+        const remoteDir = tmpDir().name;
+        await GitTestUtils.createRepoForRemoteWorkspace(wsRoot, remoteDir);
+        // Create a new note so there are some changes
+        await NoteTestUtilsV4.createNote({
+          fname: "my-new-note",
+          body: "Lorem ipsum",
+          wsRoot,
+          vault: vaults[0],
+        });
+
+        const out = await new AddAndCommit().execute();
+        const { committed, finalMessage } = out;
+        // Should try to commit since there are changes
+        expect(WorkspaceUtils.getCountForStatusDone(committed)).toEqual(1);
+        expect(finalMessage).toEqual("Finished Commit. Committed 1 repo");
+      });
+    }
+  );
+});