Skip to content
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

fix: Allow save expense with invalid fields after capture receipt #3308

Merged
merged 12 commits into from
Dec 9, 2024
1 change: 1 addition & 0 deletions src/app/core/services/transaction.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1100,6 +1100,7 @@ describe('TransactionService', () => {
const mockFileObject = cloneDeep(fileObjectData1);

spyOn(transactionService, 'upsert').and.returnValue(of(txnData2));
expensesService.createFromFile.and.returnValue(of({ data: [expenseData] }));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Mind-blowing test case, but let's make it more robust!

The test case looks good, but in true Rajinikanth style, let's make it even more powerful! Consider adding assertions to verify that expensesService.createFromFile was called with the correct parameters.

Add this assertion after the existing expect statements:

 transactionService.createTxnWithFiles({ ...txnData }, of(mockFileObject)).subscribe((res) => {
   expect(res).toEqual(txnData2);
   expect(transactionService.upsert).toHaveBeenCalledOnceWith({ ...txnData, file_ids: [fileObjectData1[0].id] });
+  expect(expensesService.createFromFile).toHaveBeenCalledOnceWith(mockFileObject[0].id, txnData.source);
   done();
 });

Committable suggestion skipped: line range outside the PR's diff.

transactionService.createTxnWithFiles({ ...txnData }, of(mockFileObject)).subscribe((res) => {
expect(res).toEqual(txnData2);
expect(transactionService.upsert).toHaveBeenCalledOnceWith({ ...txnData, file_ids: [fileObjectData1[0].id] });
Expand Down
36 changes: 26 additions & 10 deletions src/app/core/services/transaction.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -195,17 +195,22 @@
): Observable<Partial<Transaction>> {
return fileUploads$.pipe(
switchMap((fileObjs) => {
// txn contains only source key when capturing receipt
if (txn.hasOwnProperty('source') && Object.keys(txn).length === 1) {
const fileIds = fileObjs.map((fileObj) => fileObj.id);
if (fileIds.length > 0) {
return this.expensesService
.createFromFile(fileIds[0], txn.source)
.pipe(map((result) => this.transformExpense(result.data[0]).tx));
}
const fileIds = fileObjs.map((fileObj) => fileObj.id);
if (fileIds.length > 0) {
return this.expensesService.createFromFile(fileIds[0], txn.source).pipe(
switchMap((result) => {
// txn contains only source key when capturing receipt in bulk mode
if (txn.hasOwnProperty('source') && Object.keys(txn).length === 1) {
return of(this.transformExpense(result.data[0]).tx);
} else {
// capture receipt flow: patching the expense with any of the form values.
txn.file_ids = fileIds;
txn.id = result.data[0].id;
return this.upsert(this.cleanupExpensePayload(txn));
}
})
);
} else {
const fileIds = fileObjs.map((fileObj) => fileObj.id);
txn.file_ids = fileIds;
return this.upsert(txn);
}
})
Expand Down Expand Up @@ -919,4 +924,15 @@

return typeOrFilter;
}

// to be used only when updating created expense with form values during capture recept flow
private cleanupExpensePayload(txn: Partial<Transaction>): Partial<Transaction> {
const newTxn: Partial<Transaction> = {};
for (const key in txn) {
if (txn[key] !== null && txn[key] !== undefined) {
newTxn[key] = txn[key];

Check failure on line 933 in src/app/core/services/transaction.service.ts

View workflow job for this annotation

GitHub Actions / Run linters

Unsafe assignment of an `any` value
}
}
return newTxn;
}
}
Loading