--- status: pending priority: p2 issue_id: "010" tags: [testing, helpers, refactoring, vendors, dry] dependencies: [] --- # Replace Custom create-vendor Helper with test-vendor from util.clj ## Problem Statement The vendors_test.clj defines a custom `create-vendor` helper (lines 18-25) that duplicates functionality already provided by `test-vendor` in `integration/util.clj`. This: - Violates DRY principle - Creates maintenance burden (two sources of truth) - May diverge over time ## Findings **From architecture-strategist and kieran-rails-reviewer:** **Current custom helper (lines 18-25):** ```clojure (defn create-vendor "Create a vendor with a unique temp id" [name & {:as attrs}] (merge {:db/id (str "vendor-" (java.util.UUID/randomUUID)) :vendor/name name :vendor/default-account "test-account-id"} attrs)) ``` **Existing utility (util.clj lines 41-44):** ```clojure (defn test-vendor [& kwargs] (apply assoc {:db/id "vendor-id" :vendor/name "Vendorson" :vendor/default-account "test-account-id"} kwargs)) ``` **Usage comparison:** ```clojure ;; Current (vendors_test.clj): (create-vendor "Test Vendor 1") ;; Could be (using test-vendor): (test-vendor :vendor/name "Test Vendor 1") ``` **Note:** The only difference is `create-vendor` generates unique IDs while `test-vendor` uses static ID. For unique IDs, can use: ```clojure (assoc (test-vendor :vendor/name "Test") :db/id (str "vendor-" (UUID/randomUUID))) ``` ## Proposed Solutions ### Option A: Use test-vendor Directly (Recommended) **Effort:** Small (15 minutes) **Risk:** Low Remove `create-vendor` and use `test-vendor` with assoc for unique IDs: ```clojure ;; Instead of: (setup-test-data [(create-vendor "Test Vendor 1")]) ;; Use: (setup-test-data [(test-vendor :vendor/name "Test Vendor 1")]) ;; For unique IDs: (setup-test-data [(assoc (test-vendor :vendor/name "Test Vendor 1") :db/id (str "vendor-" (UUID/randomUUID)))]) ``` **Pros:** - Single source of truth - Follows existing util.clj pattern - Removes duplication **Cons:** - More verbose for unique ID cases - Need to import UUID ### Option B: Enhance test-vendor in util.clj **Effort:** Small (10 minutes) **Risk:** Low Modify `test-vendor` to auto-generate unique IDs: ```clojure (defn test-vendor [& kwargs] (apply assoc {:db/id (str "vendor-" (java.util.UUID/randomUUID)) :vendor/name "Vendorson" :vendor/default-account "test-account-id"} kwargs)) ``` Then vendors_test.clj simply calls `(test-vendor :vendor/name "Test")`. **Pros:** - Improves utility for all test files - No local helper needed **Cons:** - May break existing tests expecting static ID - Requires updating util.clj (affects all tests) ### Option C: Create Named Vendor Helper **Effort:** Small (5 minutes) **Risk:** Low Keep local helper but use test-vendor internally: ```clojure (defn vendor-with-name [name] (test-vendor :vendor/name name)) ``` **Pros:** - Shorter than test-vendor call - Uses existing utility **Cons:** - Still have local helper - Less explicit than using test-vendor directly ## Recommended Action **Go with Option A** - use `test-vendor` directly: 1. Remove `create-vendor` helper 2. Update all usages to use `test-vendor` 3. Use `assoc` for unique IDs where needed This follows DRY principle and uses established patterns. ## Technical Details **Affected Code:** - Lines 18-25: Remove `create-vendor` function - Lines 43-44, 58-59, 75-76, 98-101, etc.: Replace usages **Files:** - `test/clj/auto_ap/ssr/admin/vendors_test.clj` **Verification:** ```bash lein test auto-ap.ssr.admin.vendors-test lein cljfmt check ``` ## Acceptance Criteria - [ ] `create-vendor` helper removed - [ ] All usages replaced with `test-vendor` - [ ] Unique IDs handled with `assoc` where needed - [ ] Tests pass - [ ] Code formatted with lein cljfmt ## Work Log ### 2026-02-07 - Initial Creation **By:** Architecture Strategist Agent **Actions:** - Identified duplicate helper function - Compared with existing test-vendor utility - Documented usage patterns **Learnings:** - DRY principle violation in test helpers - test-vendor already provides same functionality - Small duplication can create maintenance burden