Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions server/routes/event/utils.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
"""Utility functions for event routes."""

from sqlalchemy import func
from sqlalchemy.orm import Session
from server.db.models.users import UserGroups, Groups


def is_admin(user, db: Session) -> bool:
"""Check if the user is an admin."""
admin_group = db.query(Groups).filter(Groups.name == "admin").first()
"""Check if the user is an admin (case-insensitive group name match)."""
admin_group = db.query(Groups).filter(func.lower(Groups.name) == "admin").first()
if not admin_group:
return False

Expand Down
63 changes: 63 additions & 0 deletions server/routes/instrument/delete_instrument.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
"""Delete instrument endpoint."""

from fastapi import APIRouter, Depends
from sqlalchemy.orm import Session

from server.db.database import get_db
from server.db.models.instrument import Instrument, FootprintCCD
from server.db.models.pointing import Pointing
from server.schemas.instrument import DeleteInstrumentResponse
from server.auth.auth import get_current_user
from server.routes.event.utils import is_admin
from server.utils.error_handling import (
not_found_exception,
permission_exception,
conflict_exception,
)

router = APIRouter(tags=["instruments"])


@router.delete("/instruments/{instrument_id}", response_model=DeleteInstrumentResponse)
async def delete_instrument(
instrument_id: int,
db: Session = Depends(get_db),
user=Depends(get_current_user),
):
"""Delete an instrument together with its footprint CCDs.

Only the instrument's submitter or an admin may delete it. Deletion is
refused while any pointing still references the instrument, since those
observation records would otherwise point at a missing instrument.
"""
instrument = db.query(Instrument).filter(Instrument.id == instrument_id).first()
if not instrument:
raise not_found_exception(f"No instrument found with 'id': {instrument_id}")

if instrument.submitterid != user.id and not is_admin(user, db):
Comment thread
sfoale marked this conversation as resolved.
raise permission_exception(
"Error: Unauthorized. Unable to alter other user's records"
)

pointing_count = (
db.query(Pointing).filter(Pointing.instrumentid == instrument_id).count()
)
if pointing_count:
raise conflict_exception(
f"Instrument {instrument_id} is referenced by {pointing_count} "
"pointing(s) and cannot be deleted."
)

deleted_footprints = (
db.query(FootprintCCD)
.filter(FootprintCCD.instrumentid == instrument_id)
.delete()
)
db.delete(instrument)
db.commit()

return DeleteInstrumentResponse(
message=f"Successfully deleted instrument {instrument_id}",
deleted_id=instrument_id,
deleted_footprints=deleted_footprints,
)
2 changes: 2 additions & 0 deletions server/routes/instrument/router.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from .get_footprints import router as get_footprints_router
from .create_instrument import router as create_instrument_router
from .create_footprint import router as create_footprint_router
from .delete_instrument import router as delete_instrument_router

# Create the main router that includes all instrument routes
router = APIRouter(tags=["instruments"])
Expand All @@ -16,3 +17,4 @@
router.include_router(get_footprints_router)
router.include_router(create_instrument_router)
router.include_router(create_footprint_router)
router.include_router(delete_instrument_router)
10 changes: 10 additions & 0 deletions server/schemas/instrument.py
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,16 @@ class InstrumentCreateResponse(BaseModel):
)


class DeleteInstrumentResponse(BaseModel):
"""Response schema for instrument deletion."""

message: str = Field(..., description="Result message")
deleted_id: int = Field(..., description="ID of the deleted instrument")
deleted_footprints: int = Field(
..., description="Number of footprint CCD rows removed with the instrument"
)


class InstrumentUpdate(BaseModel):
"""Schema for updating an instrument."""

Expand Down
98 changes: 98 additions & 0 deletions tests/fastapi/test_instrument.py
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,104 @@ def test_footprint_data_format(self):
assert footprint["footprint"].startswith("POLYGON")


class TestInstrumentDelete:
"""Test suite for deleting instruments."""

admin_token = "test_token_admin_001"
user_token = "test_token_user_002"
scientist_token = "test_token_sci_003"

def get_url(self, endpoint):
"""Get full URL for an endpoint."""
return f"{API_BASE_URL}{API_V1_PREFIX}{endpoint}"

def _create_instrument(self, token, name):
"""Create a Circular instrument and return its id."""
response = requests.post(
self.get_url("/instruments"),
json={
"instrument_name": name,
"nickname": name[:20],
"instrument_type": InstrumentType.photometric.value,
"footprint_type": "Circular",
"unit": "deg",
"radius": 1.0,
},
headers={"api_token": token},
)
assert response.status_code == status.HTTP_200_OK
result = response.json()
assert result["success"] is True, f"Instrument creation failed: {result}"
return result["instrument"]["id"]

def test_delete_instrument_success(self):
"""Owner can delete their instrument and its footprints are removed."""
instrument_id = self._create_instrument(
self.admin_token, "Deletable Telescope"
)

response = requests.delete(
self.get_url(f"/instruments/{instrument_id}"),
headers={"api_token": self.admin_token},
)
assert response.status_code == status.HTTP_200_OK
body = response.json()
assert body["deleted_id"] == instrument_id
assert body["deleted_footprints"] >= 1

# It should no longer exist.
check = requests.get(
self.get_url(f"/instruments?id={instrument_id}"),
headers={"api_token": self.admin_token},
)
assert check.status_code == status.HTTP_200_OK
assert check.json() == []

def test_delete_instrument_not_found(self):
"""Deleting a non-existent instrument returns 404."""
response = requests.delete(
self.get_url("/instruments/999999"),
headers={"api_token": self.admin_token},
)
assert response.status_code == status.HTTP_404_NOT_FOUND

def test_delete_instrument_without_auth(self):
"""Deleting without authentication returns 401."""
response = requests.delete(self.get_url("/instruments/1"))
assert response.status_code == status.HTTP_401_UNAUTHORIZED

def test_delete_instrument_forbidden_for_non_owner(self):
"""A non-admin cannot delete another user's instrument."""
instrument_id = self._create_instrument(self.user_token, "User Owned Deletable")

response = requests.delete(
self.get_url(f"/instruments/{instrument_id}"),
headers={"api_token": self.scientist_token},
)
assert response.status_code == status.HTTP_403_FORBIDDEN

def test_admin_can_delete_others_instrument(self):
"""An admin can delete an instrument submitted by another user."""
instrument_id = self._create_instrument(self.user_token, "Admin Removable")

response = requests.delete(
self.get_url(f"/instruments/{instrument_id}"),
headers={"api_token": self.admin_token},
)
assert response.status_code == status.HTTP_200_OK
assert response.json()["deleted_id"] == instrument_id

def test_delete_instrument_referenced_by_pointing(self):
"""An instrument referenced by pointings cannot be deleted (409)."""
# Seeded instrument 1 is referenced by several pointings in test data.
response = requests.delete(
self.get_url("/instruments/1"),
headers={"api_token": self.admin_token},
)
assert response.status_code == status.HTTP_409_CONFLICT
assert "pointing" in response.json()["message"].lower()


class TestInstrumentAPIValidation:
"""Test validation of instrument API endpoints."""

Expand Down
Loading