docs: add DRY refactor plan and update progress
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
db02c0b414
commit
5b08591a55
14
PROGRESS.md
14
PROGRESS.md
@ -7,7 +7,7 @@
|
||||
**Working:**
|
||||
- Theme editor with 8 presets, instant switching, full customization
|
||||
- Image optimization pipeline (AVIF/WebP/JPEG responsive variants)
|
||||
- Shop pages (home, collections, products, cart, about, contact, error)
|
||||
- Shop pages (home, collections, products, cart, about, contact, error, delivery, privacy, terms)
|
||||
- Mobile-first design with bottom navigation
|
||||
- 100% PageSpeed score
|
||||
- Variant selector with color swatches and size buttons
|
||||
@ -20,7 +20,7 @@
|
||||
- Transactional emails (order confirmation, shipping notification)
|
||||
- Demo content polished and ready for production
|
||||
|
||||
**Next up:** Default content pages — terms, privacy, delivery policy (Tier 1, Roadmap #4)
|
||||
**Tier 1 MVP complete.** Next up: Tier 2 — hosting & deployment.
|
||||
|
||||
## Roadmap
|
||||
|
||||
@ -29,7 +29,7 @@
|
||||
1. ~~**Order management admin**~~ — ✅ Complete (02cdc81). Admin UI at `/admin/orders` with status filter tabs, streamed order table, and detail view showing items, totals, and shipping address.
|
||||
2. ~~**Orders & fulfilment**~~ — ✅ Complete. Submit paid orders to Printify, track fulfilment status (submitted → processing → shipped → delivered), webhook-driven status updates with polling fallback, admin UI with submit/refresh actions.
|
||||
3. ~~**Transactional emails**~~ — ✅ Complete. Plain text order confirmation (on payment via Stripe webhook) and shipping notification (on dispatch via Printify webhook + polling fallback). OrderNotifier module, 10 tests.
|
||||
4. **Default content pages** — Static pages for terms of service, delivery & refunds policy, and privacy policy. Needed for legal compliance before taking real orders. Can be simple markdown-rendered pages initially, upgraded to editable via page editor later.
|
||||
4. ~~**Default content pages**~~ — ✅ Complete (5a43cfc). Generic `ShopLive.Content` LiveView handles about + 3 policy pages (delivery, privacy, terms) via `live_action`. Rich text with list blocks, footer links updated, theme editor preview. 10 tests (575 total).
|
||||
|
||||
### Tier 2 — Production readiness (can deploy and run reliably)
|
||||
|
||||
@ -201,6 +201,13 @@ See: [ROADMAP.md](ROADMAP.md) for design notes
|
||||
|
||||
See: [docs/plans/products-context.md](docs/plans/products-context.md) for schema design
|
||||
|
||||
### DRY Refactor
|
||||
**Status:** Planned
|
||||
|
||||
Codebase analysis identified ~380 lines of duplication across LiveViews, templates, and the theme editor. Top priorities: extract `ThemeHook` for shared mount logic, extract `<.shop_layout>` wrapper component, consolidate preview assigns. Also: split `shop_components.ex` (4,400 lines) into focused modules.
|
||||
|
||||
See: [docs/plans/dry-refactor.md](docs/plans/dry-refactor.md) for full analysis and plan
|
||||
|
||||
### Page Editor
|
||||
**Status:** Future (Tier 4)
|
||||
|
||||
@ -214,6 +221,7 @@ See: [docs/plans/page-builder.md](docs/plans/page-builder.md) for design
|
||||
|
||||
| Feature | Commit | Notes |
|
||||
|---------|--------|-------|
|
||||
| Default content pages | 5a43cfc | Generic Content LiveView, delivery/privacy/terms pages, 10 tests |
|
||||
| Transactional emails | — | Plain text order confirmation + shipping notification, 10 tests |
|
||||
| Printify order submission & fulfilment | — | Submit, track, webhooks, polling, admin UI, 33 tests |
|
||||
| Order management admin | 02cdc81 | List/detail views, status filters, 15 tests |
|
||||
|
||||
180
docs/plans/dry-refactor.md
Normal file
180
docs/plans/dry-refactor.md
Normal file
@ -0,0 +1,180 @@
|
||||
# DRY refactor plan
|
||||
|
||||
Status: Planned
|
||||
|
||||
Codebase analysis identifying repeated patterns and consolidation opportunities. Ordered by impact.
|
||||
|
||||
## High priority
|
||||
|
||||
### 1. Extract `<.shop_layout>` wrapper component
|
||||
|
||||
**Problem:** Every page template repeats ~28 lines of shell boilerplate: `#shop-container` div, `skip_link`, `announcement_bar`, `shop_header`, `shop_footer`, `cart_drawer`, `search_modal`, `mobile_bottom_nav`. That's ~224 lines of copy-paste across 8 templates.
|
||||
|
||||
**Fix:** Create a `shop_layout/1` component in `shop_components.ex` that accepts an inner block and common attrs. Templates become just their unique `<main>` content.
|
||||
|
||||
**Before:**
|
||||
```heex
|
||||
<div id="shop-container" phx-hook="CartPersist" class="shop-container min-h-screen pb-20 md:pb-0"
|
||||
style="background-color: var(--t-surface-base); ...">
|
||||
<.skip_link />
|
||||
<%= if @theme_settings.announcement_bar do %>
|
||||
<.announcement_bar theme_settings={@theme_settings} />
|
||||
<% end %>
|
||||
<.shop_header theme_settings={@theme_settings} logo_image={@logo_image} ... />
|
||||
|
||||
<main id="main-content">
|
||||
<!-- unique content -->
|
||||
</main>
|
||||
|
||||
<.shop_footer theme_settings={@theme_settings} mode={@mode} />
|
||||
<.cart_drawer cart_items={@cart_items} subtotal={@cart_subtotal} ... />
|
||||
<.search_modal hint_text={~s(...)} />
|
||||
<.mobile_bottom_nav active_page="home" mode={@mode} />
|
||||
</div>
|
||||
```
|
||||
|
||||
**After:**
|
||||
```heex
|
||||
<.shop_layout {assigns}>
|
||||
<main id="main-content">
|
||||
<!-- unique content -->
|
||||
</main>
|
||||
</.shop_layout>
|
||||
```
|
||||
|
||||
**Files:** `shop_components.ex` (new component), all 8 page templates, `collection.ex` inline render.
|
||||
|
||||
**Complexity:** Medium. The error template has slight variations (no `phx-hook`, different class). Handle with an optional attr or keep error as a special case.
|
||||
|
||||
### 2. Extract theme loading into on_mount hook
|
||||
|
||||
**Problem:** 7 shop LiveViews independently load theme settings, check CSS cache, fetch logo/header, assign 5 common values. ~12-15 identical lines per file, ~75 lines total.
|
||||
|
||||
**Fix:** Create a `ThemeHook` (like `CartHook`) using `on_mount` that assigns `theme_settings`, `generated_css`, `logo_image`, `header_image`, `mode`. Add it to the `:public_shop` live_session alongside `CartHook`.
|
||||
|
||||
**Before (every LiveView):**
|
||||
```elixir
|
||||
def mount(_params, _session, socket) do
|
||||
theme_settings = Settings.get_theme_settings()
|
||||
generated_css = case CSSCache.get() do
|
||||
{:ok, css} -> css
|
||||
:miss -> css = CSSGenerator.generate(theme_settings); CSSCache.put(css); css
|
||||
end
|
||||
socket =
|
||||
socket
|
||||
|> assign(:theme_settings, theme_settings)
|
||||
|> assign(:generated_css, generated_css)
|
||||
|> assign(:logo_image, Media.get_logo())
|
||||
|> assign(:header_image, Media.get_header())
|
||||
|> assign(:mode, :shop)
|
||||
# ... page-specific assigns
|
||||
end
|
||||
```
|
||||
|
||||
**After:**
|
||||
```elixir
|
||||
# Router
|
||||
live_session :public_shop,
|
||||
on_mount: [{CartHook, :mount_cart}, {ThemeHook, :mount_theme}] do
|
||||
# ...
|
||||
end
|
||||
|
||||
# LiveView - only page-specific logic remains
|
||||
def mount(_params, _session, socket) do
|
||||
socket = assign(socket, :page_title, "Home")
|
||||
{:ok, socket}
|
||||
end
|
||||
```
|
||||
|
||||
**Files:** New `theme_hook.ex`, `router.ex` (add to on_mount), all 7 shop LiveViews (remove boilerplate).
|
||||
|
||||
**Complexity:** Low. Follows established `CartHook` pattern exactly.
|
||||
|
||||
### 3. Extract common preview assigns helper
|
||||
|
||||
**Problem:** 10 `preview_page` clauses in `theme_live/index.ex` each pass the same 8 common attrs (theme_settings, logo_image, header_image, mode, cart_items, cart_count, cart_subtotal, cart_drawer_open). ~80 lines of duplication.
|
||||
|
||||
**Fix:** Extract a private `common_preview_assigns/1` function. Each clause merges page-specific attrs on top.
|
||||
|
||||
**Files:** `lib/simpleshop_theme_web/live/theme_live/index.ex`
|
||||
|
||||
**Complexity:** Low. Pure refactor within one file.
|
||||
|
||||
## Medium priority
|
||||
|
||||
### 4. Split `shop_components.ex` into focused modules
|
||||
|
||||
**Problem:** 4,400 lines, 55 public functions. Covers navigation, products, cart, contact, content rendering, forms. Hard to navigate, slow to compile.
|
||||
|
||||
**Fix:** Split into ~5 modules:
|
||||
- `ShopLayoutComponents` — header, footer, mobile_bottom_nav, skip_link, announcement_bar, shop_layout
|
||||
- `ShopProductComponents` — product_card, product_grid, product_gallery, variant_selector, related_products
|
||||
- `ShopCartComponents` — cart_drawer, cart_item, cart_layout, order_summary
|
||||
- `ShopContentComponents` — rich_text, hero_section, image_text_section, responsive_image
|
||||
- `ShopFormComponents` — contact_form, newsletter_card, search_modal
|
||||
|
||||
Keep `ShopComponents` as a facade that imports all sub-modules for backward compatibility.
|
||||
|
||||
**Files:** `shop_components.ex` split into 5 new files.
|
||||
|
||||
**Complexity:** Medium. Mechanical but touches many imports. Need to update `page_templates.ex` and any direct references.
|
||||
|
||||
### 5. Unify cart hydration between CartHook and CheckoutController
|
||||
|
||||
**Problem:** `CheckoutController` manually calls `Cart.hydrate()` and `Enum.reduce` for subtotal instead of using `Cart.calculate_subtotal/1`. Duplicates logic from `CartHook.update_cart_assigns/2`.
|
||||
|
||||
**Fix:** Extract a `Cart.build_state/1` function that returns `%{items: hydrated, count: n, subtotal: formatted}`. Both CartHook and CheckoutController use it.
|
||||
|
||||
**Files:** `cart.ex`, `cart_hook.ex`, `checkout_controller.ex`
|
||||
|
||||
**Complexity:** Low.
|
||||
|
||||
### 6. Consolidate encryption logic
|
||||
|
||||
**Problem:** Both `Settings.put_secret/2` and `ProviderConnection` changeset independently call `Vault.encrypt()` with different error handling patterns.
|
||||
|
||||
**Fix:** Add `Vault.encrypt!/1` that raises on failure (for changeset use) and keep `Vault.encrypt/1` for tuple returns. Or create an `Ecto.Type` for encrypted fields.
|
||||
|
||||
**Files:** `vault.ex`, `settings.ex`, `provider_connection.ex`
|
||||
|
||||
**Complexity:** Low-medium.
|
||||
|
||||
## Low priority
|
||||
|
||||
### 7. Consolidate Settings repo lookups
|
||||
|
||||
**Problem:** `get_setting`, `get_secret`, `has_secret?`, `secret_hint` each independently call `Repo.get_by(Setting, key: key)`.
|
||||
|
||||
**Fix:** Extract `defp fetch_setting(key)` that returns `{:ok, setting} | :not_found`.
|
||||
|
||||
**Files:** `settings.ex`
|
||||
|
||||
**Complexity:** Trivial.
|
||||
|
||||
### 8. Scalable secrets loading
|
||||
|
||||
**Problem:** `Secrets.load_stripe_config/0` hardcodes Stripe key loading. Adding more providers means duplicating the pattern.
|
||||
|
||||
**Fix:** Registry of `{settings_key, app, env_key}` tuples. `load_all/0` iterates.
|
||||
|
||||
**Files:** `secrets.ex`
|
||||
|
||||
**Complexity:** Trivial but low value until more providers are added.
|
||||
|
||||
## What's already good
|
||||
|
||||
- **CartHook** — textbook shared behaviour via `attach_hook`. Model for ThemeHook.
|
||||
- **Router** — clean pipeline composition, no duplication.
|
||||
- **CoreComponents vs ShopComponents** — proper separation, no overlap.
|
||||
- **Provider abstraction** — clean dispatch with Mox support.
|
||||
- **Context boundaries** — Products, Orders, Settings, Media are well-separated.
|
||||
- **Page LiveView renders** — now all use `{assigns}` spread (just refactored).
|
||||
|
||||
## Suggested order
|
||||
|
||||
1. ThemeHook (item 2) — lowest risk, biggest clarity win
|
||||
2. shop_layout component (item 1) — biggest line reduction
|
||||
3. Preview assigns helper (item 3) — quick win, one file
|
||||
4. Cart state builder (item 5) — small but prevents bugs
|
||||
5. Split shop_components (item 4) — larger effort, do when convenient
|
||||
6. Everything else — opportunistic
|
||||
Loading…
Reference in New Issue
Block a user