Restful DELETE and PUT for pointings#118
Conversation
Add delete selected button to the search/pointings page.
There was a problem hiding this comment.
Pull request overview
Adds RESTful single-pointing update (PUT) and bulk delete (DELETE) capabilities for pointings, and exposes bulk deletion in the “Search Pointings” UI when viewing “my pointings only” (Issue #54).
Changes:
- Backend: added
DELETE /api/v1/pointings(bulk delete by IDs) andPUT /api/v1/pointings/{pointing_id}(field updates on an existing pointing). - Auth/refactor: centralized admin-group membership check in
auth.is_admin_user()and reused it in event utilities. - Frontend: enabled selecting pointings in the table for “my pointings only” results and added a “Delete selected” action; added API client/types for delete + update.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/fastapi/test_pointing.py | Adds integration tests for the new PUT and DELETE pointing endpoints. |
| server/schemas/pointing.py | Introduces PointingDeleteRequest schema for bulk delete by IDs. |
| server/routes/pointing/router.py | Registers new delete_pointings and put_pointing routers. |
| server/routes/pointing/put_pointing.py | Implements PUT /pointings/{pointing_id} update endpoint. |
| server/routes/pointing/delete_pointings.py | Implements DELETE /pointings bulk delete endpoint (also deletes PointingEvent). |
| server/routes/event/utils.py | Switches admin check implementation to shared is_admin_user(). |
| server/auth/auth.py | Adds is_admin_user() helper and reuses it in verify_admin(). |
| frontend/src/routes/search/pointings/+page.svelte | Adds “Delete selected” button/flow and wiring to API. |
| frontend/src/lib/components/search/PointingsTable.svelte | Changes selection rules for pointings in the search table. |
| frontend/src/lib/api/types/pointing.types.ts | Adds PointingDeleteRequest type. |
| frontend/src/lib/api/services/pointing.service.ts | Adds deletePointings() and updatePointing() API calls. |
Comments suppressed due to low confidence (1)
server/routes/pointing/put_pointing.py:57
- An empty request body (or a body containing only non-updatable fields) still updates
dateupdatedand returns success, even though no meaningful change was applied. Consider rejecting no-op updates (e.g. require at least one updatable field orra+dec) to avoid confusing clients and unnecessary DB writes.
for field in update.model_dump(exclude_unset=True).keys() & _UPDATABLE_FIELDS:
setattr(pointing, field, getattr(update, field))
if update.ra is not None and update.dec is not None:
pointing.position = f"POINT({update.ra} {update.dec})"
pointing.dateupdated = datetime.now()
db.commit()
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Does this give the ability for a non admin user to delete their own pointing they submitted? I think we want to give both admin and the original submitter/whoever has the permission to submit for the group the ability to delete pointings. e.g. If we submitted something from LCO in error, someone from LCO should be able to clean it up. Is that possible here? (My initial read is no.) |
Yes to their own.
No this would be a larger change. There's currently no link between an instrument and groups/usergroups in the schema. Those tables exist but today only encode a single "ADMIN" group and 'gwtm' usergroup. We'd need to create meaningful groups (e.g. LCO) and associate users with them in the usergroups table before we could attempt this. |
Add restful DELETE and PUT endpoints for pointings (closes #54)
Add delete selected button to the search/pointings page.