diff --git a/server/routes/event/utils.py b/server/routes/event/utils.py index 51c88ebd..44857142 100644 --- a/server/routes/event/utils.py +++ b/server/routes/event/utils.py @@ -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 diff --git a/server/routes/instrument/delete_instrument.py b/server/routes/instrument/delete_instrument.py new file mode 100644 index 00000000..0f817e3f --- /dev/null +++ b/server/routes/instrument/delete_instrument.py @@ -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): + 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, + ) diff --git a/server/routes/instrument/router.py b/server/routes/instrument/router.py index 42f90536..7d17903e 100644 --- a/server/routes/instrument/router.py +++ b/server/routes/instrument/router.py @@ -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"]) @@ -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) diff --git a/server/schemas/instrument.py b/server/schemas/instrument.py index cc9be0a3..e25092a5 100644 --- a/server/schemas/instrument.py +++ b/server/schemas/instrument.py @@ -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.""" diff --git a/tests/fastapi/test_instrument.py b/tests/fastapi/test_instrument.py index 07393745..5d57a491 100644 --- a/tests/fastapi/test_instrument.py +++ b/tests/fastapi/test_instrument.py @@ -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."""