-- Migrates every remaining live RLS/trigger call site off
-- is_invoice_reviewer()/is_invoice_approver() (0008) onto has_permission()
-- (0015), so those two columns/functions become fully unreferenced and
-- safe to drop in a later migration (NOT this one — per 0014/0015's own
-- stated order of operations, the column drop is a separate follow-up
-- once nothing reads them, which this migration is what actually confirms).
--
-- Exhaustive grep of every migration turned up FIVE live call sites (not
-- just the two named "0009/0010" informally in conversation — that name
-- refers to the invoice-approval RLS/trigger era generally, not a literal
-- list). All five are rewritten here, same "don't leave half the surface
-- still on the old check" reasoning as 0015's own header comment:
--
--   1. invoices_select_reviewer_or_approver (0008)  -- is_invoice_reviewer() OR is_invoice_approver()
--   2. invoices_update_approver              (0009, supersedes 0008's version) -- is_invoice_approver()
--   3. invoices_restrict_content_edit_when_submitted() trigger fn (0008)       -- is_invoice_approver()
--   4. qpi_qualifications_{select,insert,update,delete}_approver (0008, x4)    -- is_invoice_approver()
--
-- (0010's own trigger, invoices_restrict_owner_status_transitions, does
-- NOT call either function -- checked, it only compares agent_id/status --
-- so there is nothing to migrate there; it needs no change.)
--
-- Mapping used throughout:
--   is_invoice_reviewer() OR is_invoice_approver()  ->  has_permission('invoicing')
--     (both original flags set can_view=true per 0014's data migration,
--     so plain has_permission('invoicing') — need_write defaults false —
--     is the correct union of former reviewers+approvers for view access.)
--   is_invoice_approver()  ->  has_permission('invoicing', need_write => true)
--     (approver-only powers: approve/reject/pay, and all qpi_qualifications
--     access including its SELECT, matching legacy's approver-only comment
--     on that column in 0008.)
--
-- ============================================================
-- FIX, folded in per explicit go-ahead: has_permission()'s own SQL (0015,
-- extended 0016) never actually checked can_view for the need_write=false
-- path -- it treated "a permissions row exists at all" as sufficient,
-- ignoring the column entirely. A user explicitly set to can_view=false
-- (e.g. via /settings/users unchecking View, which upserts false/false
-- rather than deleting the row) would still have passed
-- has_permission('invoicing') -- which this migration's own item 1
-- (invoices_select_reviewer_or_approver) now depends on for view access,
-- so it needed closing here rather than shipped still-broken.
--
-- Fix: require can_view in the permissions-row branch, unconditionally
-- (not just when need_write). Safe for the need_write=true path too --
-- the write_implies_view check constraint (0014) already guarantees
-- can_write=true rows always have can_view=true, so this is a no-op
-- there and only changes behavior for the previously-buggy
-- can_view=false case. The is_admin bypass and is_broad_reviewer clause
-- (0016) are untouched.
-- ============================================================

create or replace function public.has_permission(feature text, need_write boolean default false)
returns boolean
language sql
security definer
stable
as $$
  select exists (
    select 1 from public.permissions
    where user_id = auth.uid()
      and feature_key = feature
      and can_view
      and (not need_write or can_write)
  )
  or exists (
    select 1 from public.profiles where id = auth.uid() and is_admin = true
  )
  or (
    not need_write
    and feature <> 'invoicing'
    and exists (
      select 1 from public.profiles where id = auth.uid() and is_broad_reviewer = true
    )
  );
$$;

grant execute on function public.has_permission(text, boolean) to authenticated;

-- ============================================================
-- 1. invoices_select_reviewer_or_approver
-- ============================================================

drop policy if exists "invoices_select_reviewer_or_approver" on public.invoices;
create policy "invoices_select_reviewer_or_approver"
on public.invoices
for select
to authenticated
using (public.has_permission('invoicing'));

-- ============================================================
-- 2. invoices_update_approver — self-approval exclusion (0009) preserved
-- verbatim; only the is_invoice_approver() check is swapped.
-- ============================================================

drop policy if exists "invoices_update_approver" on public.invoices;
create policy "invoices_update_approver"
on public.invoices
for update
to authenticated
using (
  public.has_permission('invoicing', need_write => true)
  and agent_id is distinct from public.current_agent_id()
)
with check (
  public.has_permission('invoicing', need_write => true)
  and agent_id is distinct from public.current_agent_id()
);

-- ============================================================
-- 3. invoices_restrict_content_edit_when_submitted() — trigger function
-- body only; the trigger itself (trg_invoices_restrict_content_edit_when_submitted,
-- from 0008) already points at this function by name and needs no change.
-- ============================================================

create or replace function public.invoices_restrict_content_edit_when_submitted()
returns trigger language plpgsql as $$
begin
  if not public.has_permission('invoicing', need_write => true)
     and old.status = 'submitted'
     and (
       new.items is distinct from old.items
       or new.subtotal is distinct from old.subtotal
       or new.tax is distinct from old.tax
       or new.total is distinct from old.total
       or new.monthly_base is distinct from old.monthly_base
       or new.rate_snapshot is distinct from old.rate_snapshot
       or new.weeks is distinct from old.weeks
     ) then
    raise exception 'Cannot edit invoice content while awaiting review — retract to draft first';
  end if;
  return new;
end;
$$;

-- ============================================================
-- 4. qpi_qualifications — all four policies were is_invoice_approver()
-- (including SELECT: viewing qualifications was already approver-only in
-- the legacy source, not reviewer-level, per 0008's own column comment).
-- ============================================================

drop policy if exists "qpi_qualifications_select_approver" on public.qpi_qualifications;
create policy "qpi_qualifications_select_approver"
on public.qpi_qualifications
for select
to authenticated
using (public.has_permission('invoicing', need_write => true));

drop policy if exists "qpi_qualifications_insert_approver" on public.qpi_qualifications;
create policy "qpi_qualifications_insert_approver"
on public.qpi_qualifications
for insert
to authenticated
with check (public.has_permission('invoicing', need_write => true));

drop policy if exists "qpi_qualifications_update_approver" on public.qpi_qualifications;
create policy "qpi_qualifications_update_approver"
on public.qpi_qualifications
for update
to authenticated
using (public.has_permission('invoicing', need_write => true))
with check (public.has_permission('invoicing', need_write => true));

drop policy if exists "qpi_qualifications_delete_approver" on public.qpi_qualifications;
create policy "qpi_qualifications_delete_approver"
on public.qpi_qualifications
for delete
to authenticated
using (public.has_permission('invoicing', need_write => true));

-- ============================================================
-- NOT done here (separate future migration, per 0014/0015's stated order):
--   - Drop profiles.is_invoice_reviewer / profiles.is_invoice_approver
--   - Drop public.is_invoice_reviewer() / public.is_invoice_approver()
-- After this migration, both functions are fully unreferenced by any
-- policy or trigger (verify with the query in the chat writeup before
-- that follow-up migration, not just this comment's say-so).
-- ============================================================