feat(sdk): add EntityIdentifier convenience constructors#911
feat(sdk): add EntityIdentifier convenience constructors#911marythought merged 2 commits intomainfrom
Conversation
Add helper functions (forEmail, forClientId, forUserName, forToken, withRequestToken) that mirror the Go SDK's EntityIdentifier helpers. Reduces authorization call setup from ~15 lines of nested object literals to a single function call. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Mary Dickson <mary.dickson@virtru.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughA new authorization entity-identifier helper module adds five convenience constructors ( Changes
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
lib/tests/mocha/unit/entity-identifiers.spec.ts (1)
2-8: Prefer importing helpers from the SDK entrypoint in at least one test.Line 2–8 tests the internal module directly, so it won’t catch accidental regressions in the new top-level exports from
lib/src/index.ts. Consider importing from../../../src/index.js(or adding one small entrypoint-focused test).💡 Minimal test import adjustment
import { expect } from 'chai'; import { forEmail, forClientId, forUserName, forToken, withRequestToken, -} from '../../../src/platform/authorization/entity-identifiers.js'; +} from '../../../src/index.js';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/tests/mocha/unit/entity-identifiers.spec.ts` around lines 2 - 8, The test currently imports helper functions (forEmail, forClientId, forUserName, forToken, withRequestToken) directly from the internal module entity-identifiers.js which won't catch changes to the package's top-level exports; update at least one test (e.g., in entity-identifiers.spec.ts) to import those same symbols from the SDK entrypoint (../../../src/index.js) or add a small new test that imports from the entrypoint and asserts the helpers are defined/behave as expected so regressions in lib/src/index.ts are detected.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@lib/tests/mocha/unit/entity-identifiers.spec.ts`:
- Around line 2-8: The test currently imports helper functions (forEmail,
forClientId, forUserName, forToken, withRequestToken) directly from the internal
module entity-identifiers.js which won't catch changes to the package's
top-level exports; update at least one test (e.g., in
entity-identifiers.spec.ts) to import those same symbols from the SDK entrypoint
(../../../src/index.js) or add a small new test that imports from the entrypoint
and asserts the helpers are defined/behave as expected so regressions in
lib/src/index.ts are detected.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ce296dd3-66e3-4a9b-a137-1f8a08a64246
📒 Files selected for processing (3)
lib/src/index.tslib/src/platform/authorization/entity-identifiers.tslib/tests/mocha/unit/entity-identifiers.spec.ts
There was a problem hiding this comment.
Code Review
This pull request introduces a set of convenience constructors for EntityIdentifier to mirror the Go SDK's functionality, including helpers for email, client ID, username, and tokens. It also adds corresponding unit tests. The review feedback suggests improving parity with the Go SDK by adding a helper for registeredResourceValueFqn and exporting the forEntity function to allow for custom entity identifiers.
Address CodeRabbit review: import from src/index.js instead of the internal module path so regressions in exports are caught. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Mary Dickson <mary.dickson@virtru.com>
Summary
forEmail,forClientId,forUserName,forToken, andwithRequestTokenhelper functions that mirror the Go SDK's EntityIdentifier constructors@opentdf/sdktop-levelBefore:
After:
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Tests