add netadata key matching #550
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThis update introduces support for richer metadata-based matching in resource relationship rules, enabling both key and key-value constraints. It adds a PATCH endpoint for updating rules, refines the POST endpoint to be create-only with conflict detection, updates schemas and OpenAPI specs, expands RBAC and permission enums, and includes corresponding migrations and tests. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API
participant Auth
participant DB
Client->>API: PATCH /v1/resource-relationship-rules/{ruleId}
API->>Auth: Authenticate & Authorize (ResourceRelationshipRuleUpdate)
Auth-->>API: Success/Failure
API->>DB: Check if rule exists (by ruleId)
DB-->>API: Rule found/not found
alt Rule exists
API->>DB: Transaction: Update rule, replace metadataMatches and metadataEquals
DB-->>API: Updated rule & metadata
API-->>Client: 200 OK with updated rule
else Rule not found
API-->>Client: 404 Not Found
end
Note over API,Client: On error, API returns 500 Internal Server Error
sequenceDiagram
participant Client
participant API
participant Auth
participant DB
Client->>API: POST /v1/resource-relationship-rules
API->>Auth: Authenticate & Authorize (ResourceRelationshipRuleCreate)
Auth-->>API: Success/Failure
API->>DB: Check for existing rule (by unique fields)
DB-->>API: Rule found/not found
alt Rule exists
API-->>Client: 409 Conflict with error message
else No existing rule
API->>DB: Transaction: Insert rule and metadata
DB-->>API: Created rule & metadata
API-->>Client: 200 OK with created rule
end
Note over API,Client: On error, API returns 500 Internal Server Error
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🔭 Outside diff range comments (2)
openapi.v1.json (2)
3148-3161: 🛠️ Refactor suggestionAdd tags and security to POST create endpoint
The
POST /v1/resource-relationship-rulesroute registers new rules but currently has notagsorsecuritydefinitions, making it untracked for RBAC. Align it with other relationship endpoints.@@ -3148,7 +3148,9 @@ "/v1/resource-relationship-rules": { "post": { "summary": "Create a resource relationship rule", + "tags": ["Resource Relationships"], + "security": [{ "bearerAuth": [] }], "operationId": "createResourceRelationshipRule", "requestBody": { "required": true,
5919-5922:⚠️ Potential issueSpecify
uuidformat forworkspaceIdin Create schemaThe
CreateResourceRelationshipRuleschema’sworkspaceIdproperty lacksformat: "uuid", unlike other schemas, which may lead to inconsistent validation.@@ -5919,3 +5919,5 @@ "workspaceId": { - "type": "string" + "type": "string", + "format": "uuid" },
🧹 Nitpick comments (7)
apps/webservice/src/app/api/v1/resource-relationship-rules/[ruleId]/route.ts (1)
124-130: Bubble up precise error codesThe catch-all 500 hides the root cause (e.g. FK violation, permission logic bug).
Return the error code only for truly unexpected errors and surface expected DB errors with 4xx codes so clients can act accordingly.
(Example: unique-index violation → 409 Conflict.)packages/db/src/queries/get-resource-parents.ts (2)
122-152: Typo:relatipnshipTargets→relationshipTargetsThe misspelling leaks into the exported object (
getTargetsWithMetadata).
While TypeScript will still allow property access, the typo hurts discoverability and IDE autocomplete.-const relatipnshipTargets = async () => ... - -return { - relationships: ..., - getTargetsWithMetadata: relatipnshipTargets, -}; +const relationshipTargets = async () => ... + +return { + relationships: ..., + getTargetsWithMetadata: relationshipTargets, +};
20-40: Potential performance issue withexistssub-queries
isMetadataMatchSatisfiedandisMetadataEqualsSatisfiedinvoke correlatedEXISTSsub-queries for every candidate rule.
For workspaces with many resources & rules this can degrade quickly.Options:
- Pre-aggregate metadata into a CTE and join once.
- Add composite indexes on
(resource_id, key [, value])to accelerate the look-ups.Worth benchmarking before this hits production.
e2e/tests/api/resource-relationships.spec.ts (1)
129-184: Mind the HTTP semantics – consider asserting201 CreatedThe POST endpoints currently return
200 OK.
REST conventions usually favour201 Createdfor successful creations and include aLocationheader.
Aligning with the standard improves API predictability and helps client generators.If the backend is updated, remember to adjust the test assertions:
-expect(resourceRelationship.response.status).toBe(200); +expect(resourceRelationship.response.status).toBe(201);apps/webservice/src/app/api/v1/resource-relationship-rules/openapi.ts (2)
25-34: Prefer 201 for successful “create” operationsThe endpoint represents a pure “create” action, yet the spec still returns 200 instead of the more semantically-accurate 201 Created (with an optional
Locationheader). Aligning the response code with REST conventions helps client tooling and reduces ambiguity around upsert vs create semantics.- "200": { + "201": { description: "Resource relationship rule created successfully",
35-47: Consider re-using an error schema componentThe inline schema for the
409response repeats the{ error: string }pattern already used elsewhere (e.g. 500). Duplicating this in many operations inflates the spec and soon diverges. Extract a reusableErrorResponsecomponent and$refit here.apps/webservice/src/app/api/v1/resource-relationship-rules/route.ts (1)
50-56: Return 201 & Location header after successful insertAfter a successful transaction we’re creating a brand-new rule. Responding with
201andLocation: /v1/resource-relationship-rules/{id}keeps the API self-describing and aligns with REST & the updated OpenAPI suggestion.- return NextResponse.json(rule); + return NextResponse.json(rule, { status: 201, headers: { Location: `/v1/resource-relationship-rules/${rule.id}` } });
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
apps/webservice/src/app/api/v1/jobs/[jobId]/get-job.ts(1 hunks)apps/webservice/src/app/api/v1/resource-relationship-rules/[ruleId]/openapi.ts(1 hunks)apps/webservice/src/app/api/v1/resource-relationship-rules/[ruleId]/route.ts(1 hunks)apps/webservice/src/app/api/v1/resource-relationship-rules/openapi.ts(5 hunks)apps/webservice/src/app/api/v1/resource-relationship-rules/route.ts(1 hunks)e2e/api/schema.ts(9 hunks)e2e/tests/api/resource-relationships.spec.ts(4 hunks)openapi.v1.json(8 hunks)packages/api/src/router/resource-relationship-rules.ts(1 hunks)packages/auth/src/utils/rbac.ts(3 hunks)packages/db/drizzle/0099_dear_korg.sql(1 hunks)packages/db/drizzle/meta/_journal.json(1 hunks)packages/db/src/queries/get-resource-parents.ts(2 hunks)packages/db/src/schema/rbac.ts(1 hunks)packages/db/src/schema/resource-relationship-rule.ts(4 hunks)packages/validators/src/auth/index.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{ts,tsx}`: **Note on Error Handling:** Avoid strict enforcement of try/catch blocks. Code may use early returns, Promise chains (.then().catch()), or other patterns for error...
**/*.{ts,tsx}: Note on Error Handling:
Avoid strict enforcement of try/catch blocks. Code may use early returns, Promise chains (.then().catch()), or other patterns for error handling. These are acceptable as long as they maintain clarity and predictability.
packages/api/src/router/resource-relationship-rules.tspackages/db/src/schema/rbac.tspackages/validators/src/auth/index.tsapps/webservice/src/app/api/v1/jobs/[jobId]/get-job.tsapps/webservice/src/app/api/v1/resource-relationship-rules/[ruleId]/route.tspackages/auth/src/utils/rbac.tsapps/webservice/src/app/api/v1/resource-relationship-rules/[ruleId]/openapi.tsapps/webservice/src/app/api/v1/resource-relationship-rules/openapi.tse2e/api/schema.tspackages/db/src/schema/resource-relationship-rule.tsapps/webservice/src/app/api/v1/resource-relationship-rules/route.tse2e/tests/api/resource-relationships.spec.tspackages/db/src/queries/get-resource-parents.ts
🧠 Learnings (1)
packages/db/src/schema/rbac.ts (1)
Learnt from: adityachoudhari26
PR: ctrlplanedev/ctrlplane#181
File: packages/auth/src/utils/rbac.ts:102-118
Timestamp: 2024-10-29T02:05:46.185Z
Learning: The `releaseChannel` scope type is included in the `scopeType` enum in `packages/db/src/schema/rbac.ts`.
🧬 Code Graph Analysis (4)
apps/webservice/src/app/api/v1/resource-relationship-rules/[ruleId]/route.ts (5)
packages/logger/src/index.ts (1)
logger(48-48)packages/db/src/common.ts (2)
Tx(22-22)takeFirst(9-13)apps/webservice/src/app/api/v1/middleware.ts (1)
request(24-53)apps/webservice/src/app/api/v1/auth.ts (2)
authn(15-21)authz(23-58)packages/auth/src/utils/can.ts (1)
can(50-53)
packages/auth/src/utils/rbac.ts (4)
packages/db/src/client.ts (1)
db(15-15)packages/db/src/schema/resource-relationship-rule.ts (1)
resourceRelationshipRule(56-87)packages/db/src/schema/workspace.ts (1)
workspace(18-27)packages/db/src/common.ts (1)
takeFirst(9-13)
apps/webservice/src/app/api/v1/resource-relationship-rules/[ruleId]/openapi.ts (1)
apps/webservice/src/app/api/v1/resource-relationship-rules/openapi.ts (1)
openapi(3-163)
packages/db/src/queries/get-resource-parents.ts (1)
packages/db/src/common.ts (1)
Tx(22-22)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: build (linux/amd64)
- GitHub Check: Typecheck
- GitHub Check: build (linux/amd64)
- GitHub Check: Lint
- GitHub Check: build (linux/amd64)
🔇 Additional comments (16)
packages/db/src/schema/rbac.ts (1)
35-55: LGTM: Clean addition of the new scope typeThe addition of the
"resourceRelationshipRule"scope type to the enum is clean and properly aligned with the existing pattern. This enables RBAC authorization flows for resource relationship rules, which is consistent with the retrieved learning about how scope types are managed in this system.packages/db/drizzle/meta/_journal.json (1)
697-704: LGTM: Properly formatted migration journal entryThe new migration entry (index 99) follows the established pattern and correctly documents the database schema changes for the
resource_relationship_rule_metadata_equalstable.packages/api/src/router/resource-relationship-rules.ts (1)
21-21: LGTM: Properly added eager loading for metadataEqualsThe modification correctly extends the query to eagerly load both
metadataMatchesand the newly introducedmetadataEqualsrelated entities, which is necessary for the enhanced metadata matching capabilities.apps/webservice/src/app/api/v1/jobs/[jobId]/get-job.ts (1)
4-4: LGTM: Clean import path refactoringThe import path has been improved from a direct relative path to a cleaner package import. This is a good practice that improves maintainability.
packages/validators/src/auth/index.ts (1)
71-73: Appropriate permission additions for resource relationship rule operations.These new permission constants follow the established naming convention and are correctly positioned with other resource-related permissions. They enable fine-grained access control for the separate create, update, and delete operations on resource relationship rules.
packages/db/drizzle/0099_dear_korg.sql (3)
1-6: Well-structured database table for metadata key-value matching.The new table
resource_relationship_rule_metadata_equalsis properly designed with UUID primary key, appropriate columns for keys and values, and follows database normalization principles.
8-8: Proper foreign key constraint with cascade delete.The foreign key relationship to the
resource_relationship_ruletable is correctly set up with cascade delete to ensure referential integrity when a parent rule is deleted.
9-9: Appropriate unique constraint for preventing duplicate keys.The unique index on
resource_relationship_rule_idandkeyprevents duplicate keys for the same rule, which maintains data integrity.packages/auth/src/utils/rbac.ts (2)
252-270: Well-implemented RBAC scope handler for resource relationship rules.The
getResourceRelationshipRuleScopesfunction correctly follows the established pattern for other scope types by:
- Querying the database to fetch the relationship rule and its associated workspace
- Returning a properly structured array of scope objects for permission checking
The implementation maintains consistency with the rest of the codebase.
500-500: Appropriate registration of the new scope handler.The new scope handler is correctly registered in the
scopeHandlersrecord, making it available for RBAC permission checks throughout the application.apps/webservice/src/app/api/v1/resource-relationship-rules/[ruleId]/openapi.ts (2)
3-106: Well-structured OpenAPI specification for the PATCH endpoint.The OpenAPI specification for updating resource relationship rules via PATCH is well-defined with:
- Clear schema for the update payload with all appropriate fields
- Proper path parameter definition for
ruleId- Comprehensive response specifications for success and error cases
- Support for the new metadata matching capabilities
This aligns well with the REST API best practices by using PATCH for partial updates of a resource.
25-39: Complete metadata matching capabilities in the API schema.The schema properly includes both
metadataKeysMatch(string array) andmetadataKeysEquals(key-value object array) to support the enhanced metadata matching functionality. The required properties formetadataKeysEqualsitems are correctly specified.apps/webservice/src/app/api/v1/resource-relationship-rules/openapi.ts (1)
138-148: Nice consistency with the “create” schemaRequiring
keyandvaluein the array items prevents malformed objects from slipping through. 👍apps/webservice/src/app/api/v1/resource-relationship-rules/route.ts (2)
20-21: Pass a validation schema, not a DB table definition
parseBodyexpects a Zod schema. Ensure thatschema.createResourceRelationshipRuleis a Zod validator, not the Drizzle table. If it is the latter, validation will silently be skipped and typing will be misleading.
31-48: Duplicate-detection criteria may be too lenientThe uniqueness check only matches
workspaceId,reference, anddependencyType.
Two rules with differentsourceKind/Versionbut identical reference + dependency type would now be blocked even though they act on different resources. Confirm that this is intentional business logic – otherwise extend the predicate.e2e/api/schema.ts (1)
1276-1291: Generated types expose optionalkey/valueBecause the base spec omits
required, generated TS types make both fields optional. Once the spec is fixed (see openapi.ts comment), regenerate the SDK to tighten these types.
| const metadataKeys = _.uniq(metadataKeysMatch ?? []); | ||
| if (metadataKeys.length > 0) | ||
| await tx.insert(schema.resourceRelationshipRuleMetadataMatch).values( | ||
| metadataKeys.map((key) => ({ | ||
| resourceRelationshipRuleId: ruleId, | ||
| key, | ||
| })), | ||
| ); | ||
|
|
||
| return metadataKeys; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Duplicate key handling may silently drop conflicting values
_.uniq(metadataKeysMatch) (and uniqBy below) keeps only the first occurrence.
If the client unintentionally supplies the same key twice with different values in metadataKeysEquals, one of them will be discarded without warning.
Consider:
- validating in Zod that keys are unique and consistent, returning
400when conflicts are detected, or - merging duplicates deterministically and logging a warning so operators understand why data disappeared.
| const rule = await db.transaction(async (tx) => { | ||
| const rule = await tx | ||
| .update(schema.resourceRelationshipRule) | ||
| .set(body) | ||
| .where(eq(schema.resourceRelationshipRule.id, ruleId)) | ||
| .returning() | ||
| .then(takeFirst); | ||
|
|
There was a problem hiding this comment.
Avoid passing non-column & undefined fields into .set()
update().set(body) receives the entire parsed payload, which includes:
- properties that do not exist on
resourceRelationshipRule(metadataKeysMatch,metadataKeysEquals) - properties that may be
undefined(because the schema ispartial())
Drizzle will happily accept the object at runtime but will try to write all keys.
This can surface as:
- TS compilation errors (extra keys) that get silenced with
any - SQL errors such as “column metadata_keys_match does not exist”
- Columns overwritten with
NULL/DEFAULTwhenundefinedsneaks in
Sanitise the payload before issuing the update:
-const rule = await tx
- .update(schema.resourceRelationshipRule)
- .set(body)
+const updateData = _.omitBy(
+ _.omit(body, ["metadataKeysMatch", "metadataKeysEquals"]),
+ _.isUndefined,
+) as Partial<typeof schema.resourceRelationshipRule.$inferInsert>;
+
+const rule = await tx
+ .update(schema.resourceRelationshipRule)
+ .set(updateData)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const rule = await db.transaction(async (tx) => { | |
| const rule = await tx | |
| .update(schema.resourceRelationshipRule) | |
| .set(body) | |
| .where(eq(schema.resourceRelationshipRule.id, ruleId)) | |
| .returning() | |
| .then(takeFirst); | |
| const rule = await db.transaction(async (tx) => { | |
| // sanitize: drop non-columns and undefined fields | |
| const updateData = _.omitBy( | |
| _.omit(body, ["metadataKeysMatch", "metadataKeysEquals"]), | |
| _.isUndefined, | |
| ) as Partial<typeof schema.resourceRelationshipRule.$inferInsert>; | |
| const rule = await tx | |
| .update(schema.resourceRelationshipRule) | |
| .set(updateData) | |
| .where(eq(schema.resourceRelationshipRule.id, ruleId)) | |
| .returning() | |
| .then(takeFirst); | |
| return rule; | |
| }); |
| export const createResourceRelationshipRule = createInsertSchema( | ||
| resourceRelationshipRule, | ||
| ); | ||
| ) | ||
| .omit({ id: true }) | ||
| .extend({ | ||
| metadataKeysMatch: z.array(z.string()).optional(), | ||
| metadataKeysEquals: z | ||
| .array(z.object({ key: z.string(), value: z.string() })) | ||
| .optional(), | ||
| }); | ||
|
|
||
| export const updateResourceRelationshipRule = | ||
| createResourceRelationshipRule.partial(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Schema allows divergent metadataKeysEquals duplicates client-side
metadataKeysEquals is only validated as an array of objects; nothing enforces unique keys or non-empty arrays.
Given the table has a unique index on (ruleId, key) the server will throw 23505 violations for duplicates that the API layer could have caught.
Add refinement to the Zod schema:
.extend({
metadataKeysMatch: z.array(z.string()).optional(),
- metadataKeysEquals: z
- .array(z.object({ key: z.string(), value: z.string() }))
- .optional(),
+ metadataKeysEquals: z
+ .array(z.object({ key: z.string(), value: z.string() }))
+ .optional()
+ .refine(
+ (arr) => _.uniqBy(arr, "key").length === arr.length,
+ "metadataKeysEquals must contain unique keys",
+ ),
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const createResourceRelationshipRule = createInsertSchema( | |
| resourceRelationshipRule, | |
| ); | |
| ) | |
| .omit({ id: true }) | |
| .extend({ | |
| metadataKeysMatch: z.array(z.string()).optional(), | |
| metadataKeysEquals: z | |
| .array(z.object({ key: z.string(), value: z.string() })) | |
| .optional(), | |
| }); | |
| export const updateResourceRelationshipRule = | |
| createResourceRelationshipRule.partial(); | |
| export const createResourceRelationshipRule = createInsertSchema( | |
| resourceRelationshipRule, | |
| ) | |
| .omit({ id: true }) | |
| .extend({ | |
| metadataKeysMatch: z.array(z.string()).optional(), | |
| metadataKeysEquals: z | |
| .array(z.object({ key: z.string(), value: z.string() })) | |
| .optional() | |
| .refine( | |
| (arr) => _.uniqBy(arr, "key").length === arr.length, | |
| "metadataKeysEquals must contain unique keys", | |
| ), | |
| }); | |
| export const updateResourceRelationshipRule = | |
| createResourceRelationshipRule.partial(); |
| metadataKeysMatch: { | ||
| type: "array", | ||
| items: { type: "string" }, | ||
| }, | ||
| metadataKeysEquals: { | ||
| type: "array", | ||
| items: { | ||
| type: "object", | ||
| properties: { | ||
| key: { type: "string" }, | ||
| value: { type: "string" }, | ||
| }, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
metadataKeysEquals items lack required fields – may generate loose types
In ResourceRelationshipRule, the key & value fields inside each metadataKeysEquals item are optional because no required array is defined.
This differs from CreateResourceRelationshipRule, where both are marked required, and results in generated typings that allow {} items at runtime.
metadataKeysEquals: {
type: "array",
items: {
type: "object",
properties: {
key: { type: "string" },
value: { type: "string" },
},
+ required: ["key", "value"],
},
},There was a problem hiding this comment.
@jsbroks Here's the commit suggestion:
import type { Swagger } from "atlassian-openapi";
export const openapi: Swagger.SwaggerV3 = {
// content remains unchanged...
components: {
schemas: {
// content remains unchanged...
ResourceRelationshipRule: {
type: "object",
properties: {
// other properties remain unchanged...
metadataKeysEquals: {
type: "array",
items: {
type: "object",
properties: {
key: { type: "string" },
value: { type: "string" },
},
+ required: ["key", "value"],
},
},
},
required: [
// required fields remain unchanged...
],
},
// content remains unchanged...
},
},
};This change ensures that both key and value fields are required in each metadataKeysEquals item, matching the pattern used in the CreateResourceRelationshipRule schema and preventing loosely typed objects.
| const metadataKeysEquals = _.uniqBy( | ||
| body.metadataKeysEquals ?? [], | ||
| (m) => m.key, | ||
| ); | ||
| } | ||
|
|
||
| return rule; | ||
| }, | ||
| ); | ||
| if (metadataKeysEquals.length > 0) | ||
| await tx | ||
| .insert(schema.resourceRelationshipRuleMetadataEquals) | ||
| .values( | ||
| metadataKeysEquals.map((m) => ({ | ||
| resourceRelationshipRuleId: rule.id, | ||
| key: m.key, | ||
| value: m.value, | ||
| })), | ||
| ); |
There was a problem hiding this comment.
Silently dropping duplicates by key may hide conflicting values
_.uniqBy(body.metadataKeysEquals ?? [], m => m.key) discards later items with the same key but different values.
That behaviour can mask user errors. Instead, detect conflicts and return 400 with a clear message.
-const metadataKeysEquals = _.uniqBy(body.metadataKeysEquals ?? [], (m) => m.key);
+const equals = body.metadataKeysEquals ?? [];
+const duplicateKeys = _(equals).groupBy("key").pickBy(v => v.length > 1).keys().value();
+if (duplicateKeys.length)
+ throw new Error(`Duplicate metadataKeysEquals detected for keys: ${duplicateKeys.join(", ")}`);
+const metadataKeysEquals = equals;Committable suggestion skipped: line range outside the PR's diff.
| "metadataKeysMatch": { | ||
| "type": "array", | ||
| "items": { | ||
| "type": "string" | ||
| } | ||
| }, | ||
| "metadataKeysEquals": { | ||
| "type": "array", | ||
| "items": { | ||
| "type": "object", | ||
| "properties": { | ||
| "key": { | ||
| "type": "string" | ||
| }, | ||
| "value": { | ||
| "type": "string" | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Enforce required properties on metadataKeysEquals items
In the ResourceRelationshipRule schema, the metadataKeysEquals items define properties but omit a required array, allowing malformed objects. This should mirror the item schema in CreateResourceRelationshipRule/UpdateResourceRelationshipRule.
@@ -5885,7 +5885,10 @@
"metadataKeysEquals": {
"type": "array",
"items": {
- "type": "object",
+ "type": "object",
"properties": {
"key": { "type": "string" },
"value": { "type": "string" }
+ },
+ "required": ["key", "value"]
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "metadataKeysMatch": { | |
| "type": "array", | |
| "items": { | |
| "type": "string" | |
| } | |
| }, | |
| "metadataKeysEquals": { | |
| "type": "array", | |
| "items": { | |
| "type": "object", | |
| "properties": { | |
| "key": { | |
| "type": "string" | |
| }, | |
| "value": { | |
| "type": "string" | |
| } | |
| } | |
| } | |
| } | |
| "metadataKeysMatch": { | |
| "type": "array", | |
| "items": { | |
| "type": "string" | |
| } | |
| }, | |
| "metadataKeysEquals": { | |
| "type": "array", | |
| "items": { | |
| "type": "object", | |
| "properties": { | |
| "key": { | |
| "type": "string" | |
| }, | |
| "value": { | |
| "type": "string" | |
| } | |
| }, | |
| "required": ["key", "value"] | |
| } | |
| } |
| "/v1/resource-relationship-rules/{ruleId}": { | ||
| "patch": { | ||
| "summary": "Update a resource relationship rule", | ||
| "operationId": "updateResourceRelationshipRule", | ||
| "parameters": [ | ||
| { | ||
| "name": "ruleId", | ||
| "in": "path", | ||
| "required": true, | ||
| "schema": { | ||
| "type": "string", | ||
| "format": "uuid" | ||
| } | ||
| } | ||
| ], | ||
| "requestBody": { | ||
| "content": { | ||
| "application/json": { | ||
| "schema": { | ||
| "$ref": "#/components/schemas/UpdateResourceRelationshipRule" | ||
| } | ||
| } | ||
| } | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Missing requestBody required flag and security definitions for PATCH endpoint
The PATCH /v1/resource-relationship-rules/{ruleId} endpoint lacks "required": true on its requestBody (all other PATCH operations define this) and omits tags and security metadata for RBAC enforcement.
Proposed diff:
@@ -3073,8 +3073,13 @@
- "/v1/resource-relationship-rules/{ruleId}": {
- "patch": {
+ "/v1/resource-relationship-rules/{ruleId}": {
+ "patch": {
"summary": "Update a resource relationship rule",
"operationId": "updateResourceRelationshipRule",
+ "tags": ["Resource Relationships"],
+ "security": [{ "bearerAuth": [] }],
+ "requestBody": {
+ "required": true,
"content": {
"application/json": {
"schema": {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "/v1/resource-relationship-rules/{ruleId}": { | |
| "patch": { | |
| "summary": "Update a resource relationship rule", | |
| "operationId": "updateResourceRelationshipRule", | |
| "parameters": [ | |
| { | |
| "name": "ruleId", | |
| "in": "path", | |
| "required": true, | |
| "schema": { | |
| "type": "string", | |
| "format": "uuid" | |
| } | |
| } | |
| ], | |
| "requestBody": { | |
| "content": { | |
| "application/json": { | |
| "schema": { | |
| "$ref": "#/components/schemas/UpdateResourceRelationshipRule" | |
| } | |
| } | |
| } | |
| }, | |
| "/v1/resource-relationship-rules/{ruleId}": { | |
| "patch": { | |
| "summary": "Update a resource relationship rule", | |
| "operationId": "updateResourceRelationshipRule", | |
| "tags": ["Resource Relationships"], | |
| "security": [{ "bearerAuth": [] }], | |
| "parameters": [ | |
| { | |
| "name": "ruleId", | |
| "in": "path", | |
| "required": true, | |
| "schema": { | |
| "type": "string", | |
| "format": "uuid" | |
| } | |
| } | |
| ], | |
| "requestBody": { | |
| "required": true, | |
| "content": { | |
| "application/json": { | |
| "schema": { | |
| "$ref": "#/components/schemas/UpdateResourceRelationshipRule" | |
| } | |
| } | |
| } | |
| }, |
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests
Chores