# 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`

```elixir
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`

```elixir
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**:
```elixir
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:

```elixir
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**:
```elixir
# 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**:
```elixir
# 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:

```elixir
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):
```elixir
# BEFORE:
if device.site.organization_id == organization_id do  # ⚠️ Crashes

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

**Update site verification** (line 260):
```elixir
# 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):
```elixir
# 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):
```elixir
# 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**:
```elixir
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):
```heex
<!-- 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**:
```elixir
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**:
```elixir
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**:
```elixir
# 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**:
```elixir
# 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.)