integreat/todos/010-pending-p2-replace-create-vendor-helper.md

---
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