Merge pull request #31 from 1Password/builder-remove-set-vault

Remove 'setVault' from ItemBuilder
1Password · Aug 11, 2021 · 859064b · 859064b
2 parents dcd41bb + 4e84dea
commit 859064b
Show file tree

Hide file tree

Showing 5 changed files with 16 additions and 61 deletions.
diff --git a/README.md b/README.md
@@ -21,12 +21,6 @@ yarn add @1password/connect
 
 ## Usage
 
-### Environment Variables
-
-| Variable   | Description                                                                                            |
-| :--------- | ------------------------------------------------------------------------------------------------------ |
-| `OP_VAULT` | `ItemBuilder` will default to this vault UUID if `.setVault()` is not called when defining a new Item. |
-
 #### Creating an API client
 
 ```typescript
@@ -57,7 +51,6 @@ const myVaultId = {vault_uuid};
 
 // Create an Item
 const newItem = new ItemBuilder()
-	.setVault(myVaultId)
     .setCategory("LOGIN")
 	.addField({
 		label: "Example",

diff --git a/__test__/itembuilder.test.ts b/__test__/itembuilder.test.ts
@@ -19,10 +19,11 @@ describe("Test ItemBuilder", () => {
 
     test("Create Item with minimum required fields", () => {
         const newItem = new ItemBuilder()
-            .setVault(VAULT_ID)
             .setCategory(CategoryEnum.Login)
             .build();
-        expect(newItem.vault).toEqual({id: VAULT_ID});
+
+        // Vault should not be defined
+        expect(newItem.vault).toBeUndefined();
 
         // Item ID is set by server when not defined locally
         expect(newItem.id).toBeUndefined();
@@ -35,13 +36,11 @@ describe("Test ItemBuilder", () => {
         const builder = new ItemBuilder();
 
         const firstItem = builder
-            .setVault(VAULT_ID)
             .setCategory(CategoryEnum.Custom)
             .setTitle("first")
             .build();
 
         const secondItem = builder
-            .setVault("abc123")
             .setCategory(CategoryEnum.Custom)
             .setTitle("second")
             .addTag("second")
@@ -53,7 +52,6 @@ describe("Test ItemBuilder", () => {
 
     test("multiple url.primary assignments", () => {
         const newItem = new ItemBuilder()
-            .setVault(VAULT_ID)
             .setCategory(CategoryEnum.Login)
             .addUrl({href: "1password.com", primary: true})
             .addUrl({href: "agilebits.com", primary: true})
@@ -82,23 +80,20 @@ describe("Test ItemBuilder", () => {
 
         // Never called => undefined
         const itemNotFavorite = new ItemBuilder()
-            .setVault(VAULT_ID)
             .setCategory(CategoryEnum.Custom)
             .build();
 
         expect(itemNotFavorite.favorite).toBeUndefined();
 
         // Toggle favorite => True
         const item = new ItemBuilder()
-            .setVault(VAULT_ID)
             .setCategory(CategoryEnum.Custom)
             .toggleFavorite()
             .build();
         expect(item.favorite).toEqual(true);
 
         // User called toggleFavorite twice, expect "True" to be toggled back to "False"
         const itemFavoriteCalledTwice = new ItemBuilder()
-            .setVault(VAULT_ID)
             .setCategory(CategoryEnum.Custom)
             .toggleFavorite()
             .toggleFavorite()
@@ -109,8 +104,7 @@ describe("Test ItemBuilder", () => {
 
     test("set item category", () => {
 
-        const builder = new ItemBuilder()
-            .setVault(VAULT_ID);
+        const builder = new ItemBuilder();
 
         // Invalid category -> ERROR
         expect(() => {
@@ -127,7 +121,6 @@ describe("Test ItemBuilder", () => {
 
         // Case in-sensitive section names
         const itemOneSection = new ItemBuilder()
-            .setVault(VAULT_ID)
             .setCategory(CategoryEnum.Login)
             .addSection("Section 1")
             .addSection("section 1")
@@ -137,7 +130,6 @@ describe("Test ItemBuilder", () => {
         expect(itemOneSection.sections[0].label).toEqual("Section 1");
 
         const itemUtf8Sections = new ItemBuilder()
-            .setVault(VAULT_ID)
             .setCategory(CategoryEnum.Login)
             .addSection("🔐 Secure!")
             .addSection("🔐 Secure")
@@ -148,7 +140,6 @@ describe("Test ItemBuilder", () => {
         expect(itemUtf8Sections.sections[0].label).toEqual("🔐 Secure!");
 
         const itemMultipleSections = new ItemBuilder()
-            .setVault(VAULT_ID)
             .setCategory(CategoryEnum.Login)
             .addSection("It's a secret!")
             .addSection("Geheimnis")
@@ -164,8 +155,7 @@ describe("Test ItemBuilder", () => {
         const caseInsensitiveTags = ["myTag", "mytag", "MYTAG"];
 
         const itemWithTagsBuilder = new ItemBuilder()
-            .setCategory(CategoryEnum.Login)
-            .setVault(VAULT_ID);
+            .setCategory(CategoryEnum.Login);
 
         caseInsensitiveTags.forEach((tag) => {
             itemWithTagsBuilder.addTag(tag);
@@ -180,7 +170,6 @@ describe("Test ItemBuilder", () => {
     test("adding fields: defaults", () => {
         const item = new ItemBuilder()
             .setCategory(CategoryEnum.Login)
-            .setVault(VAULT_ID)
             .addField({value: "MySecret"})
             .build();
 
@@ -199,8 +188,7 @@ describe("Test ItemBuilder", () => {
         const fieldSectionName = "Test Section";
 
         const item = new ItemBuilder()
-            .setCategory(CategoryEnum.Login)
-            .setVault(VAULT_ID)
+            .setCategory(CategoryEnum.Login) 
             .addField({value: "MySecret", sectionName: fieldSectionName})
             .build();
 
@@ -221,7 +209,6 @@ describe("Test ItemBuilder", () => {
 
         const item = new ItemBuilder()
             .setCategory(CategoryEnum.Login)
-            .setVault(VAULT_ID)
             .addSection(fieldSectionName)
             .addField({value: "MySecret", sectionName: fieldSectionName})
             .build();
@@ -240,7 +227,6 @@ describe("Test ItemBuilder", () => {
     test("adding fields: generate value with invalid recipe", () => {
 
         const builder = new ItemBuilder()
-            .setVault(VAULT_ID)
             .setCategory(CategoryEnum.Login);
 
         // If `generate = false` then recipe evaluation is skipped
@@ -261,8 +247,7 @@ describe("Test ItemBuilder", () => {
 
         // When `generate` = true, expect recipe validation
         const builderWithRecipeValidation = new ItemBuilder()
-            .setCategory(CategoryEnum.Login)
-            .setVault(VAULT_ID);
+            .setCategory(CategoryEnum.Login);
 
         expect(() => {
             builderWithRecipeValidation.addField( {
@@ -276,8 +261,7 @@ describe("Test ItemBuilder", () => {
 
     test("adding fields: generate = true, recipe is valid", () => {
         const builder = new ItemBuilder()
-            .setCategory(CategoryEnum.Login)
-            .setVault(VAULT_ID);
+            .setCategory(CategoryEnum.Login);
 
         expect(() => {
             builder.addField( {
@@ -309,22 +293,8 @@ describe("Use ENV Vars as default values", () => {
         process.env = ENV_BACKUP;
     });
 
-    test("Error thrown when no OP_VAULT and vault.id is undefined", () => {
-       const builder = new ItemBuilder().setCategory(CategoryEnum.Login);
-       expect(() => {builder.build(); }).toThrowError();
-    });
-
     test("Error thrown when Item Category undefined", () => {
-       const builder = new ItemBuilder().setVault("EXAMPLE");
+       const builder = new ItemBuilder();
        expect(() => {builder.build(); }).toThrowError();
     });
-
-    test("process.env.OP_VAULT used when VaultID not set", () => {
-        process.env.OP_VAULT = "771c124d-edce-4bd7-a831-421d0c1f25c6";
-
-        const item = new ItemBuilder().setCategory(CategoryEnum.Login).build();
-
-        expect(item.vault).toBeDefined();
-        expect(item.vault.id).toEqual(process.env.OP_VAULT);
-    });
 });
diff --git a/__test__/op-connect.test.ts b/__test__/op-connect.test.ts
@@ -81,7 +81,6 @@ describe("Test OnePasswordConnect CRUD", () => {
         );
 
         const item = new ItemBuilder()
-            .setVault(VAULTID)
             .setCategory(CategoryEnum.Login)
             .build();
 

diff --git a/src/lib/builders.ts b/src/lib/builders.ts
@@ -49,17 +49,6 @@ export class ItemBuilder {
      * Performs final assembly of the new Item.
      */
     public build(): FullItem {
-        if (!this.item.vault || !this.item.vault.id) {
-            // Always fall back to using environment variable when vault is undefined
-            if (!process.env.OP_VAULT) {
-                throw Error("Vault ID is required.");
-            }
-            debug("Using OP_VAULT env var for new Item.");
-            this.item.vault = Object.assign(this.item.vault || {}, {
-                id: process.env.OP_VAULT,
-            });
-        }
-
         if (!this.item.category) {
             throw Error("Item Category is required.");
         }
@@ -75,7 +64,6 @@ export class ItemBuilder {
         debug(
             "Successfully built Item (id: %s, vault: %s)",
             builtItem.id,
-            builtItem.vault.id,
         );
         this.reset();
         return builtItem;
@@ -95,12 +83,13 @@ export class ItemBuilder {
     }
 
     /**
+     * @deprecated
      * Sets the parent Vault ID for the Item being constructed.
      *
      * @param {string} vaultId
      * @returns {ItemBuilder}
      */
-    public setVault(vaultId: string): ItemBuilder {
+     public setVault(vaultId: string): ItemBuilder {
         this.item.vault = { id: vaultId } as ItemVault;
         return this;
     }
@@ -211,7 +200,7 @@ export class ItemBuilder {
         this.item.category = category as FullItem.CategoryEnum;
         return this;
     }
-
+    
     /**
      * Creates a new Item Section if it does not exist. Otherwise, return the previously-created
      * Item Section.

diff --git a/src/lib/resources.ts b/src/lib/resources.ts
@@ -59,6 +59,10 @@ export class Items extends OPResource {
             : `v1/vaults/${vaultId}/items/`;
 
     public async create(vaultId: string, item: FullItem): Promise<FullItem> {
+        item.vault = Object.assign(item.vault || {}, {
+            id: vaultId
+        });
+
         const { data } = await this.adapter.sendRequest(
             "post",
             this.basePath(vaultId),