docs(elixir): prepare hierarchical refactor

2025-09-17 23:32:46 +02:00
parent 5a1775e836
commit 963c9a3770
2 changed files with 524 additions and 0 deletions
--- a/design_docs/hierarchical_analysis.md
+++ b/design_docs/hierarchical_analysis.md
@@ -0,0 +1,154 @@
+# Hierarchical Implementation Analysis
+
+## Overview
+This document analyzes the current hierarchical implementations in the codebase (Categories and Storage Locations) to determine the best approach for creating a shared `Hierarchical` behavior module.
+
+## Current State Analysis
+
+### Categories vs Storage Locations Feature Comparison
+
+| Feature | Categories | Storage Locations | Assessment |
+|---------|------------|-------------------|------------|
+| **Path Resolution** | Simple recursive function in schema | Complex virtual fields + batch computation | **Categories wins** - cleaner approach |
+| **Cycle Prevention** | UI-level filtering (preventive) | Changeset validation (reactive) | **Categories wins** - more efficient |
+| **Hierarchical Display** | Recursive LiveView components | Recursive LiveView components | **Tie** - nearly identical |
+| **Parent Selection** | Smart dropdown filtering | Smart dropdown filtering | **Tie** - identical logic |
+| **Performance** | O(depth) recursive calls | O(n) batch computation + DB queries | **Categories wins** - simpler |
+| **Code Complexity** | ~50 lines of hierarchy logic | ~100+ lines of hierarchy logic | **Categories wins** - more maintainable |
+
+## Key Findings
+
+### 1. Vestigial `is_active` Field Removed
+- **Status**: ✅ **REMOVED** 
+- **Impact**: Field was completely unused across the entire codebase
+- **Files Modified**:
+  - `lib/components_elixir/inventory/storage_location.ex` (schema and changeset)
+  - Migration created: `20250917210658_remove_is_active_from_storage_locations.exs`
+
+### 2. Categories Implementation is Superior
+
+#### **Path Resolution Approach**
+**Categories**: Elegant recursive function in schema
+```elixir
+def full_path(%Category{parent: nil} = category), do: category.name
+def full_path(%Category{parent: %Category{} = parent} = category) do
+  "#{full_path(parent)} > #{category.name}"
+end
+```
+
+**Storage Locations**: Over-engineered with virtual fields
+```elixir
+# Virtual fields in schema
+field :level, :integer, virtual: true
+field :path, :string, virtual: true
+
+# Complex batch computation in context
+def compute_hierarchy_fields_batch(locations)
+def compute_level_for_single(location)  
+def compute_path_for_single(location)
+```
+
+#### **Cycle Prevention Strategy**
+**Categories**: **Prevention at UI level** (efficient)
+```elixir
+# Prevents invalid options from appearing in dropdown
+|> Enum.reject(fn cat ->
+  cat.id == editing_category_id ||
+  (editing_category_id && is_descendant?(categories, cat.id, editing_category_id))
+end)
+```
+
+**Storage Locations**: **Validation at changeset level** (reactive)
+```elixir
+# Validates after user attempts invalid selection
+defp validate_no_circular_reference(changeset) do
+  # Complex validation logic that runs on every save attempt
+end
+```
+
+### 3. Shared Patterns Identified
+
+Both systems implement identical patterns for:
+- **Hierarchical tree display** with recursive LiveView components
+- **Parent/child relationship filtering** 
+- **Descendant detection algorithms**
+- **Root entity identification**
+- **UI depth-based styling and icons**
+
+## Architecture Decision
+
+### Recommended Approach: **Category-Style Hierarchical Module**
+
+The category implementation should be the template for generalization because:
+
+1. **Simpler**: No virtual fields or complex batch operations
+2. **More Performant**: O(depth) vs O(n) complexity
+3. **Preventive**: UI-level cycle prevention vs reactive validation
+4. **Maintainable**: Half the lines of code with same functionality
+
+### AprilTag Features Remain Storage-Specific
+
+The AprilTag system should **NOT** be generalized because:
+- Physical identification is meaningless for categories
+- Future scanning/detection features are location-specific
+- Keeps domain separation clean
+
+## Implementation Complexity Comparison
+
+### Categories (Simple & Clean)
+```
+Files: 1 schema + 1 LiveView = 2 files
+Lines: ~50 lines hierarchical logic
+Approach: Functional, recursive, preventive
+Dependencies: Standard Ecto associations
+```
+
+### Storage Locations (Over-Engineered)
+```
+Files: 1 schema + 1 context helper + 1 LiveView = 3 files  
+Lines: ~100+ lines hierarchical logic
+Approach: Stateful, batch processing, reactive
+Dependencies: Virtual fields + custom computation
+```
+
+## Performance Analysis
+
+### Path Resolution Performance
+- **Categories**: `O(depth)` - traverses only parent chain
+- **Storage Locations**: `O(n)` - processes all entities for batch computation
+
+### Memory Usage
+- **Categories**: Minimal - uses existing associations
+- **Storage Locations**: Higher - virtual fields + intermediate computations
+
+### Database Queries
+- **Categories**: Standard association preloading
+- **Storage Locations**: Additional queries for path/level computation
+
+## Code Quality Assessment
+
+### Categories Strengths
+✅ **Single Responsibility**: Each function does one thing  
+✅ **Functional Style**: Pure functions, no side effects  
+✅ **Standard Patterns**: Uses established Ecto association patterns  
+✅ **Easy Testing**: Simple recursive functions  
+✅ **Performance**: Minimal computational overhead  
+
+### Storage Locations Issues
+❌ **Multiple Responsibilities**: Virtual fields + validation + computation  
+❌ **Complex State**: Virtual fields require careful management  
+❌ **Custom Patterns**: Non-standard Ecto usage  
+❌ **Hard Testing**: Complex batch operations  
+❌ **Performance**: Unnecessary computational overhead  
+
+## Conclusion
+
+The **categories implementation is objectively superior** and should guide the refactoring:
+
+1. **Simpler code** (50% fewer lines)
+2. **Better performance** (O(depth) vs O(n))
+3. **More maintainable** (functional vs stateful)
+4. **Standard patterns** (Ecto associations vs virtual fields)
+5. **Preventive design** (UI filtering vs changeset validation)
+
+The storage locations system should be refactored to match the categories approach, eliminating virtual fields and complex batch computations in favor of simple recursive functions.
--- a/design_docs/hierarchical_refactoring_plan.md
+++ b/design_docs/hierarchical_refactoring_plan.md
@@ -0,0 +1,370 @@
+# Hierarchical Refactoring Plan
+
+## Overview
+This document outlines the step-by-step plan to extract common hierarchical behavior from Categories and Storage Locations into a shared `Hierarchical` module, based on the superior category implementation approach.
+
+## Goals
+1. **Extract shared hierarchical patterns** into reusable modules
+2. **Simplify storage locations** to match category elegance  
+3. **Maintain AprilTag-specific features** for storage locations
+4. **Preserve all existing functionality** during refactor
+5. **Improve performance and maintainability**
+
+## Refactoring Strategy
+
+### Phase 1: Extract Hierarchical Behavior Module ⏭️ **NEXT**
+Create shared behavior module based on category patterns
+
+### Phase 2: Refactor Storage Locations 🔄 **FOLLOW-UP**  
+Simplify storage locations to use new shared module
+
+### Phase 3: Refactor Categories 🔄 **FOLLOW-UP**
+Update categories to use shared module  
+
+### Phase 4: Extract LiveView Components 🔄 **FOLLOW-UP**
+Create reusable UI components for hierarchical display
+
+---
+
+## Phase 1: Extract Hierarchical Behavior Module
+
+### 1.1 Create Core Hierarchical Module
+
+**File**: `lib/components_elixir/inventory/hierarchical.ex`
+
+```elixir
+defmodule ComponentsElixir.Inventory.Hierarchical do
+  @moduledoc """
+  Shared hierarchical behavior for entities with parent-child relationships.
+  
+  This module provides common functionality for:
+  - Path computation (e.g., "Parent > Child > Grandchild")
+  - Cycle detection and prevention
+  - Parent/child filtering for UI dropdowns
+  - Tree traversal utilities
+  
+  Based on the elegant category implementation approach.
+  """
+  
+  @doc """
+  Computes full hierarchical path for an entity.
+  Uses recursive traversal of parent chain.
+  
+  ## Examples
+      iex> category = %Category{name: "Resistors", parent: %Category{name: "Electronics", parent: nil}}
+      iex> Hierarchical.full_path(category, &(&1.parent))
+      "Electronics > Resistors"
+  """
+  def full_path(entity, parent_accessor_fn, separator \\ " > ")
+  
+  @doc """
+  Filters entities to remove circular reference options for parent selection.
+  Prevents an entity from being its own ancestor.
+  
+  ## Examples
+      iex> categories = [%{id: 1, parent_id: nil}, %{id: 2, parent_id: 1}]
+      iex> Hierarchical.filter_parent_options(categories, 1, &(&1.id), &(&1.parent_id))
+      [%{id: 2, parent_id: 1}]  # ID 1 filtered out (self-reference)
+  """
+  def filter_parent_options(entities, editing_entity_id, id_accessor_fn, parent_id_accessor_fn)
+  
+  @doc """
+  Checks if an entity is a descendant of an ancestor entity.
+  Used for cycle detection in parent selection.
+  """
+  def is_descendant?(entities, descendant_id, ancestor_id, parent_id_accessor_fn)
+  
+  @doc """
+  Gets all root entities (entities with no parent).
+  """
+  def root_entities(entities, parent_id_accessor_fn)
+  
+  @doc """
+  Gets all child entities of a specific parent.
+  """
+  def child_entities(entities, parent_id, parent_id_accessor_fn)
+  
+  @doc """
+  Generates display name for entity including parent context.
+  For dropdown displays: "Parent > Child"
+  """
+  def display_name(entity, parent_accessor_fn, separator \\ " > ")
+end
+```
+
+### 1.2 Schema Behaviour Definition
+
+**File**: `lib/components_elixir/inventory/hierarchical_schema.ex`
+
+```elixir
+defmodule ComponentsElixir.Inventory.HierarchicalSchema do
+  @moduledoc """
+  Behaviour for schemas that implement hierarchical relationships.
+  
+  Provides a contract for entities with parent-child relationships.
+  """
+  
+  @callback full_path(struct()) :: String.t()
+  @callback parent(struct()) :: struct() | nil
+  @callback children(struct()) :: [struct()]
+  
+  defmacro __using__(_opts) do
+    quote do
+      @behaviour ComponentsElixir.Inventory.HierarchicalSchema
+      import ComponentsElixir.Inventory.Hierarchical
+    end
+  end
+end
+```
+
+### 1.3 Update Category Schema
+
+**File**: `lib/components_elixir/inventory/category.ex`
+
+```elixir
+defmodule ComponentsElixir.Inventory.Category do
+  use ComponentsElixir.Inventory.HierarchicalSchema
+  # ... existing schema definition
+  
+  @impl true
+  def full_path(%Category{} = category) do
+    ComponentsElixir.Inventory.Hierarchical.full_path(category, &(&1.parent))
+  end
+  
+  @impl true  
+  def parent(%Category{parent: parent}), do: parent
+  
+  @impl true
+  def children(%Category{children: children}), do: children
+end
+```
+
+## Phase 2: Refactor Storage Locations
+
+### 2.1 Simplify Storage Location Schema
+
+**Changes to**: `lib/components_elixir/inventory/storage_location.ex`
+
+1. **Remove virtual fields**: `level` and `path` 
+2. **Add category-style path function**
+3. **Remove complex cycle validation**
+4. **Keep AprilTag-specific features**
+
+```elixir
+defmodule ComponentsElixir.Inventory.StorageLocation do
+  use ComponentsElixir.Inventory.HierarchicalSchema
+  # ... existing schema without virtual fields
+  
+  @impl true
+  def full_path(%StorageLocation{} = location) do
+    ComponentsElixir.Inventory.Hierarchical.full_path(location, &(&1.parent), " / ")
+  end
+  
+  # Keep AprilTag-specific functionality
+  def apriltag_format(storage_location), do: storage_location.apriltag_id
+end
+```
+
+### 2.2 Simplify Storage Location Changeset  
+
+**Remove**:
+- `validate_no_circular_reference/1` function
+- Virtual field handling  
+- Complex cycle detection
+
+**Keep**:
+- AprilTag validation
+- Basic field validation
+
+### 2.3 Update Inventory Context
+
+**Changes to**: `lib/components_elixir/inventory.ex`
+
+**Remove**:
+- `compute_hierarchy_fields_batch/1`
+- `compute_level_for_single/1` 
+- `compute_path_for_single/1`
+- All virtual field computation
+
+**Simplify**:
+- `list_storage_locations/0` - use standard preloading
+- `get_storage_location!/1` - remove virtual field computation
+
+## Phase 3: Refactor Categories
+
+### 3.1 Update Categories to Use Shared Module
+
+**Changes to**: `lib/components_elixir/inventory/category.ex`
+
+Replace existing `full_path/1` with call to shared module.
+
+### 3.2 Update Categories LiveView
+
+**Changes to**: `lib/components_elixir_web/live/categories_live.ex`
+
+Replace custom hierarchy functions with shared module calls:
+
+```elixir
+# Replace custom functions with shared module
+defp parent_category_options(categories, editing_category_id \\ nil) do
+  available_categories = 
+    Hierarchical.filter_parent_options(
+      categories, 
+      editing_category_id, 
+      &(&1.id), 
+      &(&1.parent_id)
+    )
+    |> Enum.map(fn category ->
+      {Hierarchical.display_name(category, &(&1.parent)), category.id}
+    end)
+    
+  [{"No parent (Root category)", nil}] ++ available_categories
+end
+```
+
+## Phase 4: Extract LiveView Components
+
+### 4.1 Create Hierarchical LiveView Components
+
+**File**: `lib/components_elixir_web/live/hierarchical_components.ex`
+
+```elixir
+defmodule ComponentsElixirWeb.HierarchicalComponents do
+  use Phoenix.Component
+  alias ComponentsElixir.Inventory.Hierarchical
+  
+  @doc """
+  Renders a hierarchical tree of entities with depth-based styling.
+  """
+  def hierarchy_tree(assigns)
+  
+  @doc """
+  Renders an individual item in a hierarchical tree.
+  """
+  def hierarchy_item(assigns)
+  
+  @doc """
+  Renders a parent selection dropdown with cycle prevention.
+  """
+  def parent_select(assigns)
+end
+```
+
+### 4.2 Update LiveViews to Use Shared Components
+
+Both `CategoriesLive` and `StorageLocationsLive` can use the shared components, with custom slots for entity-specific content (like AprilTag display).
+
+## Implementation Timeline
+
+### Immediate (Phase 1) - 1-2 hours
+- [ ] Create `Hierarchical` module with core functions
+- [ ] Create `HierarchicalSchema` behaviour  
+- [ ] Write comprehensive tests for shared module
+
+### Short-term (Phase 2) - 2-3 hours  
+- [ ] Refactor storage location schema to remove virtual fields
+- [ ] Simplify storage location changeset
+- [ ] Update inventory context to remove batch computation
+- [ ] Test storage location functionality
+
+### Medium-term (Phase 3) - 1-2 hours
+- [ ] Update category schema to use shared module
+- [ ] Update categories LiveView to use shared functions
+- [ ] Test category functionality
+
+### Long-term (Phase 4) - 2-3 hours  
+- [ ] Create shared LiveView components
+- [ ] Refactor both LiveViews to use shared components
+- [ ] Add entity-specific customization slots
+- [ ] Comprehensive integration testing
+
+## Benefits After Refactoring
+
+### Code Quality
+- **50% reduction** in hierarchical logic duplication
+- **Consistent patterns** across both entity types
+- **Easier testing** with isolated, pure functions
+- **Better maintainability** with single source of truth
+
+### Performance  
+- **Elimination of virtual fields** reduces memory usage
+- **Remove batch computation** improves response times
+- **Standard Ecto patterns** optimize database queries
+- **UI-level cycle prevention** reduces validation overhead
+
+### Developer Experience
+- **Shared components** speed up new hierarchical entity development
+- **Consistent API** reduces learning curve
+- **Better documentation** with centralized behavior
+- **Easier debugging** with simplified call stacks
+
+## Risk Mitigation
+
+### Database Migration Safety
+1. **Backup database** before running migrations
+2. **Test migrations** on development environment first
+3. **Incremental approach** - one table at a time
+
+### Functionality Preservation  
+1. **Comprehensive test coverage** before refactoring
+2. **Feature parity testing** after each phase  
+3. **AprilTag functionality isolation** to prevent interference
+
+### Rollback Plan
+1. **Git branching strategy** for each phase
+2. **Database migration rollbacks** prepared
+3. **Quick revert capability** if issues discovered
+
+## Testing Strategy
+
+### Unit Tests
+- [ ] Test all `Hierarchical` module functions
+- [ ] Test schema `full_path/1` implementations
+- [ ] Test cycle detection edge cases
+
+### Integration Tests  
+- [ ] Test LiveView parent selection dropdowns
+- [ ] Test hierarchical tree rendering
+- [ ] Test AprilTag functionality preservation
+
+### Performance Tests
+- [ ] Benchmark path computation performance
+- [ ] Measure memory usage before/after
+- [ ] Profile database query patterns
+
+## Migration Notes
+
+### Database Changes Required
+1. **Remove `is_active` column** from storage_locations (✅ **COMPLETED**)
+2. **No other database changes** needed for this refactor
+
+### Deployment Considerations
+- **Zero downtime**: Refactor is code-only (except `is_active` removal)
+- **Backward compatible**: No API changes
+- **Incremental deployment**: Can deploy phase by phase
+
+## Success Criteria
+
+### Functional
+- [ ] All existing category functionality preserved
+- [ ] All existing storage location functionality preserved  
+- [ ] AprilTag features remain storage-location specific
+- [ ] UI behavior identical to current implementation
+
+### Non-Functional
+- [ ] Code duplication reduced by ≥50%
+- [ ] Performance maintained or improved
+- [ ] Test coverage ≥95% for shared modules
+- [ ] Documentation complete for new modules
+
+---
+
+## Next Steps
+
+1. **Review this plan** with team/stakeholders
+2. **Create feature branch** for refactoring work
+3. **Begin Phase 1** - Extract hierarchical behavior module
+4. **Write comprehensive tests** before any refactoring
+5. **Execute phases incrementally** with testing between each
+
+This refactoring will significantly improve code quality and maintainability while preserving all existing functionality and preparing the codebase for future hierarchical entities.