feat: Entity Panel templating - BED 9017 - #3095
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR adds restricted Markdown template rendering for node and relationship kind information. Services preserve fallback content and per-entry template errors, handlers return the pre-rendered values, access checks move into the service layer, and OpenAPI schemas document the response fields. ChangesKind-info Markdown rendering
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant GetNodeByID
participant Service.GetNode
participant renderNodeKindInfos
Client->>GetNodeByID: Request node details
GetNodeByID->>Service.GetNode: Fetch node and kind information
Service.GetNode->>renderNodeKindInfos: Render Markdown templates
renderNodeKindInfos-->>Service.GetNode: Rendered content or fallback with template_error
Service.GetNode-->>GetNodeByID: NodeView
GetNodeByID-->>Client: Node response
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
server/graphdb/internal/handlers/relationship_test.go (1)
63-70: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd response coverage for
template_error.The fixture verifies rendered content but does not verify
TemplateError. Add a case with one failed relationship template and assert that the response preserves its fallback content andtemplate_error.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/graphdb/internal/handlers/relationship_test.go` around lines 63 - 70, Add response coverage in the relationship fixture around the existing RenderedMarkdown case by including a relationship with a failed template, its fallback content, and the expected TemplateError value. Assert that the handler response preserves both the fallback rendered content and template_error for that relationship while retaining the existing successful-template coverage.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@server/graphdb/internal/services/template.go`:
- Around line 171-174: Update both WarnContext calls in the relationship-kind
markdown rendering flow to pass structured slog.String and slog.Any attributes
instead of alternating key-value arguments, preserving the existing info_key and
error values.
- Around line 68-97: Update the unsupportedFns list to use Sprig’s exact randInt
identifier instead of randint, and add a regression test verifying that parsing
a template containing {{ randInt 1 2 }} fails.
---
Nitpick comments:
In `@server/graphdb/internal/handlers/relationship_test.go`:
- Around line 63-70: Add response coverage in the relationship fixture around
the existing RenderedMarkdown case by including a relationship with a failed
template, its fallback content, and the expected TemplateError value. Assert
that the handler response preserves both the fallback rendered content and
template_error for that relationship while retaining the existing
successful-template coverage.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 4f1d0075-6f30-4b0a-9a4d-24ea061533e5
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (15)
go.modpackages/go/openapi/doc/openapi.jsonpackages/go/openapi/src/paths/graph.nodes.id.yamlpackages/go/openapi/src/schemas/model.kind-info-markdown.yamlserver/graphdb/internal/handlers/node.goserver/graphdb/internal/handlers/node_test.goserver/graphdb/internal/handlers/relationship.goserver/graphdb/internal/handlers/relationship_test.goserver/graphdb/internal/services/node.goserver/graphdb/internal/services/node_test.goserver/graphdb/internal/services/relationship.goserver/graphdb/internal/services/relationship_test.goserver/graphdb/internal/services/services.goserver/graphdb/internal/services/template.goserver/graphdb/internal/services/template_test.go
There was a problem hiding this comment.
🧹 Nitpick comments (2)
server/graphdb/internal/handlers/relationship_test.go (2)
50-51: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueGroup the endpoint node declarations in a
var (...)block.Proposed change
- sourceNode := services.Node{ID: relationship.SourceNodeID} - targetNode := services.Node{ID: relationship.TargetNodeID} + var ( + sourceNode = services.Node{ID: relationship.SourceNodeID} + targetNode = services.Node{ID: relationship.TargetNodeID} + )As per coding guidelines, “When possible, group variable initializations in a
var (...)block and hoist them to the top of the function.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/graphdb/internal/handlers/relationship_test.go` around lines 50 - 51, Group the sourceNode and targetNode declarations at the top of the containing test function in a single var (...) block, preserving their existing services.Node initializers and identifiers.Source: Coding guidelines
77-85: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a response test for
template_error.Add a
KindInfofixture withRenderedMarkdownset to the original template andTemplateErrorset. Assert that the HTTP response contains both fallback Markdown content andtemplate_error. The current test only covers successful rendering.Based on PR objectives, failed rendering must return the original template and a
template_errorfield.Also applies to: 128-147
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/graphdb/internal/handlers/relationship_test.go` around lines 77 - 85, Add a failed-rendering response case in the relationship handler test alongside the existing successful KindInfo fixture: create a fixture with RenderedMarkdown containing the original template and TemplateError set, then assert the HTTP response includes both the fallback Markdown content and the template_error field. Keep the existing successful-rendering assertions intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@server/graphdb/internal/handlers/relationship_test.go`:
- Around line 50-51: Group the sourceNode and targetNode declarations at the top
of the containing test function in a single var (...) block, preserving their
existing services.Node initializers and identifiers.
- Around line 77-85: Add a failed-rendering response case in the relationship
handler test alongside the existing successful KindInfo fixture: create a
fixture with RenderedMarkdown containing the original template and TemplateError
set, then assert the HTTP response includes both the fallback Markdown content
and the template_error field. Keep the existing successful-rendering assertions
intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 50c482c5-0d34-4b31-9d22-dbf8bc6cf33f
📒 Files selected for processing (2)
server/graphdb/internal/handlers/relationship.goserver/graphdb/internal/handlers/relationship_test.go
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
server/graphdb/internal/services/relationship.go (1)
68-77: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHoist the endpoint variables.
Define
sourceNodeandtargetNodein the existingvar (...)block. Assign them witherrduring endpoint retrieval.As per coding guidelines, “When possible, group variable initializations in a
var (...)block and hoist them to the top of the function.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/graphdb/internal/services/relationship.go` around lines 68 - 77, Update the relationship endpoint retrieval in the surrounding function by declaring sourceNode and targetNode in the existing var (...) block, then assign them using the shared err variable when calling GetNode. Preserve the current source- and target-specific wrapped error messages and return behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@server/graphdb/internal/services/relationship.go`:
- Around line 68-77: Update the relationship retrieval function around
GetKindByName so both source and target nodes are resolved and ETAC-authorized
before handling the ErrKindNotFound early return. Preserve the existing endpoint
error wrapping and prevent graph-only relationships from exposing IDs or
properties without authorization. Add a regression test covering denied endpoint
access through the ErrKindNotFound branch.
---
Nitpick comments:
In `@server/graphdb/internal/services/relationship.go`:
- Around line 68-77: Update the relationship endpoint retrieval in the
surrounding function by declaring sourceNode and targetNode in the existing var
(...) block, then assign them using the shared err variable when calling
GetNode. Preserve the current source- and target-specific wrapped error messages
and return behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 6d1c5a24-8067-4a33-9dfb-7f15f80e03de
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (18)
go.modpackages/go/openapi/doc/openapi.jsonserver/graphdb/graphdb.goserver/graphdb/graphdb_e2e_test.goserver/graphdb/internal/handlers/handlers.goserver/graphdb/internal/handlers/mocks/nodeauthorizer.goserver/graphdb/internal/handlers/node.goserver/graphdb/internal/handlers/node_test.goserver/graphdb/internal/handlers/relationship.goserver/graphdb/internal/handlers/relationship_test.goserver/graphdb/internal/routes/routes_test.goserver/graphdb/internal/services/access_checker_test.goserver/graphdb/internal/services/mocks/nodeaccesschecker.goserver/graphdb/internal/services/node.goserver/graphdb/internal/services/node_test.goserver/graphdb/internal/services/relationship.goserver/graphdb/internal/services/relationship_test.goserver/graphdb/internal/services/services.go
💤 Files with no reviewable changes (1)
- server/graphdb/internal/handlers/mocks/nodeauthorizer.go
🚧 Files skipped from review as they are similar to previous changes (3)
- go.mod
- server/graphdb/internal/services/node_test.go
- packages/go/openapi/doc/openapi.json
|
Is there anyway the Dog Park schema can be updated to include a template for one of the node or relationship kinds? |
LawsonWillard
left a comment
There was a problem hiding this comment.
Non blocking: may be worth an end 2 end test checking against an expected templated response
Otherwise looks good to me! 🚀
Description
spriglibrary for fancy templatinggraphdb/services, following new onion archapi/v2/nodes/{node_id}andapi/v2/relationships/{relationship_id}template_errorfield, such as:Motivation and Context
Resolves BED-9017
We want users to be able to define "dynamic" entity panels. For example, a node entity panel should be able to render dynamic information about that node, and the same is true for a relationship.
We can achieve dynamic entity panels through "templating". This PR uses the
spriglibrary to implement templating on entity panels.How Has This Been Tested?
infoobject like this:Screenshots (optional):
The original ticket provided an example template for a node panel and an edge panel. Here are those dynamic templates rendered in the application:
for a node:

for an edge:

Types of changes
Checklist:
Summary by CodeRabbit
New Features
Bug Fixes
Documentation