feat: implement update/upsert for delegate models

[ymc9]

No description provided.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

✨ Finishing Touches

🧪 Generate unit tests

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch feat/delegate-update

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>, please review it.
  • Explain this complex logic.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai explain this code block.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  • @coderabbitai read src/utils.ts and explain its main purpose.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
  • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need 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)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai generate docstrings to generate docstrings for this PR.
• @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
• @coderabbitai generate unit tests to generate unit tests for this PR.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[claude]

Claude finished @ymc9's task —— View job

---

Pull Request Review

Reviewing PR for delegate model update/upsert implementation...

Todo List

• Read and analyze changed files
• Review core implementation in base operations
• Review update operations changes
• Analyze test coverage and quality
• Check for code quality and best practices
• Identify potential bugs or issues
• Evaluate performance considerations
• Check for security concerns
• Provide comprehensive feedback

---

Implementation Review

✅ Strengths

1. Comprehensive Delegate Model Support

The implementation properly handles multi-level delegate hierarchies (e.g., RatedVideo extends Video extends Asset). The field splitting logic correctly separates fields between base and child models:

// base.ts:1219-1248 - processBaseModelUpdate
Object.entries(updateFields).forEach(([field, value]) => {
    const fieldDef = this.getField(model, field);
    if (fieldDef) {
        thisUpdateFields[field] = value;
    } else {
        remainingFields[field] = value;
    }
});

2. Proper Filter Handling

The isIdFilter method (base.ts:1211-1217) and needIdRead logic (base.ts:1077-1094) correctly handle cases where update filters may reference fields from different hierarchy levels, requiring ID resolution.

3. Extensive Test Coverage

The test suite covers:

• ✅ Single and multi-level updates (asset.update, video.update, ratedVideo.update)
• ✅ Nested relation updates
• ✅ Bulk operations (updateMany, updateManyAndReturn)
• ✅ Upsert operations (both create and update paths)
• ✅ Various filter types (ID filters, unique field filters)

4. Transaction Safety

Operations properly use safeTransaction to ensure atomicity across base/child model updates.

---

⚠️ Issues & Concerns

1. Performance - Multiple Queries Per Update

Location: base.ts:1085-1094

The implementation may execute unnecessary reads when needIdRead is true:

if (needIdRead) {
    const readResult = await this.readUnique(kysely, model, {
        where: combinedWhere,
        select: this.makeIdSelect(model),
    });
    // ... additional logic
}

Impact: Each delegate model update with non-ID filters triggers an extra SELECT query.
Suggestion: Consider optimizing by checking if the base update returns ID values that can be reused.

2. Error Handling - Inconsistent NotFoundError

Location: base.ts:1090-1094 vs base.ts:1174-1180

// Inconsistent handling:
if (!readResult && throwIfNotFound) {
    throw new NotFoundError(model);  // Simple message
}
// vs later:
if (!updatedEntity) {
    if (throwIfNotFound) {
        throw new NotFoundError(model);  // Same simple message  
    }
}

Issue: Error messages don't provide context about which step failed (base vs child update).

3. Type Safety - Missing Validation

Location: base.ts:1096-1108

const baseUpdateResult = await this.processBaseModelUpdate(
    kysely,
    modelDef.baseModel,
    combinedWhere,
    finalData,
    throwIfNotFound,
);
// No validation that baseUpdateResult.baseEntity contains required ID fields
combinedWhere = baseUpdateResult.baseEntity;

Risk: If base update fails to return proper ID fields, subsequent child update could fail silently or with unclear errors.

4. Code Duplication

Locations: base.ts:1219-1248 and base.ts:1410-1440

The field splitting logic is duplicated between processBaseModelUpdate and processBaseModelUpdateMany:

// Similar code in both methods:
Object.entries(updateFields).forEach(([field, value]) => {
    const fieldDef = this.getField(model, field);
    if (fieldDef) {
        thisUpdateFields[field] = value;
    } else {
        remainingFields[field] = value;
    }
});

Suggestion: Extract field splitting into a shared helper method.

---

🔄 Performance Considerations

1. UpdateMany Optimization

The updateManyAndReturn implementation (update.ts:74-83) performs:

1. updateMany to get IDs
2. read to fetch full records

For large datasets, this could be expensive. Consider if the dialect supports UPDATE...RETURNING to do this in one query.

2. Transaction Overhead

Every update operation uses transactions, even simple single-model updates. Consider optimizing simple cases to avoid transaction overhead.

---

🔒 Security Assessment

✅ No security concerns identified

• Proper input validation through inputValidator
• Safe SQL generation through Kysely query builder
• No raw SQL injection points
• Appropriate use of parameterized queries

---

🧪 Test Quality

Excellent coverage with good edge case testing:

• ✅ Multi-level inheritance scenarios
• ✅ Relation updates at different hierarchy levels
• ✅ Error cases (delegate model direct operations)
• ✅ Both ID and unique field filters

Minor gap: No tests for concurrent update scenarios or transaction rollback cases.

---

📝 Recommendations

1. Extract field splitting logic into a shared helper method
2. Add context to error messages to distinguish base vs child update failures
3. Consider optimization for simple single-model updates to avoid extra queries
4. Add JSDoc comments to new public methods like processBaseModelUpdate
5. Add validation that base model updates return required ID fields

---

Overall Assessment

✅ Approve with minor improvements suggested

This PR successfully implements update/upsert functionality for delegate models with:

• Robust handling of complex inheritance hierarchies
• Comprehensive test coverage
• Proper transaction safety
• No security issues

The identified issues are optimization opportunities and code quality improvements rather than blocking problems. The core functionality is solid and well-tested.

---

[Copilot]

Pull Request Overview

This PR implements update and upsert operations for delegate models, extending the existing CRUD functionality to support polymorphic model hierarchies. This allows delegate models (which inherit from base models) to be updated and upserted while properly handling the hierarchical structure.

• Implementation of update, updateMany, updateManyAndReturn, and upsert operations for delegate models
• Support for nested updates and updates to base model fields from delegate models
• Addition of comprehensive test coverage for all new update operations

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
packages/runtime/test/client-api/delegate.test.ts	Adds comprehensive test suite covering update, updateMany, updateManyAndReturn, and upsert operations for delegate models
packages/runtime/src/client/crud/operations/update.ts	Refactors update operation to handle delegate models with proper read-back logic and transaction handling
packages/runtime/src/client/crud/operations/base.ts	Implements core logic for handling base model updates, including field separation and hierarchical processing