Skip to content

feat: add CRUD REST API for saved searches (/api/v2/saved-searches)#2544

Open
ankitcharolia wants to merge 4 commits into
hyperdxio:mainfrom
ankitcharolia:feat/saved-searches-external-api
Open

feat: add CRUD REST API for saved searches (/api/v2/saved-searches)#2544
ankitcharolia wants to merge 4 commits into
hyperdxio:mainfrom
ankitcharolia:feat/saved-searches-external-api

Conversation

@ankitcharolia

Copy link
Copy Markdown

Summary

Adds full CRUD endpoints for saved searches to the external API (/api/v2/saved-searches), enabling infrastructure-as-code tooling to provision saved searches programmatically.

The SavedSearch mongoose model and toExternalJSON() method already existed — only the router and its registration were missing.

New endpoints:

Method Path Description
GET /api/v2/saved-searches List all saved searches for the team
POST /api/v2/saved-searches Create a saved search
GET /api/v2/saved-searches/:id Get a saved search by ID
PUT /api/v2/saved-searches/:id Update a saved search
DELETE /api/v2/saved-searches/:id Delete a saved search

Screenshots or video

Before After
/api/v2/saved-searches → 404 GET /api/v2/saved-searches{ data: [...] }

How to test on Vercel preview

Steps:

  1. Obtain a Bearer token from Team Settings → API Keys
  2. curl -H 'Authorization: Bearer <token>' <preview-url>/api/v2/saved-searches — should return { "data": [] }
  3. POST a new saved search with name, sourceId, and optional where/whereLanguage/select/orderBy/tags
  4. Verify GET/PUT/DELETE round-trips work correctly
  5. Confirm that saved searches created here appear in the UI and can be referenced in alerts via savedSearchId

References

Adds GET/POST /api/v2/saved-searches and GET/PUT/DELETE
/api/v2/saved-searches/:id to the external API.

The SavedSearch mongoose model and toExternalJSON() already exist;
this PR adds only the missing router and index registration.

Closes hyperdxio#2543
@vercel

vercel Bot commented Jun 29, 2026

Copy link
Copy Markdown

@ankitcharolia is attempting to deploy a commit to the HyperDX Team on Vercel.

A member of the Team first needs to authorize it.

@changeset-bot

changeset-bot Bot commented Jun 29, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 2498903

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@greptile-apps

greptile-apps Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds saved-search CRUD support to the external API. The main changes are:

  • New /api/v2/saved-searches router for list, create, read, update, and delete.
  • Router registration behind the existing external API auth and rate limits.
  • Integration tests for CRUD behavior, authentication, team scoping, and source validation.

Confidence Score: 5/5

This looks safe to merge.

  • No blocking issues found in the changed code.

Important Files Changed

Filename Overview
packages/api/src/routers/external-api/v2/savedSearches.ts Adds the saved-search CRUD handlers with team-scoped source validation before writes.
packages/api/src/routers/external-api/v2/index.ts Mounts the saved-searches router under the external API v2 route tree.
packages/api/src/routers/external-api/tests/savedSearches.test.ts Adds integration coverage for saved-search CRUD, auth checks, team scoping, and invalid source references.

Reviews (4): Last reviewed commit: "Merge branch 'main' into feat/saved-sear..." | Re-trigger Greptile

Comment thread packages/api/src/routers/external-api/v2/savedSearches.ts
Comment thread packages/api/src/routers/external-api/v2/savedSearches.ts
ankitcharolia and others added 3 commits June 29, 2026 23:42
Adds a Source.findOne({ _id: sourceId, team: teamId }) check to both
POST and PUT handlers, returning 400 if the source does not exist or
belongs to another team. Also adds test coverage for cross-team and
non-existent sourceId scenarios.

Addresses greptile review comments on unchecked source ownership.
@github-actions

github-actions Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Deep Review

Scope: New external CRUD API for saved searches — packages/api/src/routers/external-api/v2/savedSearches.ts (new), its registration in v2/index.ts, and integration tests. Diffed against 727d327.

Intent: Add team-scoped CRUD endpoints (/api/v2/saved-searches) for infra-as-code provisioning, following the connections.ts pattern. Authorization (team-scoping, sourceId ownership) is well-implemented and the security reviewer found no exploitable vulnerabilities. The findings below concern data-integrity divergence from the existing internal delete/create paths, contract/doc gaps, and test coverage.

🔴 P0/P1 — must fix

  • packages/api/src/routers/external-api/v2/savedSearches.ts:250 — the DELETE handler calls SavedSearch.findOneAndDelete directly and skips the alert cascade that the established delete path (controllers/savedSearch.ts:71deleteSavedSearchAlertsAlert.deleteMany({ savedSearch, team })) performs, so deleting a saved search via the external API orphans every Alert that references it, which tasks/checkAlerts/index.ts then evaluates against a now-missing search.
    • Fix: Route the handler through the existing deleteSavedSearch controller so dependent alerts are removed with the search.
    • agent-native, maintainability

🟡 P2 — recommended

  • packages/api/src/routers/external-api/v2/savedSearches.ts:13where, whereLanguage, select, and tags are .optional() here, but the canonical SavedSearchSchema marks them required and the internal create path enforces it via SavedSearchSchema.omit({ id: true }), so a POST { name, sourceId } persists a document with undefined fields that checkAlerts/index.ts:156-157 later reads directly into query construction.
    • Fix: Require the same fields the internal path guarantees, or apply matching defaults, so externally-created searches satisfy the shared invariant.
    • adversarial
  • packages/api/src/routers/external-api/v2/savedSearches.ts:210 — the PUT handler spreads all optional body fields into $set, so an omitted field is neither a documented full-replace (cleared) nor an intentional merge; the behavior depends on driver/ODM undefined handling and is undocumented and untested, which is exactly the drift an IaC client cannot tolerate.
    • Fix: Decide the semantics, document them in the OpenAPI description, and add a test that omits a previously-set field to lock the behavior in.
    • correctness, adversarial, api-contract, testing, kieran-typescript
  • packages/api/src/routers/external-api/v2/savedSearches.ts:104 — both POST and PUT OpenAPI blocks reference $ref: '#/components/schemas/CreateSavedSearchRequest', a component that is defined nowhere in the repo, producing a dangling reference in the generated spec that breaks Swagger UI rendering and SDK codegen for these endpoints.
    • Fix: Define a components: schemas: CreateSavedSearchRequest block (and a response schema) inline as every sibling router does, or inline the request schema.
    • api-contract, project-standards, maintainability
  • packages/api/src/routers/external-api/v2/savedSearches.ts:140 — the inline create/update path never sets createdBy/updatedBy, unlike controllers/savedSearch.ts which persists them from the user id, so searches provisioned via the external API silently lack the audit metadata the internal API guarantees on the shared model.
    • Fix: Set createdBy/updatedBy from req.user, ideally by reusing the existing controller functions.
    • maintainability
  • packages/api/src/routers/external-api/v2/savedSearches.ts:1 — no .changeset/ entry accompanies this new user-facing @hyperdx/api endpoint surface, which AGENTS.md rule 5 requires before pushing.
    • Fix: Add a changeset for @hyperdx/api describing the new saved-searches CRUD endpoints.
    • project-standards
  • packages/api/src/routers/external-api/__tests__/savedSearches.test.ts:217 — tests cover the happy path and team-scoping but omit new-behavior coverage for zod boundary limits, whereLanguage enum rejection, invalid-ObjectId 400 on PUT/DELETE, and the PUT-omit semantics above.
    • Fix: Add tests for max-length/enum rejection, malformed-id 400 on PUT/DELETE, and the field-omission behavior on update.
    • testing, correctness
🔵 P3 nitpicks (6)
  • packages/api/src/routers/external-api/v2/savedSearches.ts:126 — the sourceId not found response uses { error: ... }, while search.ts, dashboards.ts, and team.ts (and this router's own validation failures) use { message: ... }.
    • Fix: Return { message: 'sourceId not found' } at lines 126 and 192 for a consistent error contract.
    • api-contract, project-standards
  • packages/api/src/routers/external-api/v2/savedSearches.ts:13whereLanguage is hand-rolled as z.enum(['lucene', 'sql']) instead of the shared SearchConditionTrimmedLanguageSchema, risking drift and dropping promql support that other paths permit.
    • Fix: Import and reuse the shared enum schema.
  • packages/api/src/routers/external-api/v2/savedSearches.ts:124 — the four-line sourceId ownership check is duplicated verbatim in POST and PUT.
    • Fix: Extract a small assertOwnedSource(teamId, sourceId) helper.
  • packages/api/src/utils/swagger.ts:19 — the Saved Searches tag used in the annotations is not registered in the central swagger tags list, unlike sibling groups.
    • Fix: Add a Saved Searches tag entry with a description.
  • packages/api/src/routers/external-api/__tests__/savedSearches.test.ts:300 — the delete test does not assert the {} response body documented as the DELETE contract.
    • Fix: Add expect(res.body).toEqual({}) to the delete test.
  • packages/api/src/routers/external-api/v2/savedSearches.ts:13 — the model supports filters, but the external bodySchema omits it, so toExternalJSON always returns filters: undefined for API-created searches with no way to set it.
    • Fix: Accept filters in the body schema or document the omission as intentional.

Reviewers (11): correctness, security, adversarial, testing, maintainability, project-standards, api-contract, reliability, kieran-typescript, agent-native, learnings-researcher.

Testing gaps:

  • The 403 no-team path (req.user?.team == null) is unexercised across all five handlers (systemic across v2 routers).
  • No test simulates a downstream DB failure to confirm the next(e) → error-handler path returns a clean 500.
  • No CI check validates the generated OpenAPI spec, so the dangling $ref would not be caught automatically.

Pre-existing / out of scope (not counted in verdict): req.body is typed any after validateRequest (systemic across routers); a sourceId check-then-write TOCTOU exists but matches existing patterns and has no demonstrated failure path.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: add CRUD REST API for saved searches (/api/v2/saved-searches)

1 participant