chore: several agreements#17
Conversation
WalkthroughThis set of changes removes the concept of a single "active agreement" from the partner domain model, database schema, and UI. Instead, it introduces handling for multiple agreements per partner by expanding the legal entity relation to include an agreements array. All UI and logic that previously depended on a single active agreement are updated to work with the agreements collection. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI
participant Store
participant API
participant DB
User->>UI: View Partner Details
UI->>Store: Fetch PartnerWithData
Store->>API: Request partner data
API->>DB: Query partners (include legalEntity.agreements)
DB-->>API: Return partner with agreements array
API-->>Store: Return PartnerWithData
Store-->>UI: Provide partner (with agreements)
UI->>UI: Render all agreements from legalEntity.agreements
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~18 minutes Possibly related PRs
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
🪧 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
CodeRabbit Configuration File (
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
apps/web-app/app/pages/partner/[id]/agreement.vue (1)
16-16: Guard CreateCard behind a legalEntity checkEnsure we only render and invoke the create-agreement modal when
legalEntityIdis defined.
- File: apps/web-app/app/pages/partner/[id]/agreement.vue, around the
<CreateCard>block:
- Wrap the card in a
v-if="partner?.legalEntity?.id"(or disable it) solegalEntityIdis never passed asundefined.Suggested diff:
- <div> - <CreateCard - label="Добавить новый договор" - icon="i-lucide-scroll-text" - @click="modalCreatePartnerAgreement.open({ partnerId: partner?.id, legalEntityId: partner?.legalEntity?.id })" - /> - </div> + <div v-if="partner?.legalEntity?.id"> + <CreateCard + label="Добавить новый договор" + icon="i-lucide-scroll-text" + @click="modalCreatePartnerAgreement.open({ partnerId: partner?.id, legalEntityId: partner.legalEntity.id })" + /> + </div> + <!-- Optionally: show a disabled state or message when no legalEntity is available -->
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/web-app/app/components/PartnerCard.vue(1 hunks)apps/web-app/app/components/form/UpdatePartnerAgreement.vue(2 hunks)apps/web-app/app/pages/partner/[id].vue(1 hunks)apps/web-app/app/pages/partner/[id]/agreement.vue(1 hunks)apps/web-app/app/pages/partner/[id]/index.vue(3 hunks)apps/web-app/app/stores/partner.ts(1 hunks)apps/web-app/server/api/partner/id/[partnerId]/agreement.post.ts(0 hunks)packages/database/src/repository/partner.ts(1 hunks)packages/database/src/tables.ts(0 hunks)
💤 Files with no reviewable changes (2)
- apps/web-app/server/api/partner/id/[partnerId]/agreement.post.ts
- packages/database/src/tables.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (11)
apps/web-app/app/pages/partner/[id]/agreement.vue (1)
4-10: Template refactoring looks correct for multiple agreements support.The change from displaying a single
activeAgreementto iterating overpartner?.legalEntity?.agreementsis implemented properly with:
- Appropriate null safety using optional chaining
- Correct key binding with
agreement.id- Maintained styling consistency
apps/web-app/app/pages/partner/[id]/index.vue (3)
21-23: Minor styling change noted.The class change from
"text-base"to"text-base/5"appears to be a CSS utility update, possibly adding opacity. This seems unrelated to the main agreements refactoring but is a minor styling adjustment.
39-41: Consistent styling change applied.Same styling update as line 21-23, changing to
"text-base/5"class.
49-55: Correctly refactored to display multiple agreements.The change from conditional single agreement rendering to
v-foriteration overpartner?.legalEntity?.agreementsis implemented correctly with:
- Proper null safety using optional chaining
- Correct key binding with
agreement.id- Consistent styling with
lg:col-span-2This aligns well with the overall refactoring pattern across the codebase.
packages/database/src/repository/partner.ts (1)
33-38: Database query correctly updated for multiple agreements structure.The change from separate
legalEntityandactiveAgreementrelations to a nested structure withlegalEntity.agreementsis correct and aligns with the overall refactoring. This provides the UI components with access to all agreements through the legal entity relationship.The Drizzle ORM query syntax is properly structured for the nested relation.
apps/web-app/app/stores/partner.ts (2)
3-5: Well-structured type definition for the new agreements model.The
PartnerLegalEntityWithDatatype correctly extends the basePartnerLegalEntitytype with anagreementsarray, providing proper TypeScript support for the refactored data structure.
7-10: Type definition correctly updated to support multiple agreements.The updated
PartnerWithDatatype properly:
- Uses the new
PartnerLegalEntityWithDatatype for thelegalEntityproperty- Removes the
activeAgreementproperty, aligning with the database schema changesThis type structure matches the data returned by the updated repository query and supports the UI component requirements.
apps/web-app/app/components/form/UpdatePartnerAgreement.vue (1)
31-31: LGTM! Improved UX with finer input control.Adding
step="0.1"to the marketing fee input enables more precise percentage input, which is appropriate for financial calculations.apps/web-app/app/components/PartnerCard.vue (3)
52-52: LGTM! Improved content visibility.Increasing the line clamp from 3 to 4 provides better visibility for city information.
60-66: LGTM! Type updates align with the new data structure.The type updates correctly reflect the transition from single active agreement to multiple agreements within the legal entity.
73-85: LGTM! Progress calculation logic is sound.The progress calculation correctly adapts to use the minimal agreement while maintaining the same mathematical logic for determining agreement progress based on concluded and end dates.
| const partner = computed(() => partnerStore.partners.find((partner) => partner.legalEntity?.agreements.some((agreement) => agreement.id === agreementId))) | ||
| const agreement = computed(() => partner.value?.legalEntity?.agreements.find((agreement) => agreement.id === agreementId)) |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify agreement lookup logic and consider performance optimization.
The new agreement lookup logic correctly implements the transition from single active agreement to multiple agreements. However, there are a few considerations:
- Performance: The partner lookup uses nested
find()andsome()operations which could be inefficient with many partners/agreements - Error handling: No explicit handling if the agreement is not found
Consider this more efficient approach:
-const partner = computed(() => partnerStore.partners.find((partner) => partner.legalEntity?.agreements.some((agreement) => agreement.id === agreementId)))
-const agreement = computed(() => partner.value?.legalEntity?.agreements.find((agreement) => agreement.id === agreementId))
+const partnerAndAgreement = computed(() => {
+ for (const partner of partnerStore.partners) {
+ const agreement = partner.legalEntity?.agreements.find(a => a.id === agreementId)
+ if (agreement) {
+ return { partner, agreement }
+ }
+ }
+ return { partner: null, agreement: null }
+})
+const partner = computed(() => partnerAndAgreement.value.partner)
+const agreement = computed(() => partnerAndAgreement.value.agreement)Please verify that all agreement lookups across the codebase handle the case where agreements might be undefined or empty arrays.
🏁 Script executed:
#!/bin/bash
# Description: Check for potential null/undefined access patterns in agreement lookups
# Expected: Find all places where agreements array is accessed to ensure proper null checking
rg -A 3 -B 1 "\.agreements\." --type vue --type ts
rg -A 3 -B 1 "agreements\[" --type vue --type ts
rg -A 3 -B 1 "agreements\.find" --type vue --type ts
rg -A 3 -B 1 "agreements\.some" --type vue --type tsLength of output: 325
🏁 Script executed:
#!/bin/bash
# Description: Search for all usages of `agreements` in .vue and .ts files
# to verify proper null/undefined checks around agreement lookups.
# Search for property access (e.g., partner.legalEntity.agreements)
rg -n "\.agreements" -g "*.vue" -g "*.ts" -A2 -B1
# Search for direct array indexing (e.g., agreements[0])
rg -n "agreements\[" -g "*.vue" -g "*.ts" -A2 -B1Length of output: 2147
Ensure safe and efficient agreement lookups
Most components guard against missing legalEntity, but we still see un‐handled cases and nested searches that can be optimized.
• apps/web-app/app/components/form/UpdatePartnerAgreement.vue (91–92)
– Two separate find()/some() calls scan the list twice.
– No fallback if no match is found.
• apps/web-app/app/pages/partner/[id].vue (49)
– Uses partner.value?.legalEntity?.agreements.length without default → may yield undefined.
• apps/web-app/app/pages/partner/[id]/index.vue (50–52) & …/agreement.vue (5–7)
– v-for="agreement in partner?.legalEntity?.agreements" on undefined can throw.
• apps/web-app/app/components/PartnerCard.vue (71)
– partner.legalEntity?.agreements.toSorted(…) calls .toSorted on undefined if agreements is missing.
Suggested fixes:
- Consolidate lookup in UpdatePartnerAgreement.vue into a single pass and handle “not found”:
- const partner = computed(() =>
- partnerStore.partners.find(p => p.legalEntity?.agreements.some(a => a.id === agreementId))
- )
- const agreement = computed(() =>
- partner.value?.legalEntity?.agreements.find(a => a.id === agreementId)
- )
+ const partnerAndAgreement = computed(() => {
+ for (const p of partnerStore.partners) {
+ const match = p.legalEntity?.agreements?.find(a => a.id === agreementId)
+ if (match) return { partner: p, agreement: match }
+ }
+ return { partner: null, agreement: null }
+ })
+ const partner = computed(() => partnerAndAgreement.value.partner)
+ const agreement = computed(() => partnerAndAgreement.value.agreement)- Everywhere you iterate or access
.agreements, default to an empty array:
- partner?.legalEntity?.agreements
+ partner?.legalEntity?.agreements ?? []This ensures you never call .length, .find, .some, .toSorted, or v-for on undefined.
📝 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 partner = computed(() => partnerStore.partners.find((partner) => partner.legalEntity?.agreements.some((agreement) => agreement.id === agreementId))) | |
| const agreement = computed(() => partner.value?.legalEntity?.agreements.find((agreement) => agreement.id === agreementId)) | |
| const partnerAndAgreement = computed(() => { | |
| for (const p of partnerStore.partners) { | |
| const match = p.legalEntity?.agreements?.find(a => a.id === agreementId) | |
| if (match) return { partner: p, agreement: match } | |
| } | |
| return { partner: null, agreement: null } | |
| }) | |
| const partner = computed(() => partnerAndAgreement.value.partner) | |
| const agreement = computed(() => partnerAndAgreement.value.agreement) |
🤖 Prompt for AI Agents
In apps/web-app/app/components/form/UpdatePartnerAgreement.vue lines 91-92,
replace the two separate find/some calls with a single iteration that locates
the partner and agreement together in one pass, and add a fallback for when no
match is found to avoid undefined errors. Also, throughout the app wherever
agreements are accessed or iterated, ensure to default to an empty array if
agreements is undefined, so methods like length, find, some, toSorted, or v-for
do not throw errors.
|
|
||
| const { imagesMode } = useApp() | ||
|
|
||
| const minimalAgreement = computed(() => partner.legalEntity?.agreements.toSorted((a, b) => new Date(a.willEndAt ?? '').getTime() - new Date(b.willEndAt ?? '').getTime())[0]) |
There was a problem hiding this comment.
Potential runtime error with agreement sorting logic.
The minimalAgreement computation has several potential issues:
- Null date handling:
new Date(a.willEndAt ?? '')will create an invalid date ifwillEndAtis null/undefined - Empty agreements array: No handling if
agreementsis empty - Business logic: Using the earliest ending agreement might not always be the most relevant for progress display
Apply this diff to add proper error handling:
-const minimalAgreement = computed(() => partner.legalEntity?.agreements.toSorted((a, b) => new Date(a.willEndAt ?? '').getTime() - new Date(b.willEndAt ?? '').getTime())[0])
+const minimalAgreement = computed(() => {
+ const agreements = partner.legalEntity?.agreements
+ if (!agreements?.length) return null
+
+ return agreements
+ .filter(agreement => agreement.willEndAt) // Filter out agreements without end dates
+ .toSorted((a, b) => new Date(a.willEndAt!).getTime() - new Date(b.willEndAt!).getTime())[0] || null
+})📝 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 minimalAgreement = computed(() => partner.legalEntity?.agreements.toSorted((a, b) => new Date(a.willEndAt ?? '').getTime() - new Date(b.willEndAt ?? '').getTime())[0]) | |
| const minimalAgreement = computed(() => { | |
| const agreements = partner.legalEntity?.agreements | |
| if (!agreements?.length) return null | |
| return ( | |
| agreements | |
| .filter(agreement => agreement.willEndAt) // Filter out agreements without end dates | |
| .toSorted( | |
| (a, b) => | |
| new Date(a.willEndAt!).getTime() - new Date(b.willEndAt!).getTime() | |
| )[0] || null | |
| ) | |
| }) |
🤖 Prompt for AI Agents
In apps/web-app/app/components/PartnerCard.vue at line 71, the minimalAgreement
computed property can cause runtime errors due to invalid date parsing when
willEndAt is null or undefined, and it does not handle empty agreements arrays.
To fix this, first check if partner.legalEntity and its agreements array exist
and are non-empty. Then filter out agreements with invalid or missing willEndAt
dates before sorting. Finally, adjust the sorting logic to handle only valid
dates and ensure the selected agreement aligns with the intended business logic
for progress display.
| label: 'Договора', | ||
| to: `/partner/${partner.value?.id}/agreement`, | ||
| icon: 'i-lucide-scroll-text', | ||
| badge: activeAgreementTo.value, | ||
| badge: partner.value?.legalEntity?.agreements.length, | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Good refactoring to plural form and count display.
The changes appropriately reflect the shift from single to multiple agreements:
- Label changed to plural "Договора"
- Badge now shows agreement count instead of end date
However, there's a potential null safety issue with partner.value?.legalEntity?.agreements.length. If agreements is undefined, this will throw a runtime error.
- badge: partner.value?.legalEntity?.agreements.length,
+ badge: partner.value?.legalEntity?.agreements?.length ?? 0,📝 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.
| label: 'Договора', | |
| to: `/partner/${partner.value?.id}/agreement`, | |
| icon: 'i-lucide-scroll-text', | |
| badge: activeAgreementTo.value, | |
| badge: partner.value?.legalEntity?.agreements.length, | |
| }, | |
| label: 'Договора', | |
| to: `/partner/${partner.value?.id}/agreement`, | |
| icon: 'i-lucide-scroll-text', | |
| badge: partner.value?.legalEntity?.agreements?.length ?? 0, | |
| }, |
🤖 Prompt for AI Agents
In apps/web-app/app/pages/partner/[id].vue around lines 46 to 50, the badge
property accesses partner.value?.legalEntity?.agreements.length without checking
if agreements is defined, which can cause a runtime error if agreements is
undefined. Fix this by safely accessing the length using optional chaining or a
fallback, for example, replace partner.value?.legalEntity?.agreements.length
with partner.value?.legalEntity?.agreements?.length ?? 0 to ensure it does not
throw when agreements is undefined.
1 participant
Summary by CodeRabbit
New Features
Enhancements
Style
Bug Fixes
Chores