Skip to content

[Feature] Add unified dashboards + folders (projects) - #150

Open
yotamloe wants to merge 25 commits into
masterfrom
feat/unified-dashboards
Open

[Feature] Add unified dashboards + folders (projects)#150
yotamloe wants to merge 25 commits into
masterfrom
feat/unified-dashboards

Conversation

@yotamloe

@yotamloe yotamloe commented Sep 16, 2025

Copy link
Copy Markdown
Contributor

Description

-Add support for unified dashboards + folders (projects)

What type of PR is this?

(check all applicable)

  • 🍕 Feature
  • 🐛 Bug Fix
  • 🧑‍💻 Code Refactor
  • 🔥 Performance Improvements
  • ✅ Test
  • 🤖 Build / CI
  • ⏩ Revert

Added tests?

  • 👍 yes
  • 🙅 no, because they aren't needed
  • 🙋 no, because I need help from somebody

Comment thread unified_dashboards/unified_dashboards_test.go
Comment thread unified_projects/unified_projects_test.go
yotamloe and others added 19 commits August 14, 2026 13:19
…ses-public contract

- CreateProject/UpdateProject now send flat JSON bodies ({"name": ...} /
  {"displayName": ..., "description": ...}) instead of a Perses
  Project{Kind,Metadata,Spec} envelope that the perses-public API never
  expected.
- UpdateProject takes a new UpdateProjectRequest{DisplayName, Description}
  and returns *ProjectSummary (was *Project).
- SearchProjects is now a GET with query/limit/page/sort encoded as query
  parameters (was a POST with a JSON body), and returns []ProjectSummary
  directly (was *SearchProjectsResponse wrapping items/total/page/limit).
- Deleted the now-unused Project, ProjectMetadata, ProjectSpec,
  ProjectDisplay, and SearchProjectsResponse types, and the validators
  written against them.
- Updated fixtures, unit tests (new update/search test files), integration
  tests, and README examples/function table to match.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Implement RenameProject operation with TDD:
- Add RenameProject method to rename dashboard folders
- Includes validation for folderId and newName parameters
- Supports both success (200) and error (404, 500) responses
- Unit tests with mocked HTTP endpoints
- Integration test with live API

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add comprehensive unit tests for GetProject and DeleteProject operations:
- GetProject: success, API failure, not found, and validation errors
- DeleteProject: success, API failure, not found, and validation errors
- Tests use mocked HTTP endpoints and fixtures
- Validates error messages and response data

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add MoveDashboard operation to relocate dashboards to different folders.
Includes types, validator, implementation, unit tests, and integration tests.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add the repo's newest integration-test convention (see unified_alerts) to
every TestIntegration* in unified_projects and unified_dashboards: skip
immediately when LOGZIO_API_TOKEN is unset. RenameProject and
MoveDashboard already had the gate from earlier tasks.

Also strip the create-dashboard test's leftover debug scaffolding
(fmt.Printf logging, a project-verification block, a manual 5x-retry
loop) now that CallLogzioApi's built-in not-found retry/backoff covers
the eventual-consistency case; its body now matches its get/update/delete
siblings.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Move setupUnifiedProjectsIntegrationTest() from unified_dashboards_create_integration_test.go to unified_dashboards_test.go as a shared test helper. Normalize sleep duration in delete_integration_test.go from 10s to 2s. Add documentation clarifying name-vs-id addressing asymmetry in unified_projects/README.md. Remove vestigial snapshots note from unified_dashboards/README.md.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ublic contract

Verified against api.logz.io on 2026-08-14 (the published API docs are wrong
in several places):
- project create/update bodies are Perses Project envelopes (flat bodies 400/500)
- all single-project ops address by id, not name
- list entries are {project, dashboards} pairs; search is POST returning
  {results, total, pagination}
- dashboard docs must be full Perses Dashboard envelopes; responses carry
  id/uid/projectId/version and numeric createdBy (dropped string fields that
  failed to unmarshal)
- response unmarshalling now guards against silent zero-fill (missing id/uid
  becomes an explicit error with the offending body)
- not-found unit tests pin the 'failed with missing' sentinel; request-body
  tests pin literal wire keys
- move endpoint not yet deployed on the gateway: integration test skips with
  a dated note
- all 12 integration tests pass against a live account; rename is a live
  server-side no-op (documented in README)
…minology

Addresses the re-review round (code-reviewer + silent-failure-hunter, both
critical-free):
- MoveDashboard response now guarded like its siblings (empty uid -> explicit
  error with body) — the one endpoint that cannot be live-verified yet
- collection unmarshal errors carry operation + body snippet
- dashboards request-body unit tests pin the literal {"doc": ...} wrapper key
  (projects send the bare envelope — asymmetry documented in the README)
- ProjectSummary.MetadataName() exposes the Perses identity so read-modify-
  write updates cannot silently rewrite it (covered live in the update IT,
  along with the omit-Description-clears-it round trip)
- ListProjects(true) embedded-dashboards mapping now live-verified in the
  dashboard create IT; ProjectDashboard.Uid dropped (never populated)
- delete/rename address by id (params, error strings, README aligned)
- move IT scenario kept ready behind the dated skip; CHANGELOG notes move is
  pending gateway rollout

Unit suites green; 12/12 integration tests pass against a live account.
@pincher95
pincher95 force-pushed the feat/unified-dashboards branch from 9cd7165 to 1b020de Compare August 14, 2026 22:08
Comment thread unified_dashboards/unified_dashboards_test.go Outdated
Comment thread unified_projects/unified_projects_test.go Outdated
Clears the Cycode SAST 'Unsanitized user input in file path' findings on the
two new fixture helpers; callers pass bare file names, Base pins the lookup
inside testdata/fixtures.
Comment thread unified_projects/unified_projects_test.go Outdated
Comment thread unified_dashboards/unified_dashboards_test.go Outdated
Root-caused against the backend handlers: the published docs describe request
shapes the server never reads, so three 'broken' endpoints actually work:
- rename: body field is newProjectName (docs say newName -> silent 200 no-op)
- search: POST {filter:{searchTerm,createdBy},pagination:{pageNumber,pageSize}}
  returning {results,total,pagination}; semantics are a dashboard search
  grouped by folder (all folders returned, matching dashboards nested)
- move: PUT /dashboards/move {dashboardId,oldProjectId,newProjectId} -> {id}
  (docs' POST {uid,targetFolderId} has no route, hence the 404)

Rename IT now hard-asserts the rename; move IT un-skipped and asserts
present-in-destination + gone-from-source. 13/13 integration tests pass
against a live account (no skips).
Removes the OS file-path sink entirely (embed.FS lookups cannot escape the
embedded tree), which is both cleaner and clears the Cycode SAST
'Unsanitized user input in file path' rule that does not recognize
filepath.Base as a sanitizer.
…(review round 3)

- search fixture's pagination stanza was stale (page/limit from before the
  schema rewire); recaptured as pageNumber/pageSize and asserted in both the
  unit test and, live, in the integration test (server echo verified)
- pin filter.createdBy wire key; coherent search fixture (nested matching
  dashboard, total=1) with decode assertions
- create-dashboard IT: found-flag on the embedded-list verify + assert the
  embedded dashboard Id equals the addressable uid (live-verified)
- MetadataName(): unit-pinned incl. nil-safety; godoc documents the empty
  sentinel; update IT's spec/display walk asserts instead of comma-ok
- move IT cleanup logs both folder errors; SearchProjects godoc matches the
  grouped-dashboard-search semantics; README rename/search bullets split

13/13 integration tests pass live; unit suites green.
Comment thread unified_dashboards/unified_dashboards_test.go
Comment thread unified_projects/unified_projects_test.go
@pincher95
pincher95 requested a review from 8naama August 18, 2026 14:49

@8naama 8naama left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work! Left small comments

Comment on lines +11 to +14
dashboardsListEndpoint = "%s/perses-public/api/v1/dashboards"
dashboardByUidEndpoint = "%s/perses-public/api/v1/projects/%s/dashboards/%s"
dashboardsCreateEndpoint = "%s/perses-public/api/v1/projects/%s/dashboards"
dashboardsMoveEndpoint = "%s/perses-public/api/v1/dashboards/move"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing some endpoint that might be handy when working on the provider:

  • GET /projects/<projectId>/dashboards - per-project list
  • POST /dashboards/search - search for datasource

worth to verify with app team if there are others that may be useful

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both exist — added, with unit + live integration tests.

  • GET /projects/{projectId}/dashboardsListFolderDashboards(folderId). One gotcha worth knowing: an id matching no folder answers 200 [], identical to an existing empty folder, so it cannot be used to detect a missing folder.
  • POST /dashboards/searchSearchDashboards(req), returning flat dashboards as {results, total, pagination}.

The search one is a straight correction on my side: I removed a SearchDashboards earlier in this PR because the published docs do not list the route. The docs are wrong — it is live on /perses-public, and I had also been probing it on the old /perses gateway at the time. Probed 2026-08-20:

  • filter.searchTerm genuinely filters (exact term → 1 result, nonsense term → 0). That is a real difference from POST /projects/search, which returns every folder regardless of the term.
  • pagination.pageNumber/pageSize are honoured and echoed; total is the unpaginated match count. Default page size is 20.
  • GET /dashboards/search → 404, so POST only.
  • filter.createdBy is accepted but ignored — a bogus account id still returns everything. Mapped for symmetry with the projects filter, documented as non-functional.

The app-team check for further endpoints is still open — I only probed the two you listed plus the routes this package already uses.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Following up on the "search for datasource" part specifically — that endpoint cannot do it.

I created a dashboard whose display name, panel name, datasource reference and PromQL query each carried a distinctive token, then searched for each in turn:

searchTerm = datasource name  zzdatasource132334      -> total=0   matched: False
searchTerm = panel name       panel one               -> total=0   matched: False
searchTerm = display name     Totally Unrelated       -> total=0   matched: False
searchTerm = promql query     up                      -> total=0   matched: False
searchTerm = metadata name    ds-20260820132334-d     -> total=1   matched: True

filter.searchTerm matches the Perses document's metadata.name and nothing else — it is an identity lookup, not a full-text search over the document. Nothing on this gateway finds dashboards by the datasource they reference; you would have to list and walk Doc client-side.

Documented in the godoc and the README, and the search IT now gives the dashboard a display name distinct from its metadata.name and asserts the display name matches nothing, so this cannot quietly change meaning later.

Pushed as 5e0f4b7. If finding dashboards by datasource is the actual need, that is worth raising with the app team as a gap rather than something the client can work around cheaply.

return nil, fmt.Errorf("API token not defined")
}
if len(baseUrl) == 0 {
return nil, fmt.Errorf("Base URL not defined")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

previous PR lowercased such errors, worth to keep coherent

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed — base URL not defined in both packages, and the tests that pin the string.

Left API token not defined capitalised, which is what master does post-#163 (initialism, so ST1005 allows it). Also picked up the rest of that PR's modernization while here: interface{}any across both packages, since master has none left.

Comment thread unified_dashboards/README.md Outdated
### Response Types

- `Dashboard` — `Id`, `Uid` (the stable identifier), `Name`, `ProjectId`, `Doc`, `Version`, `CreatedAt`, `UpdatedAt`, `IsPrivate`
- `MoveDashboardResponse` — `Id` (the moved dashboard's uid)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the comment is a bit confusing, the dashboard object has both a id and uid fields; so does the returned id actually the uid or the new id following the change?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The doubt was well placed — that line was wrong, and so was the code under it.

Re-probed live with a dashboard at version 2: MoveDashboardResponse's id is the version-row id, not the uid. The reason it read as a uid is that the server creates a dashboard with id == uid == name; the three only diverge after the first update, and everything in this PR had only ever been checked against freshly created dashboards.

The README now carries an Identifiers section instead of that one-liner: which field is which, what each is for, that MoveDashboardResponse.Id is a version-row id, that unified_projects.ProjectDashboard exposes both, and the version-1 coincidence that hides the difference in the first place.

}

type MoveDashboardResponse struct {
Id string `json:"id"` // the moved dashboard's uid

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure the comment is correct here, if it's the dashboard uid or it's new id

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct to doubt it — the comment was wrong. MoveDashboardResponse.Id is the moved dashboard's current version-row id, not its uid.

Probe against api.logz.io, 2026-08-20:

v2: rowId=c26f75a3-…  uid=fc82f90d-…
PUT /dashboards/move -> 200 {"id": "c26f75a3-…"}
  move.id == uid   ? False
  move.id == rowId ? True

Against a version-1 dashboard the same call returns a value equal to the uid — but only because a new dashboard has id == uid == name. That is exactly why the old integration test passed: it created a dashboard and moved it immediately, so assert.Equal(created.Uid, moved.Id) held for either field and proved nothing.

Fixed both sides: the field is documented for what it actually is (an acknowledgement, not a handle — keep using the uid you passed as DashboardId), and the move IT now updates the dashboard first so the row id forks off the uid, then asserts moved.Id == updated.Id and moved.Id != created.Uid.


// ProjectDashboard is the dashboard payload embedded in project list/search
// responses. Id is the dashboard's addressable identifier.
type ProjectDashboard struct {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does the API return uid here too? if so, worth mapping it since it's the stable id

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it does — and mapping it turned out to matter more than cosmetics, because the Id that was there is a trap.

Probe against api.logz.io, 2026-08-20. Embedded payload keys:

["createdAt","createdBy","deletedAt","deletedBy","doc","id","isDeleted",
 "isPrivate","name","projectId","uid","updatedAt","updatedBy","version"]

On a version-2 dashboard the two identifiers diverge, and only one of them resolves:

embedded id  = a10ef4b5-…   GET /projects/{p}/dashboards/{id}  -> 404
embedded uid = 6701d8fd-…   GET /projects/{p}/dashboards/{uid} -> 200

So ProjectDashboard.Id is the version-row id, replaced on every update, and not addressable. A provider persisting it would have stored a value that breaks the first time a user edits the dashboard — silently, because it works fine until then.

What made this invisible: a brand-new dashboard is created with id == uid == name. The round-3 assertion item.Dashboards[0].Id == created.Uid ran against a freshly created dashboard, so it passed for either field. It was a coincidence, not a verification, and I read it as settling the question.

Changes:

  • ProjectDashboard now maps Uid alongside Id, with Uid documented as the handle and Id as the non-addressable version row.
  • Embedded-dashboard fixtures carry different uid and id values, so a dropped field cannot pass by matching.
  • Unit tests pin both fields; the create-dashboard IT updates the dashboard first, then asserts Uid resolves and Id returns "failed with missing unified dashboard".

…eview round 4)

Addresses @8naama's review. Two of the review comments turned out to be
pointing at real bugs, not just unclear wording — both had been "verified"
live against a freshly created dashboard, where the server sets
id == uid == name, so the assertions held for either field and proved nothing.
Re-probed against api.logz.io on 2026-08-20 with a version-2 dashboard, which
is what separates them.

Corrections:
- unified_projects.ProjectDashboard now maps `uid`. The embedded payload does
  carry it, and its `id` is the version-row id: passing that id to any
  folder-scoped dashboard route returns 404. Storing it in provider state
  would have broken the first time a user edited a dashboard.
- MoveDashboardResponse.Id is the current version-row id, not the uid. The
  previous comment said uid; the field is documented for what it is now.
- Both integration tests update a dashboard before asserting on identifiers,
  and the create IT additionally asserts that Uid resolves while Id 404s, so
  the version-1 coincidence can no longer hide a regression.
- Embedded-dashboard fixtures carry distinct uid/id values, unit tests pin both.

New endpoints (both live-verified; the earlier removal of a dashboards search
was based on the published docs, which do not list it):
- ListFolderDashboards — GET /projects/{projectId}/dashboards. Note it answers
  200 [] for an unknown folder rather than 404.
- SearchDashboards — POST /dashboards/search, returning flat dashboards with
  {results, total, pagination}. Filter.SearchTerm genuinely filters here,
  unlike the projects search. GET on that route 404s; Filter.CreatedBy is
  accepted but ignored by the server.

Coherence with #163:
- "Base URL not defined" -> "base URL not defined" (master lowercased these;
  "API token" stays capitalised there as an initialism).
- interface{} -> any across both packages, matching master.

Unit suites green; 15/15 integration tests pass against a live account.
@pincher95
pincher95 requested a review from 8naama August 20, 2026 17:21
Probed with a dashboard whose display name, panel name, datasource reference
and PromQL query each carried a distinctive token: searching for any of them
returned zero results, and only metadata.name matched. So the endpoint is an
identity lookup, not a full-text search over the document — worth stating,
since @8naama's comment described it as "search for datasource", which it
cannot do.

The search integration test now gives the dashboard a display name distinct
from its metadata.name and asserts the display name matches nothing.
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.

3 participants