towerops/docs/plans/make-sites-optional.md

Refactor Plan: Make Sites Optional in Towerops

Goal

Remove the "create site first" wall for new users. Allow devices to belong directly to organizations without requiring site creation, while preserving backward compatibility for existing multi-site users.

Architecture Decision

Approach: True optional sites with nullable site_id

• Add organization_id foreign key directly to devices table
• Make site_id nullable
• Support 2-tier credential cascade (org → device) for site-less devices
• Preserve 3-tier cascade (org → site → device) when site is present

Current State Analysis

Database Schema

Organization (id)
  └── Site (organization_id, NOT NULL)
      └── Device (site_id, NOT NULL)  ← Devices MUST have site

Problems:

• Devices have NO direct organization_id - only indirect via site.organization_id
• Authorization checks crash if site is nil: device.site.organization_id
• Credential resolution crashes: device.site.snmp_community

Code Impact

• 42 functions access device.site.* pattern and will crash if site is nil
• 17 credential resolution functions in lib/towerops/devices.ex need refactoring
• 5 API controller functions use device.site.organization_id for auth
• 3-tier credential cascade for SNMP v2, SNMP v3, and MikroTik credentials

MVP Scope (Minimal Viable Onboarding Fix)

In Scope ✅

1. Database migrations (add organization_id, make site_id nullable)
2. Core credential resolution refactor (CredentialResolver module)
3. Device form - remove site requirement, make site optional
4. Basic authorization fixes (API controllers, agent channel)
5. Core query functions (list_organization_devices, etc.)

Out of Scope (Future Enhancements) 🚫

• Progressive disclosure UI (use_sites toggle in org settings)
• Conditional navigation (hiding "Sites" link)
• Device grouping UI changes (flat vs grouped view)
• Automatic default site creation
• Settings page site toggle
• Help text and education materials
• Mobile app changes

Rationale: Focus on core backend functionality first. UI polish can come in follow-up iterations.

---

Implementation Plan

Phase 1: Database Schema Changes

Migration 1: Add organization_id (with backfill)

File: priv/repo/migrations/XXXXXX_add_organization_id_to_devices.exs

def up do
  # 1. Add nullable organization_id column
  alter table(:devices) do
    add :organization_id, references(:organizations, type: :binary_id, on_delete: :delete_all)
  end

  # 2. Backfill from existing site relationships
  execute """
  UPDATE devices d
  SET organization_id = s.organization_id
  FROM sites s
  WHERE d.site_id = s.id
  """

  # 3. Make organization_id NOT NULL (data is backfilled)
  alter table(:devices) do
    modify :organization_id, :binary_id, null: false
  end

  # 4. Add indexes
  create index(:devices, [:organization_id])
  create index(:devices, [:organization_id, :display_order, :name])

  # 5. Add check constraint (site must belong to same org)
  create constraint(:devices, :devices_site_organization_match,
    check: """
    site_id IS NULL OR
    organization_id = (SELECT organization_id FROM sites WHERE id = site_id)
    """
  )
end

def down do
  drop constraint(:devices, :devices_site_organization_match)
  drop index(:devices, [:organization_id, :display_order, :name])
  drop index(:devices, [:organization_id])

  alter table(:devices) do
    remove :organization_id
  end
end

Safety:

• No downtime (backfill happens before NOT NULL)
• Rollbackable (can drop organization_id column)
• Check constraint prevents invalid data

Migration 2: Make site_id nullable

File: priv/repo/migrations/XXXXXX_make_site_id_nullable_on_devices.exs

def up do
  alter table(:devices) do
    modify :site_id, :binary_id, null: true, from: {:binary_id, null: false}
  end
end

def down do
  # Cannot rollback if NULL site_id values exist
  execute """
  DO $$
  BEGIN
    IF EXISTS (SELECT 1 FROM devices WHERE site_id IS NULL) THEN
      RAISE EXCEPTION 'Cannot revert: devices with NULL site_id exist';
    END IF;
  END $$;
  """

  alter table(:devices) do
    modify :site_id, :binary_id, null: false
  end
end

Safety: One-way migration once devices without sites exist

---

Phase 2: Core Schema Updates

Update Device Schema

File: lib/towerops/devices/device.ex

Changes:

1. Add belongs_to :organization, Organization (new field)
2. Change validate_required to include :organization_id, remove :site_id
3. Add validate_site_belongs_to_organization/1 function
4. Update typespec to show site_id: UUID.t() | nil

Key validation:

defp validate_site_belongs_to_organization(changeset) do
  site_id = get_field(changeset, :site_id)
  org_id = get_field(changeset, :organization_id)

  if site_id && org_id do
    site = Repo.get!(Site, site_id)
    if site.organization_id != org_id do
      add_error(changeset, :site_id, "must belong to the same organization")
    else
      changeset
    end
  else
    changeset
  end
end

---

Phase 3: Credential Resolution Refactor

Create CredentialResolver Module

File: lib/towerops/devices/credential_resolver.ex (new file)

Centralizes safe credential resolution with 2-tier and 3-tier support:

defmodule Towerops.Devices.CredentialResolver do
  @moduledoc """
  Safe credential resolution with optional sites.

  Supports:
  - 2-tier: device → organization (when site_id is nil)
  - 3-tier: device → site → organization (when site_id present)
  """

  def ensure_credentials_loaded(%Device{} = device) do
    cond do
      !Ecto.assoc_loaded?(device.organization) ->
        preload_all(device)
      device.site_id && !Ecto.assoc_loaded?(device.site) ->
        Repo.preload(device, [:site])
      true ->
        device
    end
  end

  defp preload_all(device) do
    if device.site_id do
      Repo.preload(device, [:organization, :site])
    else
      Repo.preload(device, [:organization])
    end
  end

  def resolve_field(device, device_field, site_field, org_field, opts \\ []) do
    default = Keyword.get(opts, :default)
    device = ensure_credentials_loaded(device)

    device_value = Map.get(device, device_field)

    site_value =
      if device.site_id && device.site do
        Map.get(device.site, site_field)
      else
        nil
      end

    org_value = Map.get(device.organization, org_field)

    device_value || site_value || org_value || default
  end

  def determine_source(device, device_field, site_field, org_field) do
    device = ensure_credentials_loaded(device)

    cond do
      Map.get(device, device_field) != nil -> :device
      device.site_id && device.site && Map.get(device.site, site_field) != nil -> :site
      Map.get(device.organization, org_field) != nil -> :organization
      true -> :default
    end
  end
end

Update Credential Resolution Functions

File: lib/towerops/devices.ex

Refactor these functions (lines 299-628):

• resolve_snmp_config/1 - SNMP v2c credentials
• resolve_snmpv3_config/1 - SNMP v3 credentials (6 fields)
• resolve_mikrotik_config/1 - MikroTik API credentials (5 fields)

Pattern:

# BEFORE (crashes if site is nil):
defp resolve_snmp_community(device) do
  device.snmp_community ||
    device.site.snmp_community ||  # ⚠️ Crashes
    device.site.organization.snmp_community
end

# AFTER (safe with CredentialResolver):
alias Towerops.Devices.CredentialResolver

defp resolve_snmp_community(device) do
  CredentialResolver.resolve_field(
    device,
    :snmp_community,
    :snmp_community,
    :snmp_community
  )
end

---

Phase 4: Query Function Updates

Update Organization Device Queries

File: lib/towerops/devices.ex

Functions to update:

1. list_organization_devices/2 (lines 39-63)
2. count_organization_devices/1 (lines 83-91)
3. list_devices_for_organizations/1 (lines 68-78)
4. list_mikrotik_devices_with_api/0 (lines 141-156)

Pattern:

# BEFORE (requires site join):
def list_organization_devices(organization_id, filters) do
  from(d in Device,
    join: s in assoc(d, :site),  # ⚠️ Inner join excludes site-less devices
    where: s.organization_id == ^organization_id,
    preload: [site: s]
  )
end

# AFTER (direct organization_id):
def list_organization_devices(organization_id, filters) do
  from(d in Device,
    where: d.organization_id == ^organization_id,
    order_by: [asc: d.display_order, asc: d.name],
    preload: [:site, :organization]  # Left join - includes site-less devices
  )
end

Update Preload Functions

Add safe preload helper:

def preload_device_with_associations(device) do
  if device.site_id do
    Repo.preload(device, [:organization, :site])
  else
    Repo.preload(device, [:organization])
  end
end

Update get_device/1, get_device!/1, get_device_with_details/1 to use it.

---

Phase 5: Authorization Fixes

API Controllers

File: lib/towerops_web/controllers/api/v1/devices_controller.ex

Update authorization checks (lines 147, 190, 236):

# BEFORE:
if device.site.organization_id == organization_id do  # ⚠️ Crashes

# AFTER:
if device.organization_id == organization_id do  # ✅ Safe

Update site verification (line 260):

# BEFORE:
defp verify_site_access(nil, _organization_id),
  do: {:error, "site_id is required"}

# AFTER (site is optional):
defp verify_site_access(nil, _organization_id),
  do: :ok

Agent Channel

File: lib/towerops_web/channels/agent_channel.ex

Update device verification (line 639):

# BEFORE:
if is_nil(organization_id) or device.site.organization_id == organization_id do

# AFTER:
if is_nil(organization_id) or device.organization_id == organization_id do

---

Phase 6: Device Form Updates

Remove Site Requirement

File: lib/towerops_web/live/device_live/form.ex

Remove blocking check (lines 34-38):

# DELETE THIS:
if Enum.empty?(sites) do
  {:ok,
   socket
   |> put_flash(:info, "Please create a site before adding a device.")
   |> push_navigate(to: ~p"/sites/new")}

Update changeset to auto-inject organization_id:

def handle_event("save", %{"device" => device_params}, socket) do
  organization = socket.assigns.current_scope.organization

  # Inject organization_id from current context
  device_params = Map.put(device_params, "organization_id", organization.id)

  save_device(socket, socket.assigns.live_action, device_params)
end

Make Site Selector Optional in Form

File: lib/towerops_web/live/device_live/form.html.heex

Update site input (lines 168-177):

<!-- Site is now optional -->
<div class="col-span-full">
  <.input
    field={@form[:site_id]}
    type="select"
    label="Site (Optional)"
    prompt="No site (ungrouped)"
    options={Enum.map(@available_sites, &{&1.name, &1.id})}
  />
  <p class="mt-1 text-sm text-gray-500">
    Optionally group this device into a site (office, datacenter, etc.)
  </p>
</div>

---

Phase 7: API Updates

Device Create Endpoint

File: lib/towerops_web/controllers/api/v1/devices_controller.ex

Update to auto-inject organization_id:

def create(conn, %{"device" => device_params}) do
  organization_id = conn.assigns.current_organization_id

  # Inject organization_id from authenticated context
  device_params = Map.put(device_params, "organization_id", organization_id)

  # site_id is now optional
  with :ok <- verify_site_access(device_params["site_id"], organization_id),
       {:ok, device} <- Devices.create_device(device_params) do
    # ...
  end
end

---

Phase 8: Agent Assignment Updates

Safe Agent Resolution

File: lib/towerops/devices.ex (lines 167-189)

Update agent fallback chain:

def resolve_agent_token_id(device) do
  device =
    if device.site_id do
      Repo.preload(device, [:organization, :site, agent_assignments: []])
    else
      Repo.preload(device, [:organization, agent_assignments: []])
    end

  get_direct_agent(device) ||
    get_site_agent(device) ||
    get_org_default_agent(device) ||
    get_global_default_agent()
end

# Safe: only access site if site_id present
defp get_site_agent(device) do
  if device.site_id && device.site do
    device.site.agent_token_id
  else
    nil
  end
end

# Safe: use direct organization relationship
defp get_org_default_agent(device) do
  device.organization.default_agent_token_id
end

---

Phase 9: Propagation Functions

Update Credential Propagation

File: lib/towerops/devices.ex

Functions to update:

• propagate_organization_community_change/2 (lines 905-934)
• propagate_organization_mikrotik_change/2 (lines 490-521)
• propagate_organization_snmpv3_change/2 (lines 662-688)

Pattern:

# BEFORE (inner join excludes site-less devices):
from(d in Device,
  join: s in assoc(d, :site),
  where: s.organization_id == ^organization_id,
  ...
)

# AFTER (includes site-less devices):
from(d in Device,
  left_join: s in assoc(d, :site),
  where: d.organization_id == ^organization_id,
  where: (is_nil(d.site_id)) or (is_nil(s.snmp_community) or s.snmp_community == ""),
  ...
)

---

Phase 10: Device Quota Validation

Update Quota Checks

File: lib/towerops/devices.ex (lines 727-756)

Change to use organization_id:

# BEFORE (derives org from site):
defp do_check_quota(changeset) do
  case Ecto.Changeset.fetch_change(changeset, :site_id) do
    {:ok, site_id} ->
      validate_device_quota(changeset, site_id)
    :error ->
      {:ok, changeset}
  end
end

defp validate_device_quota(changeset, site_id) do
  site = site_id |> Sites.get_site!() |> Repo.preload(:organization)
  organization = site.organization
  # ...
end

# AFTER (uses organization_id directly):
defp do_check_quota(changeset) do
  case Ecto.Changeset.fetch_change(changeset, :organization_id) do
    {:ok, organization_id} ->
      validate_device_quota(changeset, organization_id)
    :error ->
      {:ok, changeset}
  end
end

defp validate_device_quota(changeset, organization_id) do
  organization = Repo.get!(Organization, organization_id)
  # ...
end

---

Testing Strategy

Unit Tests

File: test/towerops/devices/credential_resolver_test.exs (new)

Test scenarios:

• 3-tier resolution (device with site): device → site → org → default
• 2-tier resolution (device without site): device → org → default
• All credential types (SNMP v2, SNMP v3, MikroTik)

File: test/towerops/devices_test.exs

Add tests for:

• get_snmp_config/1 with and without site
• resolve_agent_token_id/1 with and without site
• Query functions return site-less devices
• Propagation functions handle site-less devices

Integration Tests

File: test/towerops_web/controllers/api/v1/devices_controller_test.exs

Test scenarios:

• Create device without site_id → succeeds
• Create device with site_id from different org → fails
• Show/update/delete device without site → succeeds

File: test/towerops_web/live/device_live_test.exs

Test scenarios:

• Navigate to /devices/new without sites → no redirect
• Submit device form without site_id → succeeds
• Device appears in list view

Migration Tests

Verify:

• Migration 1 backfills all organization_id values correctly
• Check constraint prevents mismatched site/org
• Migration 2 rollback blocked when NULL site_id exists

---

Implementation Sequence

Day 1-2: Schema Foundation

1. Write and test migration 1 (add organization_id)
2. Update Device schema module
3. Write CredentialResolver module
4. Add unit tests for CredentialResolver

Day 3-4: Credential Resolution

1. Refactor SNMP credential resolution
2. Refactor MikroTik credential resolution
3. Refactor SNMPv3 credential resolution
4. Update agent assignment resolution
5. Add integration tests

Day 5-6: Query Updates

1. Update list_organization_devices and related functions
2. Update preload functions
3. Update propagation functions
4. Update quota validation
5. Add query tests

Day 7-8: Authorization & Forms

1. Update API controller authorization
2. Update agent channel authorization
3. Update device form (remove site wall, make optional)
4. Update API create endpoint
5. Add E2E tests

Day 9: Migration & Deployment

1. Write and test migration 2 (make site_id nullable)
2. Run migrations in staging
3. Manual testing with real devices
4. Deploy to production

Day 10: Validation

1. Monitor error rates
2. Test creating devices without sites
3. Verify existing devices still work
4. Performance testing

---

Critical Files Summary

Migrations (New)

• priv/repo/migrations/XXXXXX_add_organization_id_to_devices.exs
• priv/repo/migrations/XXXXXX_make_site_id_nullable_on_devices.exs

Schema & Context (Modified)

• lib/towerops/devices/device.ex - Add organization_id field, validation
• lib/towerops/devices/credential_resolver.ex - New module for safe resolution
• lib/towerops/devices.ex - 30+ functions need updates

Controllers & LiveViews (Modified)

• lib/towerops_web/controllers/api/v1/devices_controller.ex - Auth fixes
• lib/towerops_web/channels/agent_channel.ex - Auth fixes
• lib/towerops_web/live/device_live/form.ex - Remove site wall
• lib/towerops_web/live/device_live/form.html.heex - Optional site selector

Tests (New/Modified)

• test/towerops/devices/credential_resolver_test.exs - New unit tests
• test/towerops/devices_test.exs - Updated integration tests
• test/towerops_web/controllers/api/v1/devices_controller_test.exs - API tests
• test/towerops_web/live/device_live_test.exs - LiveView tests

---

Rollback Plan

If issues arise:

1. Revert application code (Git revert)
2. Keep migration 1 (organization_id column is additive, doesn't break anything)
3. Revert migration 2 (only if no NULL site_id values exist)
4. System continues working with 3-tier credential resolution only

---

Success Criteria

✅ New user registers → can add device immediately (no site required) ✅ Existing multi-site users see no change in behavior ✅ All credential resolution works (SNMP, SNMPv3, MikroTik) ✅ Agent polling continues for all devices ✅ API endpoints work for site-less devices ✅ No N+1 query regressions ✅ All tests passing (unit, integration, E2E) ✅ Zero downtime deployment

---

Future Enhancements (Post-MVP)

After core functionality is stable, consider:

• Progressive disclosure UI (use_sites toggle in org settings)
• Conditional navigation (hide "Sites" link when unused)
• Device grouping UI (flat vs grouped view)
• Auto-create default site on org creation
• Help text and onboarding education
• Mobile app updates
• Advanced site features (hierarchical sites, site tags, etc.)