Bryce
a7daf839ec
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>
134 lines
4.4 KiB
Markdown
134 lines
4.4 KiB
Markdown
---
|
|
status: pending
|
|
priority: p1
|
|
issue_id: "006"
|
|
tags: [testing, imports, code-quality, vendors, clojure]
|
|
dependencies: []
|
|
---
|
|
|
|
# Fix Import Organization and Remove Unused Imports in vendors_test.clj
|
|
|
|
## Problem Statement
|
|
|
|
The vendors test file has import organization issues that violate project conventions:
|
|
1. Imports are not sorted alphabetically within groups
|
|
2. Multiple unused imports (test-account, test-client, test-vendor, user-token, clojure.string)
|
|
3. Inconsistent with accounts_test.clj pattern
|
|
|
|
This creates technical debt and confusion for developers trying to follow established patterns.
|
|
|
|
## Findings
|
|
|
|
**Current imports (vendors_test.clj lines 1-14):**
|
|
```clojure
|
|
(:require
|
|
[auto-ap.datomic :refer [conn]] ; internal
|
|
[auto-ap.integration.util :refer [admin-token ; internal
|
|
setup-test-data
|
|
test-account ; UNUSED
|
|
test-client ; UNUSED
|
|
test-vendor ; UNUSED
|
|
user-token ; UNUSED
|
|
wrap-setup]]
|
|
[auto-ap.ssr.admin.vendors :as sut] ; internal
|
|
[clojure.string :as str] ; stdlib
|
|
[clojure.test :refer [deftest is testing use-fixtures]] ; stdlib
|
|
[datomic.api :as dc]) ; third-party
|
|
```
|
|
|
|
**Issues identified by kieran-rails-reviewer:**
|
|
- Unused: `test-account`, `test-client`, `test-vendor`, `user-token`, `clojure.string`
|
|
- `auto-ap.integration.util` imports not alphabetized (admin-token before setup-test-data)
|
|
- Missing `:as t` alias for clojure.test (accounts_test.clj uses this)
|
|
|
|
**Expected order (per AGENTS.md):**
|
|
- Standard library imports first (clojure.test, clojure.string)
|
|
- Third-party libraries second (datomic.api)
|
|
- Internal library imports last (auto-ap.*)
|
|
|
|
## Proposed Solutions
|
|
|
|
### Option A: Minimal Fix (Recommended)
|
|
**Effort:** Small (5 minutes)
|
|
**Risk:** Low
|
|
|
|
Remove unused imports and reorganize:
|
|
```clojure
|
|
(:require
|
|
[auto-ap.datomic :refer [conn]]
|
|
[auto-ap.integration.util :refer [admin-token setup-test-data wrap-setup]]
|
|
[auto-ap.ssr.admin.vendors :as sut]
|
|
[clojure.test :refer [deftest is testing use-fixtures]]
|
|
[datomic.api :as dc])
|
|
```
|
|
|
|
**Pros:**
|
|
- Quick fix
|
|
- Addresses immediate issues
|
|
- Follows existing accounts_test.clj pattern
|
|
|
|
**Cons:**
|
|
- Still doesn't match AGENTS.md import ordering standard
|
|
|
|
### Option B: Full Standards Compliance
|
|
**Effort:** Small (10 minutes)
|
|
**Risk:** Low
|
|
|
|
Reorder to match AGENTS.md standard:
|
|
```clojure
|
|
(:require
|
|
[clojure.test :refer [deftest is testing use-fixtures]] ; stdlib first
|
|
[datomic.api :as dc] ; third-party second
|
|
[auto-ap.datomic :refer [conn]] ; internal last
|
|
[auto-ap.integration.util :refer [admin-token setup-test-data wrap-setup]]
|
|
[auto-ap.ssr.admin.vendors :as sut])
|
|
```
|
|
|
|
**Pros:**
|
|
- Follows project standards exactly
|
|
- Consistent with documented conventions
|
|
|
|
**Cons:**
|
|
- Different from accounts_test.clj pattern
|
|
- May require updating accounts_test.clj too for consistency
|
|
|
|
## Recommended Action
|
|
|
|
**Go with Option A** - minimal fix that removes unused imports and matches accounts_test.clj pattern. This provides immediate value without introducing inconsistency with the reference test file.
|
|
|
|
## Technical Details
|
|
|
|
**Affected Files:**
|
|
- `test/clj/auto_ap/ssr/admin/vendors_test.clj` (lines 1-14)
|
|
|
|
**Dependencies:** None
|
|
|
|
**Verification:**
|
|
```bash
|
|
lein test auto-ap.ssr.admin.vendors-test # Ensure tests still pass
|
|
lein cljfmt check # Verify formatting
|
|
```
|
|
|
|
## Acceptance Criteria
|
|
|
|
- [ ] All unused imports removed (test-account, test-client, test-vendor, user-token, clojure.string)
|
|
- [ ] Remaining imports organized consistently with accounts_test.clj
|
|
- [ ] Tests continue to pass
|
|
- [ ] Code formatted with lein cljfmt
|
|
|
|
## Work Log
|
|
|
|
### 2026-02-07 - Initial Creation
|
|
|
|
**By:** Code Review Agent
|
|
|
|
**Actions:**
|
|
- Identified import issues during comprehensive review
|
|
- Documented current state and expected patterns
|
|
- Created todo for tracking
|
|
|
|
**Learnings:**
|
|
- accounts_test.clj serves as reference pattern
|
|
- AGENTS.md documents import ordering standards
|
|
- Multiple agents flagged this as inconsistency
|