Browse Source

fix: address PR review comments from copilot review

- Wrap KDocument write ops (update/supersede/createMany) in $transaction
- Fix BOX_KEY_TO_K1DATA_FIELD: 4c → 4, L_CAPITAL_CONTRIBUTED → L_CONTRIBUTED
- Fix customLabel logic: use ?? instead of ternary to preserve override value
- Fix getSectionLabel() to match actual K1BoxSection enum values from backend
- Replace $executeRawUnsafe with $executeRaw; fix total_amount type to string
- Remove agg.breakdown template refs to match simplified {name,value} type

Co-authored-by: RobertgPatch <5817970+RobertgPatch@users.noreply.github.com>
Agent-Logs-Url: https://github.com/RobertgPatch/portfolio-management/sessions/784d8100-7260-4f87-ac51-662285407c97
pull/6701/head
copilot-swe-agent[bot] 2 months ago
parent
commit
77dfde34b9
  1. 2
      apps/api/src/app/k1-import/k1-field-mapper.service.ts
  2. 125
      apps/api/src/app/k1-import/k1-import.service.ts
  3. 6
      apps/api/src/app/k1-import/k1-materialized-view.service.ts
  4. 7
      apps/client/src/app/pages/cell-mapping/cell-mapping-page.component.ts
  5. 10
      apps/client/src/app/pages/k-documents/k-document-detail/k-document-detail.html

2
apps/api/src/app/k1-import/k1-field-mapper.service.ts

@ -42,7 +42,7 @@ export class K1FieldMapperService {
mappedFields.push({ mappedFields.push({
...field, ...field,
label: def.label, label: def.label,
customLabel: def.customLabel ? def.label : field.customLabel, customLabel: def.customLabel ?? field.customLabel,
cellType: def.dataType cellType: def.dataType
} as any); } as any);
} else { } else {

125
apps/api/src/app/k1-import/k1-import.service.ts

@ -712,9 +712,9 @@ export class K1ImportService {
'1': 'ordinaryIncome', '1': 'ordinaryIncome',
'2': 'netRentalIncome', '2': 'netRentalIncome',
'3': 'otherRentalIncome', '3': 'otherRentalIncome',
'4a': 'guaranteedPayments', // 4a guaranteed payments (services) '4': 'guaranteedPayments', // 4 guaranteed payments (services)
'4b': 'guaranteedPayments', // 4b guaranteed payments (capital) — merged '4a': 'guaranteedPayments', // 4a guaranteed payments (capital) — merged
'4c': 'guaranteedPayments', // 4c total guaranteed payments '4b': 'guaranteedPayments', // 4b total guaranteed payments — merged
'5': 'interestIncome', '5': 'interestIncome',
'6a': 'dividends', '6a': 'dividends',
'6b': 'qualifiedDividends', '6b': 'qualifiedDividends',
@ -734,7 +734,7 @@ export class K1ImportService {
'21': 'foreignTaxesPaid', '21': 'foreignTaxesPaid',
// Section L: Tax basis / capital account fields // Section L: Tax basis / capital account fields
'L_BEG_CAPITAL': 'beginningTaxBasis', 'L_BEG_CAPITAL': 'beginningTaxBasis',
'L_CAPITAL_CONTRIBUTED': 'k1CapitalAccount', 'L_CONTRIBUTED': 'k1CapitalAccount',
'L_END_CAPITAL': 'endingTaxBasis', 'L_END_CAPITAL': 'endingTaxBasis',
}; };
@ -753,69 +753,74 @@ export class K1ImportService {
// Merge named fields into the document data (named fields take precedence for views) // Merge named fields into the document data (named fields take precedence for views)
const finalDocumentData = { ...kDocumentData, ...k1DataSnapshot }; const finalDocumentData = { ...kDocumentData, ...k1DataSnapshot };
// FR-012: Create or update KDocument // FR-012: Create or update KDocument with K1LineItems in a single transaction
// to ensure consistent state (no superseded-but-unreplaced line items).
let kDocument; let kDocument;
if (existingKDocument && data.existingKDocumentAction === 'UPDATE') { await this.prismaService.$transaction(async (tx) => {
kDocument = await this.prismaService.kDocument.update({ if (existingKDocument && data.existingKDocumentAction === 'UPDATE') {
where: { id: existingKDocument.id }, kDocument = await tx.kDocument.update({
data: { where: { id: existingKDocument.id },
filingStatus: data.filingStatus, data: {
data: finalDocumentData as any, filingStatus: data.filingStatus,
documentFileId: session.documentId data: finalDocumentData as any,
documentFileId: session.documentId
}
});
// FR-016: Mark existing active K1LineItems as superseded (ESTIMATED→FINAL)
await tx.k1LineItem.updateMany({
where: {
kDocumentId: existingKDocument.id,
isSuperseded: false
},
data: { isSuperseded: true }
});
} else {
// CREATE_NEW or no existing document
if (existingKDocument && data.existingKDocumentAction === 'CREATE_NEW') {
// Delete existing unique constraint holder to create new
await tx.kDocument.delete({
where: { id: existingKDocument.id }
});
} }
});
// FR-016: Mark existing active K1LineItems as superseded (ESTIMATED→FINAL) kDocument = await tx.kDocument.create({
await this.prismaService.k1LineItem.updateMany({ data: {
where: { partnershipId: session.partnershipId,
kDocumentId: existingKDocument.id, type: 'K1',
isSuperseded: false taxYear: session.taxYear,
}, filingStatus: data.filingStatus,
data: { isSuperseded: true } data: finalDocumentData as any,
}); documentFileId: session.documentId
} else { }
// CREATE_NEW or no existing document
if (existingKDocument && data.existingKDocumentAction === 'CREATE_NEW') {
// Delete existing unique constraint holder to create new
await this.prismaService.kDocument.delete({
where: { id: existingKDocument.id }
}); });
} }
kDocument = await this.prismaService.kDocument.create({ // Create K1LineItem rows (the authoritative normalized data)
data: { if (lineItemsToCreate.length > 0) {
partnershipId: session.partnershipId, await tx.k1LineItem.createMany({
type: 'K1', data: lineItemsToCreate.map((item) => ({
taxYear: session.taxYear, kDocumentId: kDocument.id,
filingStatus: data.filingStatus, boxKey: item.boxKey,
data: finalDocumentData as any, amount: item.amount,
documentFileId: session.documentId textValue: item.textValue,
} rawText: item.rawText,
}); confidence: item.confidence,
} sourcePage: item.sourcePage,
sourceCoords: item.sourceCoords,
// Create K1LineItem rows (the authoritative normalized data) isUserEdited: item.isUserEdited,
if (lineItemsToCreate.length > 0) { isSuperseded: false
await this.prismaService.k1LineItem.createMany({ }))
data: lineItemsToCreate.map((item) => ({ });
kDocumentId: kDocument.id,
boxKey: item.boxKey,
amount: item.amount,
textValue: item.textValue,
rawText: item.rawText,
confidence: item.confidence,
sourcePage: item.sourcePage,
sourceCoords: item.sourceCoords,
isUserEdited: item.isUserEdited,
isSuperseded: false
}))
});
this.logger.log( this.logger.log(
`Session ${sessionId}: Created ${lineItemsToCreate.length} K1LineItem rows for KDocument ${kDocument.id}` `Session ${sessionId}: Created ${lineItemsToCreate.length} K1LineItem rows for KDocument ${kDocument.id}`
); );
}
});
// Emit event to refresh materialized views (US5) // Emit event after the transaction commits to refresh materialized views (US5)
if (kDocument && lineItemsToCreate.length > 0) {
this.eventEmitter.emit('k-document.changed', { this.eventEmitter.emit('k-document.changed', {
kDocumentId: kDocument.id, kDocumentId: kDocument.id,
partnershipId: kDocument.partnershipId partnershipId: kDocument.partnershipId
@ -886,7 +891,7 @@ export class K1ImportService {
// This drives Portfolio Summary (DPI/RVPI/TVPI) computations // This drives Portfolio Summary (DPI/RVPI/TVPI) computations
const sectionLData = { const sectionLData = {
beginningCapital: boxValues['L_BEG_CAPITAL'] ?? null, beginningCapital: boxValues['L_BEG_CAPITAL'] ?? null,
capitalContributed: boxValues['L_CAPITAL_CONTRIBUTED'] ?? null, capitalContributed: boxValues['L_CONTRIBUTED'] ?? null,
endingCapital: boxValues['L_END_CAPITAL'] ?? null, endingCapital: boxValues['L_END_CAPITAL'] ?? null,
withdrawals: boxValues['L_WITHDRAWALS'] ?? null withdrawals: boxValues['L_WITHDRAWALS'] ?? null
}; };

6
apps/api/src/app/k1-import/k1-materialized-view.service.ts

@ -22,9 +22,7 @@ export class K1MaterializedViewService {
async refreshAll() { async refreshAll() {
try { try {
await this.prismaService.$executeRawUnsafe( await this.prismaService.$executeRaw`REFRESH MATERIALIZED VIEW CONCURRENTLY mv_k1_partnership_year_summary`;
`REFRESH MATERIALIZED VIEW CONCURRENTLY mv_k1_partnership_year_summary`
);
this.logger.log('Materialized view mv_k1_partnership_year_summary refreshed.'); this.logger.log('Materialized view mv_k1_partnership_year_summary refreshed.');
} catch (error) { } catch (error) {
this.logger.error( this.logger.error(
@ -45,7 +43,7 @@ export class K1MaterializedViewService {
box_key: string; box_key: string;
label: string; label: string;
section: string | null; section: string | null;
total_amount: number | null; total_amount: string | null;
line_count: bigint; line_count: bigint;
}> }>
> { > {

7
apps/client/src/app/pages/cell-mapping/cell-mapping-page.component.ts

@ -105,10 +105,9 @@ export class CellMappingPageComponent implements OnInit {
SECTION_J: 'Section J', SECTION_J: 'Section J',
SECTION_K: 'Section K', SECTION_K: 'Section K',
SECTION_L: 'Section L', SECTION_L: 'Section L',
SECTION_M_N: 'Sections M & N', SECTION_M: 'Section M',
PART_III_A: 'Part III (Income)', SECTION_N: 'Section N',
PART_III_B: 'Part III (Deductions)', PART_III: 'Part III'
PART_III_C: 'Part III (Other)'
}; };
return map[section] ?? section; return map[section] ?? section;
} }

10
apps/client/src/app/pages/k-documents/k-document-detail/k-document-detail.html

@ -54,16 +54,6 @@
<div class="aggregation-value"> <div class="aggregation-value">
{{ agg.value | currency:'USD':'symbol':'1.2-6' }} {{ agg.value | currency:'USD':'symbol':'1.2-6' }}
</div> </div>
@if (agg.breakdown && agg.breakdown.length > 0) {
<div class="breakdown">
@for (item of agg.breakdown; track item.boxNumber) {
<div class="breakdown-row">
<span class="box-label">Box {{ item.boxNumber }}:</span>
<span class="box-value">{{ item.value | currency:'USD':'symbol':'1.2-6' }}</span>
</div>
}
</div>
}
</mat-card-content> </mat-card-content>
</mat-card> </mat-card>
} }

Loading…
Cancel
Save