graham/towerops
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions
towerops/REFACTOR_SUMMARY.md

3.8 KiB
Raw Blame History

DRY Refactoring Summary - FINAL

Completed: 15 out of 20 findings (75%)

✅ All High-Impact Findings Complete (5/5)

  1. #1: Merged duplicate credential_source_atom functions

    • Consolidated two near-identical functions using pattern matching
    • Eliminated 7 lines of duplication

  2. #2: Extracted propagate_credential_change/3 helper

    • Refactored 6 near-identical functions to use common helper
    • Eliminated ~100 lines of duplicated code

  3. #6: Merged get_fallback_agent_token variants

    • Used pattern matching instead of cond
    • One function delegates to the other

  4. #10: Extracted do_resolve_alert and do_acknowledge_alert helpers

    • Shared changeset + update logic for alert operations
    • Each public function focuses on side effects

  5. #11: Introduced Towerops.Time.now() utility

    • Replaced 199 instances across 74 files
    • Single source of truth for timestamps

✅ Medium/Low Impact Complete (10/15)

  1. #9: Simplified normalize_attrs_alert_type to delegate
  2. #12: Removed sanitize_like/1 wrappers (5 files)
  3. #17: Module attributes for repeated preloads (@device_preloads, @site_preloads)
  4. #14: Pattern matching in AccessControl verify_ functions*
  5. #15: Simplified mikrotik_use_ssl default logic
  6. #7: Extracted maybe_preselect_site/3 helper
  7. #8: Extracted find_org_agent/2 helper
  8. #16: Pattern matching in should_trigger_discovery?
  9. #18: maybe_filter_ helpers in list_organization_devices*
  10. #5: Simplified determine_effective_agent_id

⏭️ Deferred for Future PR (2/5)

#3: Repeated form LiveView pattern (4 files)

  • Complex: Requires behavior module or macro abstraction
  • Impact: Would eliminate handle_params/validate/save boilerplate
  • Recommendation: Separate PR with comprehensive testing

#4: Repeated access control error handling (6+ files)

  • Complex: AccessControl needs socket-ready response helpers
  • Impact: Would eliminate repeated flash + error handling blocks
  • Recommendation: Separate focused PR

⏭️ Skipped - Minimal Benefit (3/5)

#13: handle_params tab/filter pattern

  • Reason: Pattern already clear (Map.get(params, key, default))

#19: Collapse severity_color + severity_badge_class

  • Reason: severity_color used independently for text colors

#20: Consolidate time-formatting helpers

  • Reason: Different semantics (seconds vs minutes), context-appropriate

Final Impact

Code Changes

  • Files changed: 90+
  • Commits: 16
  • Net code reduction: ~300+ lines eliminated through deduplication
  • New utilities added: Towerops.Time module

Quality Improvements

  • ✅ All high-impact duplication eliminated
  • ✅ More idiomatic Elixir (pattern matching over cond/if)
  • ✅ Better composability (query filter helpers)
  • ✅ Single source of truth (Time.now, preload lists, credential helpers)
  • ✅ Reduced maintenance burden

Test Coverage

  • All existing tests pass
  • No behavioral changes - pure refactoring
  • Compilation warnings unchanged (only pre-existing proto warnings)

Remaining Work for Future PRs

  1. Form LiveView Pattern (#3) - Medium Priority

    • Create reusable behavior or macro for form LiveViews
    • Apply across schedule_live, escalation_policy_live, maintenance_live, site_live
    • Estimated effort: 2-3 hours with comprehensive testing

  2. Access Control Helpers (#4) - Low Priority

    • Add flash/navigate helpers to AccessControl module
    • Update 6+ LiveView files to use new helpers
    • Estimated effort: 1-2 hours

Recommendations

✅ Merge this PR - High-impact work complete, code quality significantly improved

🚀 Future optimization - Tackle #3 and #4 when refactoring LiveView patterns more broadly

📊 Metrics - 75% of findings addressed, 100% of high-impact duplication eliminated