Merge pull request #17 from akrantz/manifest-command-fixes

office-addin-manifest command fixes

Author: akrantz

packages/office-addin-manifest/package.json

@@ -32,6 +32,8 @@
     "@types/mocha": "^5.2.5",
     "@types/node": "^10.5.7",
     "@types/node-fetch": "^2.1.2",
+    "@types/uuid": "^3.4.4",
+    "@types/validator": "^9.4.2",
     "@types/xml2js": "^0.4.3",
     "concurrently": "^3.6.1",
     "copy-dir": "^0.4.0",

packages/office-addin-manifest/src/commands.ts

@@ -4,17 +4,28 @@
 import * as commnder from "commander";
 import * as manifestInfo from "./manifestInfo";
 
-export async function info(path: string) {
+function getCommandOptionString(option: string | boolean, defaultValue?: string): string | undefined {
+  // For a command option defined with an optional value, e.g. "--option [value]",
+  // when the option is provided with a value, it will be of type "string", return the specified value;
+  // when the option is provided without a value, it will be of type "boolean", return undefined.
+  return (typeof(option) === "boolean") ? defaultValue : option;
+}
+
+export async function info(manifestPath: string) {
   try {
-    const manifest = await manifestInfo.readManifestFile(path);
-    logManifestInfo(path, manifest);
+    const manifest = await manifestInfo.readManifestFile(manifestPath);
+    logManifestInfo(manifestPath, manifest);
   } catch (err) {
-    console.error(`Error: ${err}`);
+    logErrorMessage(err);
   }
 }
 
-function logManifestInfo(path: string, manifest: manifestInfo.ManifestInfo) {
-  console.log(`Manifest: ${path}`);
+function logErrorMessage(err: any) {
+  console.error(`Error: ${err instanceof Error ? err.message : err}`);
+}
+
+function logManifestInfo(manifestPath: string, manifest: manifestInfo.ManifestInfo) {
+  console.log(`Manifest: ${manifestPath}`);
   console.log(`  Id: ${manifest.id || ""}`);
   console.log(`  Name: ${manifest.displayName || ""}`);
   console.log(`  Provider: ${manifest.providerName || ""}`);
@@ -24,18 +35,15 @@ function logManifestInfo(path: string, manifest: manifestInfo.ManifestInfo) {
   console.log(`  Description: ${manifest.description || ""}`);
 }
 
-export async function modify(path: string, command: commnder.Command) {
+export async function modify(manifestPath: string, command: commnder.Command) {
   try {
-    const guid: string | undefined = (command.guid) ? command.guid : undefined;
-    const displayName: string | undefined = (command.displayName) ? command.displayName : undefined;
-
-    if (guid === undefined && displayName === undefined) {
-      throw new Error("You need to specify something to change in the manifest.");
-    }
+    // if the --guid command option is provided without a value, use "" to specify to change to a random guid value.
+    const guid: string | undefined = getCommandOptionString(command.guid, "");
+    const displayName: string | undefined = getCommandOptionString(command.displayName);
 
-    const manifest = await manifestInfo.modifyManifestFile(path, guid, displayName);
-    logManifestInfo(path, manifest);
+    const manifest = await manifestInfo.modifyManifestFile(manifestPath, guid, displayName);
+    logManifestInfo(manifestPath, manifest);
   } catch (err) {
-    console.error(`Error: ${err}`);
+    logErrorMessage(err);
   }
-}
+}

packages/office-addin-manifest/src/manifest.ts

@@ -8,13 +8,13 @@ import * as commands from "./commands";
 
 if (process.argv[1].endsWith("\\manifest.js")) {
   commander
-    .command("info <path>")
+    .command("info <manifest-path>")
     .action(commands.info);
 
   commander
-    .command("modify <path>")
-    .option("-g,--guid [guid]", "Change the guid. Specify \"random\" for a random guid.")
-    .option("-d, --displayName [name]", "Display name for the add-in.")
+    .command("modify <manifest-path>")
+    .option("-g,--guid [guid]", "Change the guid. A random guid is used if one is not provided.")
+    .option("-d, --displayName <name>", "Change the display name.")
     .action(commands.modify);
 
   commander.parse(process.argv);

packages/office-addin-manifest/src/manifestInfo.ts

@@ -1,9 +1,9 @@
 import * as fs from "fs";
 import * as util from "util";
+import * as uuid from "uuid";
 import * as xml2js from "xml2js";
 import * as xmlMethods from "./xml";
 const readFileAsync = util.promisify(fs.readFile);
-const uuid = require('uuid/v1');
 const writeFileAsync = util.promisify(fs.writeFile);
 type Xml = any;
 
@@ -35,7 +35,7 @@ function parseManifest(xml: Xml): ManifestInfo {
 export async function modifyManifestFile(manifestPath: string, guid?: string, displayName?: string): Promise<ManifestInfo> {
   let manifestData: ManifestInfo = {};
   if (manifestPath) {
-    if (!guid && !displayName) {
+    if (guid === undefined && displayName === undefined) {
       throw new Error("You need to specify something to change in the manifest.");
     } else {
       manifestData = await modifyManifestXml(manifestPath, guid, displayName);
@@ -85,15 +85,15 @@ async function readXmlFromManifestFile(manifestPath: string): Promise<Xml> {
   return xml;
 }
 
-function setModifiedXmlData(xml: any, guid: string | undefined, displayName: string | undefined) {
-  if (guid) {
-    if (guid === "random") {
+function setModifiedXmlData(xml: any, guid: string | undefined, displayName: string | undefined): void {
+  if (typeof(guid) !== "undefined") {
+    if (!guid || guid === "random") {
       guid = uuid();
     }
     xmlMethods.setXmlElementValue(xml, "Id", guid);
   }
 
-  if (displayName) {
+  if (typeof(displayName) !== "undefined") {
     xmlMethods.setXmlElementAttributeValue(xml, "DisplayName", displayName);
   }
 }

packages/office-addin-manifest/test/test.ts

@@ -1,9 +1,9 @@
 import * as assert from "assert";
 import * as fs from "fs";
 import * as mocha from "mocha";
+import * as uuid from "uuid";
+import { isUUID } from "validator";
 import * as manifestInfo from "../src/manifestInfo";
-const uuid = require('uuid');
-const validator = require("validator");
 const manifestOriginalFolder = process.cwd() + "/test/manifests";
 const manifestTestFolder = process.cwd() + "\\testExecution\\testManifests";
 const testManifest = manifestTestFolder + "\\manifest.xml";
@@ -70,9 +70,9 @@ describe("Manifest", function() {
       const updatedInfo = await manifestInfo.modifyManifestFile(testManifest, "random", undefined);
 
       // verify guid updated, that it"s a valid guid and that the displayName is not updated
-      assert.notStrictEqual(originalInfo.id, updatedInfo.id);
-      assert.equal(true, validator.isUUID(updatedInfo.id));
-      assert.strictEqual(originalInfo.displayName, updatedInfo.displayName);
+      assert.notStrictEqual(updatedInfo.id, originalInfo.id);
+      assert.strictEqual(updatedInfo.id && isUUID(updatedInfo.id), true);
+      assert.strictEqual(updatedInfo.displayName, originalInfo.displayName);
     });
     it("should handle specifying displayName only", async function() {
       // get original manifest info and create copy of manifest that we can overwrite in this test
@@ -83,18 +83,18 @@ describe("Manifest", function() {
       const updatedInfo = await manifestInfo.modifyManifestFile(testManifest, undefined, testDisplayName);
 
       // verify displayName updated and guid not updated
-      assert.notStrictEqual(originalInfo.displayName, updatedInfo.displayName);
+      assert.notStrictEqual(updatedInfo.displayName, originalInfo.displayName);
       assert.strictEqual(updatedInfo.displayName, testDisplayName);
-      assert.strictEqual(originalInfo.id, updatedInfo.id);
+      assert.strictEqual( updatedInfo.id, originalInfo.id);
     });
     it("should handle not specifying either a guid or displayName", async function() {
       let result;
       try {
-        await manifestInfo.modifyManifestFile(testManifest, undefined, undefined);
+        await manifestInfo.modifyManifestFile(testManifest);
       } catch (err) {
-        result = err;
+        result = err.message;
       }
-      assert.equal(result, "Error: You need to specify something to change in the manifest.");
+      assert.strictEqual(result, `You need to specify something to change in the manifest.`);
     });
     it("should handle an invalid manifest file path", async function() {
       // call  modify, specifying an invalid manifest path with a valid guid and displayName
@@ -108,7 +108,7 @@ describe("Manifest", function() {
         result = err.message;
       }
 
-      assert.equal(result, `Unable to modify xml data for manifest file: ${invalidManifest}. \nError: ENOENT: no such file or directory, open '${invalidManifest}'`);
+      assert.strictEqual(result, `Unable to modify xml data for manifest file: ${invalidManifest}. \nError: ENOENT: no such file or directory, open '${invalidManifest}'`);
     });
   });
 });