Files
integreat/todos/010-pending-p2-replace-create-vendor-helper.md
Bryce a7daf839ec feat(tests): Add comprehensive tests for SSR admin vendors module
Add 8 BDD-style tests for the vendors module covering grid/list
operations and vendor merge functionality. Tests follow established
patterns from accounts_test.clj and include proper database
verification.

Tests Implemented:
- vendor-grid-loads-with-empty-database
- vendor-fetch-ids-returns-correct-structure
- vendor-fetch-page-returns-vendors
- vendor-hydrate-results-works
- vendor-merge-transfers-references
- vendor-merge-same-vendor-rejected
- vendor-merge-invalid-vendor-handled
- vendor-hydration-includes-all-fields

Key Implementation Details:
- Uses setup-test-data helper with unique temp IDs
- Tests focus on public interface (fetch-page, merge-submit)
- Follows BDD Given/When/Then pattern
- All 8 tests passing (26 assertions)

Documentation:
- Created implementation plan in docs/plans/
- Documented solution patterns in docs/solutions/
- Created code review todos for future improvements

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2026-02-06 23:53:31 -08:00

4.2 KiB

status, priority, issue_id, tags, dependencies
status priority issue_id tags dependencies
pending p2 010
testing
helpers
refactoring
vendors
dry

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

(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):

(defn test-vendor [& kwargs]
  (apply assoc {:db/id            "vendor-id"
                :vendor/name      "Vendorson"
                :vendor/default-account "test-account-id"}
         kwargs))

Usage comparison:

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

(assoc (test-vendor :vendor/name "Test") :db/id (str "vendor-" (UUID/randomUUID)))

Proposed Solutions

Effort: Small (15 minutes) Risk: Low

Remove create-vendor and use test-vendor with assoc for unique IDs:

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

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

(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

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:

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