Files
music_library/docs/project-conventions.md
T
Claudio Ortolina 96836ae715 Improve tests: better assertions, meaningful tests
Track resulting principles in project conventions
2026-03-27 15:46:31 +00:00

8.5 KiB

Project Conventions

Rules extracted from commit history that are specific to this project and not already covered by CLAUDE.md usage rules.

Commit Messages

  • Imperative present tense, single-line, under 60 characters
  • Describe intent/behavior, not implementation details
  • Reverts use Revert "Original message" convention
  • NEVER ADD a "Co-Authored-By" reference in the message body
  • If you're working on a GitHub issue, make sure to include the issue ID in the commit message body. Specifies if the commit closes the issue.

Architecture

  • Context modules own all queries. LiveViews never query the database directly -- they call context functions.
  • Schemas hold pure accessor/helper functions on the struct (e.g., RecordSet.count_by_status/1). No side effects in schemas.
  • LiveView standard structure: mount/3 sets @current_section, handle_params/3 loads data and sets @page_title (via pattern-matched private page_title/2), handle_info/2 receives LiveComponent messages.
  • LiveComponents communicate with parent via send(self(), {__MODULE__, msg}).
  • Function components go in domain-specific modules: CoreComponents for generic UI, RecordComponents for records, ScrobbleComponents for scrobbles, SearchComponents for search.
  • External API integrations follow a three-module pattern: Facade (public API), API (Req HTTP client), Config (NimbleOptions).
  • Behaviours are separate modules containing only @callback definitions. Concrete implementations use @behaviour and @impl true.
  • Oban workers are thin wrappers that delegate to context modules. perform/1 should be minimal.
  • Shared utilities live at parent namespace level (MusicLibrary.Batch, not MusicLibrary.Records.Batch).
  • Domain sub-modules group under their context. Modules strongly related to a context live as sub-modules (e.g., Chats.StreamProvider, Chats.RecordChat), distinct from shared utilities which live at the parent namespace level.
  • Shared LiveView logic goes in LiveHelpers.* modules. Event handlers, param parsing, and rendering helpers used across 2+ LiveViews are extracted to LiveHelpers.* rather than duplicated.

Extraction / Refactoring

  • Extract when duplicated 3+ times. Identical template markup in 3+ places becomes a function component.
  • Delete thin wrapper modules with a single caller -- inline them instead.
  • Parameterize the differences when extracting shared logic.
  • Private helpers go at module bottom, public functions first.

Template / UI

  • Gettext wraps ALL user-facing strings. Every commit that adds UI text must also update .pot/.po files.
  • Dark mode always paired: text-zinc-900 dark:text-zinc-100, bg-zinc-50 dark:bg-zinc-800.
  • Wishlisted items get dimmed styling: opacity-60 hover:opacity-100 transition-opacity.
  • Icons inside buttons use the icon class instead of explicit size classes (h-5 w-5, size-3.5, etc.). The icon class is provided by Fluxon and auto-sizes icons based on the button's size prop.
  • Artist name display uses the MusicBrainz joinphrase field — never join artist names with a literal ", ".
  • Charts use CSS Grid and responsive HTML, not SVG.

Routes / Navigation

  • Three routes per resource with show modals: :show, :edit (at /show/edit), :add_* (at /show/add-*).
  • Modals close via JS.patch back to the base route.
  • Search state in URL query params via push_patch.
  • Filter empty params from URLs: Enum.filter(fn {_, v} -> v not in ["", nil] end).
  • Conditional links based on purchased_at: determines /collection/ vs /wishlist/ paths.
  • /dev/ namespace is for developer tooling only (LiveDashboard, Oban Web, ErrorTracker). User-facing admin routes belong in the main authenticated live_session.

Database

  • SQLite JSON patterns: json_each() and json_extract() via fragment for JSON column queries. Expression-based indexes on json_extract for performance.
  • Materialized views via triggers (SQLite lacks native materialized views). Use explicit up/down in migrations for non-reversible DDL.
  • Read-only schemas for materialized/view tables: @primary_key false, no changeset functions, no timestamps.
  • Every execute provides both up and down SQL. Every index has a comment explaining which query it helps.
  • Config-driven constants. Pagination defaults and similar magic numbers live in config/config.exs, read via Application.compile_env!/2 into module attributes.

Error Handling

  • Toast notifications: put_toast/3 (arity 3) in LiveViews, put_toast!/2 (arity 2) in LiveComponents. :info for success, :error for failures.
  • User-facing error reasons use ErrorMessages.friendly_message/1 — never inspect(reason). Call sites keep their contextual prefix (e.g. gettext("Error refreshing cover")) and append ": " <> ErrorMessages.friendly_message(reason) for the reason part. Logger.error calls keep inspect for debugging.
  • handle_async always handles three cases: {:ok, {:ok, result}}, {:ok, {:error, reason}}, and {:exit, reason}.
  • Data cascade on upstream changes: When artist metadata changes, regenerate dependent record embeddings.
  • Non-actionable errors use ErrorTracker.Ignorer behaviour to filter (e.g., NoRouteError from bot scanners) rather than blocking paths in the endpoint.
  • Muted errors skip notifications. ErrorTracker.ErrorNotifier checks the muted flag before sending email notifications.

Testing

  • @tag :logged_out for public endpoint tests. @tag :capture_log on tests with expected error log output.
  • Fixture modules use System.unique_integer([:positive]) for unique names and call through context functions (not raw Repo.insert).
  • Verify outcomes through context modules, not just UI assertions. Delete tests assert both refute has_element? and assert_raise Ecto.NoResultsError.
  • render_hook/3 for testing JS hook interactions.
  • Avoid starting test descriptions with "it".
  • No boilerplate-only tests. Do not add test files that just verify Phoenix generator output (e.g., error view literal strings). Tests must exercise application behaviour.
  • Assert specific values, not just shape. Prefer assert data == expected or assert data["name"] == "Steven Wilson" over assert data != nil or assert {:ok, _} = result. Wildcard matches (_) in assertions are a signal the test is too vague.
  • Worker tests that enqueue jobs must assert_enqueued. perform_job returning {:ok, []} is not sufficient — verify the expected downstream workers were enqueued with correct args.
  • Do not test the same guard at every call site. If a shared check (e.g., session key presence) is enforced in one place, test it once. Do not duplicate the same assertion across every function that calls the shared check.
  • Consolidate identical assertions across endpoints. When multiple routes share the same plug/middleware behaviour (e.g., auth), test it once with a loop or parameterised approach, not N separate identical tests.
  • Error assertions must match the error type. assert {:error, _reason} is too broad — match the specific error struct or atom (e.g., %Req.TransportError{reason: :timeout}, :no_session_key).
  • Do not test untestable operations. If a database operation (e.g., VACUUM) cannot run in the test sandbox, do not write a test that asserts the sandbox error message. Delete or skip it.

Tech Debt / Hygiene

  • Clean up first, then enforce. Remove all violations before enabling a lint rule.
  • Reverts are total. Remove every trace: source, CSS, npm deps, config.
  • Never leak sensitive data in prod. show_sensitive_data_on_connection_error: false.
  • Commits are small and single-purpose. One logical change per commit.
  • Unused aliases are removed when their module is no longer referenced. Aliases stay alphabetically sorted.
  • Markdown sanitization via MDEx (ammonia). Use Markdown.to_html/1 for user content. Annotate raw output with # sobelow_skip ["XSS.Raw"] and a comment explaining the sanitization.
  • Sobelow runs on CI and pre-commit in skip mode for security analysis.
  • All modules require @moduledoc. The Credo ModuleDoc check is enforced in strict mode.
  • The project does not use dialyzer. If any skill suggests its usage, ignore it.

JavaScript

  • Factory function pattern for JS hooks when two hooks share logic.
  • Data attributes (data-*) for HTML-to-JS communication. Hooks read dataset and pushEvent to the server.