feat(automations): filter the automations list by creator - #539
henriquehirako wants to merge 5 commits into
Conversation
Add an optional created_by query param to GET /api/automation/v1: `me` keeps the caller's automations and `others` the rest of the org's. The filter applies before counting, so `total` and offset paging cover only the matching automations. Without the param the list is unchanged; any other value is a 422. Lets Agent Canvas filter "Created by" on the server instead of over the page it has loaded.
Seed explicit created_at values so the creator-filter tests check the newest-first order and that two pages hold no duplicates, and cover the unfiltered list with both creators. Note in the docstring that Git Sync imports belong to the admin who configured the sync.
|
Warning Your comment is too long (maximum is 65536 characters), so the coverage report was not added. See the job log for how to reduce it. |
Give the no-filter case its own test with a shared seed helper, parse each page once in the paging test, and drop the docstring line about Git Sync ownership, which this endpoint does not decide.
The created_by branches read plainly in the code, so the docstring goes back to the original line.
The teammate's automation is the newer of the two, not the older.
all-hands-bot
left a comment
There was a problem hiding this comment.
This review was posted by an AI agent (OpenHands).
Review: feat(automations): filter the automations list by creator
Scope — Correct repository. The change extends GET /api/automation/v1 in the automation service, which is the component this repo owns (list endpoint, org scope, paging). The linked product requests are ready-for-dev: OpenHands/OpenHands#17816 and OpenHands/extensions#708. No product/architecture decision is pending.
What I verified on head 4384439
created_byis typedLiteral["me", "others"] | None, so any other value is rejected with 422 by FastAPI validation, as documented and tested.me/othersaddAutomation.user_id == / != user.user_idto the base query, so the predicate runs inside the DB query, is applied before the count, and composes with the existingorg_id+deleted_at IS NULLscope and thecreated_at DESCordering.totalandlimit/offsettherefore cover only the matching set — the stated bug (client-side filtering of the newest 50) is fixed at the correct layer.- With no param the query is byte-for-byte the previous behavior.
Automation.user_idisnullable=Falseand carriesix_automations_user_id, so the new predicate cannot miss NULL rows and is index-backed.- Consumer integration: Canvas builds the request via
buildListQuery, which omits keys whose value isundefined;automations-list.tsxsetsserverCreatedBy = createdByFilter !== "all" ? createdByFilter : undefined. "Anyone" sends no param (unfiltered), "Me"/"Others" sendme/others— exactly the three accepted shapes. There is no path that sendsallto the service, so the strict 422 cannot fire in normal use. - CI on the exact head: 8 checks passed, 2 skipped; no failures.
Tests — _seed_automation/_seed_mine_teammate_and_other_org cover unfiltered, me, others, filtered paging with total counting only matches, newest-first order, and the 422. Other-org exclusion is implicitly asserted (total == 2). I could not execute the suite here because it requires a Docker-backed PostgreSQL container and Docker is unavailable in this sandbox; CI unit-tests is green on this head, and the new tests read as correct against the implementation.
Non-blocking note — With created_by=me, an automation whose user_id differs from the caller is invisible to a member; admins/owners likewise cannot find another member's automation under "Me". That matches the stated product intent (automations imported by Git Sync belong to the configured admin), so it is not a blocker.
No blocking bugs, security problems, or design flaws found.
✅ APPROVED
Why
Agent Canvas is adding a "Created by" filter (Anyone / Me / Others) to the Automations list (OpenHands/OpenHands#17814). The list endpoint has no creator filter, so Canvas can only filter the rows it has already loaded. The list loads 50 at a time, newest first. In an org with more than 50 automations, "Me" checks only the newest 50: if none of them are the caller's, the user is told they have no automations when they do.
Summary
GET /api/automation/v1takes an optionalcreated_byquery param:mekeeps the caller's automations (Automation.user_id == user.user_id),otherskeeps the rest of the caller's org.totalandlimit/offsetpaging cover only the matching automations. The org scope, the soft-delete filter, and the newest-first order do not change.mereturns all of them andothersnone.Consumer: OpenHands/OpenHands#17814 sends
created_byfrom the "Created by" filter. An automation service without this change ignores the unknown param, and Canvas keeps its client-side filter as the fallback.How to Test
uv run python -m pytest tests/test_router.py -k TestListAutomations. The new tests cover: no param lists every creator;meandothers; two filtered pages keep the newest-first order with no duplicates, andtotalcounts only the matches; an unknown value is a 422.OH_AUTOMATION_LOCAL_PATH=<this repo> npm run devin OpenHands/OpenHands). Local mode has one user, so give some automations anotheruser_idin the local SQLite DB. Results below.What I ran:
uv run pre-commit run --all-files: pass.uv run python -m pytest tests/: 1801 passed, 60 skipped.Local service with 5 of my automations and 2 with another
user_id:totalcreated_by=mecreated_by=otherscreated_by=me&limit=2, then&offset=2created_by=teamInput should be 'me' or 'others'Full stack: Agent Canvas (feat(automations): filter the automations dashboard by creator OpenHands#17814 on top of fix(automations): page the list by offset so Load more works past 100 OpenHands#17839) on this service, 60 automations, with mine the 5 oldest. With no filter, none of mine are on the first page. "Me" sent
created_by=meand showed all 5. "Others" showed 50, and "Load more" sentoffset=50&created_by=othersand showed 55.