-
Notifications
You must be signed in to change notification settings - Fork 11
add new upser tfunctions #457
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe changes update the release management methods within the rule-engine repository. The Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant DBReleaseRepository
participant Database
Client->>DBReleaseRepository: createRelease(release)
DBReleaseRepository->>Database: Insert release record
Database-->>DBReleaseRepository: Confirmation
Client->>DBReleaseRepository: upsertRelease(versionId, variables)
DBReleaseRepository->>Database: Upsert release record
Database-->>DBReleaseRepository: Upsert result
Client->>DBReleaseRepository: updateReleaseVariables(variables)
DBReleaseRepository->>DBReleaseRepository: Fetch latest versionId
DBReleaseRepository->>Database: Upsert release with variables
Database-->>DBReleaseRepository: Upsert result
Client->>DBReleaseRepository: updateReleaseVersion(versionId)
DBReleaseRepository->>DBReleaseRepository: Retrieve latest variables
DBReleaseRepository->>Database: Upsert release with versionId
Database-->>DBReleaseRepository: Upsert result
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:
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
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (4)
packages/rule-engine/src/repositories/types.ts (1)
62-65: Method signature updated to be more explicitThe renaming from
upserttoupsertReleasemakes the method's purpose clearer, and the parameter changes make it more explicit by directly specifying what's needed instead of using an options object.However, the JSDoc comment above (lines 55-61) still refers to the old parameter structure with
@param options. Consider updating the documentation to match the new signature./** * Creates or retrieves an existing release for given parameters - * @param options - The release target identifier * @param versionId - Version ID for the release * @param variables - Variables for the release * @returns Object with created flag and the release */packages/rule-engine/src/repositories/db-release-repository.ts (3)
166-168: Incomplete method documentationThe JSDoc comment for the
upsertReleasemethod appears to be incomplete. Line 168 is empty but might have previously contained important documentation.Consider completing the documentation to fully describe the method's purpose and behavior.
210-216: New helper method adds useful functionalityThis new method provides a convenient way to update just the variables while keeping the current version ID, which is a common use case.
Consider adding JSDoc documentation to maintain consistency with other methods in the class:
+/** + * Creates or retrieves an existing release with current version ID and new variables + * @param variables - The variables for the release + * @returns Object indicating if a new release was created and the final release, or null if no version ID exists + */ async upsertReleaseWithVariables( variables: MaybeVariable[], ): Promise<{ created: boolean; release: ReleaseWithId } | null> {
218-223: New helper method adds useful functionalityThis new method provides a convenient way to update just the version while keeping the current variables, which is a common use case.
Consider adding JSDoc documentation to maintain consistency with other methods in the class:
+/** + * Creates or retrieves an existing release with new version ID and current variables + * @param versionId - The version ID for the release + * @returns Object indicating if a new release was created and the final release + */ async upsertReleaseWithVersionId( versionId: string, ): Promise<{ created: boolean; release: ReleaseWithId }> {
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/rule-engine/src/repositories/db-release-repository.ts(4 hunks)packages/rule-engine/src/repositories/types.ts(2 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/rule-engine/src/repositories/types.tspackages/rule-engine/src/repositories/db-release-repository.ts
🧬 Code Definitions (1)
packages/rule-engine/src/repositories/db-release-repository.ts (2)
packages/rule-engine/src/repositories/variables/types.ts (1)
MaybeVariable(9-9)packages/rule-engine/src/repositories/types.ts (1)
ReleaseWithId(16-16)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: build (linux/amd64)
- GitHub Check: Typecheck
- GitHub Check: Lint
🔇 Additional comments (2)
packages/rule-engine/src/repositories/db-release-repository.ts (2)
158-164: Method rename enhances clarityThe rename from
createtocreateReleasemakes the method's purpose more explicit and follows a consistent naming pattern with other methods in the class.
173-208: Method signature updated for clarityThe renaming from
upserttoupsertReleaseand the parameter changes improve clarity by being more explicit about what the method needs.The implementation correctly uses the new parameters and properly updates the call to the renamed
createReleasemethod.
Summary by CodeRabbit
New Features
Refactor