-
Notifications
You must be signed in to change notification settings - Fork 0
Feat/merge (resources) #16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
9e60ac6
86a0a9a
bc49d49
686a379
e881008
b17a6c3
7e22186
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| from stitch.api.errors import StitchAPIError | ||
|
|
||
|
|
||
| class ResourceNotFoundError(StitchAPIError): ... | ||
|
|
||
|
|
||
| class ResourceIntegrityError(StitchAPIError): ... | ||
|
|
||
|
|
||
| class InvalidActionError(StitchAPIError): ... |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| class StitchAPIError(Exception): ... |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,12 +1,17 @@ | ||||||
| import logging | ||||||
|
|
||||||
| from collections.abc import Sequence | ||||||
|
|
||||||
| from fastapi import APIRouter | ||||||
| from fastapi import APIRouter, HTTPException | ||||||
|
|
||||||
| from pydantic import BaseModel | ||||||
|
|
||||||
| from stitch.api.db import resource_actions | ||||||
| from stitch.api.db.config import UnitOfWorkDep | ||||||
| from stitch.api.auth import CurrentUser | ||||||
| from stitch.api.entities import CreateResource, Resource | ||||||
|
|
||||||
| logger = logging.getLogger(__name__) | ||||||
|
|
||||||
| router = APIRouter( | ||||||
| prefix="/resources", | ||||||
|
|
@@ -33,3 +38,42 @@ async def create_resource( | |||||
| return await resource_actions.create( | ||||||
| session=uow.session, user=user, resource=resource_in | ||||||
| ) | ||||||
|
|
||||||
|
|
||||||
| class MergeRequest(BaseModel): | ||||||
| resource_ids: list[int] | ||||||
|
Comment on lines
+43
to
+44
|
||||||
|
|
||||||
|
|
||||||
| @router.post("/merge", response_model=Resource) | ||||||
| async def merge_resources_endpoint( | ||||||
| *, uow: UnitOfWorkDep, user: CurrentUser, payload: MergeRequest | ||||||
| ) -> Resource: | ||||||
| """ | ||||||
| Merge multiple resources into one (STUB): | ||||||
| repoint resource_ids[1:] -> resource_ids[0] | ||||||
| """ | ||||||
| ids = payload.resource_ids | ||||||
| # preserve order but drop duplicates | ||||||
| unique_ids = list(dict.fromkeys(ids)) | ||||||
| if len(unique_ids) < 2: | ||||||
| raise HTTPException( | ||||||
| status_code=400, detail="Provide at least 2 unique resource IDs" | ||||||
AlexAxthelm marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
| ) | ||||||
|
|
||||||
| logger.info( | ||||||
| "Merge requested by user=%s for resource_ids=%s", | ||||||
| getattr(user, "sub", "<anon>"), | ||||||
|
||||||
| getattr(user, "sub", "<anon>"), | |
| user.sub, |
Copilot
AI
Feb 25, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logger.exception call already includes the full exception traceback and the exception message automatically. Including the exception in the log message string as well (with %s, exc) results in duplicate exception information in the logs. Consider removing exc from the message format and just using: logger.exception("Error while merging resources %s", unique_ids)
| logger.exception("Error while merging resources %s: %s", unique_ids, exc) | |
| logger.exception("Error while merging resources %s", unique_ids) |
Copilot
AI
Feb 25, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new merge endpoint lacks test coverage. The repository has comprehensive test infrastructure for resources endpoints (both unit and integration tests in tests/routers/), and all other endpoints have corresponding tests. Consider adding at least:
- Unit tests covering the validation logic (empty list, single ID, duplicate IDs)
- Unit tests for error handling (non-existent resource IDs, HTTPException propagation)
- Integration tests verifying the actual merge behavior (repointing, target resource returned)
Uh oh!
There was an error while loading. Please reload this page.