Skip to content

PS-469 - add more logs #123

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.

Sign up for GitHub

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

Merged
vas3a merged 2 commits into from
Dec 3, 2025
Merged

PS-469 - add more logs #123

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
def6cc2
PS-469 - add more logs in admin service & payment service to track pa…
vas3a Dec 3, 2025
3f35d39
use log instead of debug for logging
vas3a Dec 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
93 changes: 91 additions & 2 deletions src/api/admin/admin.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,34 +79,58 @@ export class AdminService {

let needsReconciliation = false;
const winningsId = body.winningsId;
this.logger.log(
Copy link

@github-actions github-actions bot Dec 3, 2025

Choose a reason for hiding this comment

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

[⚠️ maintainability]
Consider using logger.info for informational messages instead of logger.log to maintain consistency and clarity in log levels.

`updateWinnings called by ${userId} for winningsId=${winningsId}`,
);
this.logger.log(`updateWinnings payload: ${JSON.stringify(body)}`);
Copy link

@github-actions github-actions bot Dec 3, 2025

Choose a reason for hiding this comment

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

[⚠️ performance]
Switching from logger.debug to logger.log may result in excessive logging in production environments. Ensure that the log level is appropriate for the environment to avoid performance issues and log noise.


try {
const payments = await this.getPaymentsByWinningsId(
winningsId,
body.paymentId,
);

this.logger.log(
Copy link

@github-actions github-actions bot Dec 3, 2025

Choose a reason for hiding this comment

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

[⚠️ maintainability]
Changing the logging level from debug to log may result in more verbose output in production environments. Ensure that this change aligns with the desired logging strategy and does not expose sensitive information or overwhelm log storage.

`Found ${payments.length} payment(s) for winningsId=${winningsId}`,
);
if (payments.length === 0) {
this.logger.warn(
`No payments found for winningsId=${winningsId}, paymentId=${body.paymentId}`,
);
throw new NotFoundException('failed to get current payments');
}

let releaseDate;
if (body.paymentStatus) {
releaseDate = await this.getPaymentReleaseDateByWinningsId(winningsId);
this.logger.log(
Copy link

@github-actions github-actions bot Dec 3, 2025

Choose a reason for hiding this comment

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

[⚠️ maintainability]
Consider using this.logger.debug instead of this.logger.log if the intention is to log detailed information primarily useful for debugging. log might be too verbose for production logs and could clutter the log files.

`Payment release date for winningsId=${winningsId}: ${releaseDate}`,
);
}

const transactions: ((
tx: Prisma.TransactionClient,
) => Promise<unknown>)[] = [];
const now = new Date().getTime();

// iterate payments and build transaction list
payments.forEach((payment) => {
this.logger.log(
Copy link

@github-actions github-actions bot Dec 3, 2025

Choose a reason for hiding this comment

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

[⚠️ maintainability]
Changing the log level from debug to log may result in more verbose output in production environments. Ensure this change aligns with the intended logging strategy and does not expose sensitive information or overwhelm log storage.

`Processing payment ${payment.payment_id} (installment ${payment.installment_number}) with current status=${payment.payment_status}`,
);

if (
payment.payment_status &&
payment.payment_status === PaymentStatus.CANCELLED
) {
this.logger.warn(
`Attempt to update cancelled payment ${payment.payment_id} — rejecting`,
);
throw new BadRequestException('cannot update cancelled winnings');
}

let version = payment.version ?? 1;
const queuedActions: string[] = [];

if (body.description) {
transactions.push((tx) =>
Expand All @@ -129,6 +153,9 @@ export class AdminService {
},
}),
);
queuedActions.push(
`update description -> "${body.description}" (version ${version})`,
);

if (payment.installment_number === 1) {
transactions.push((tx) =>
Expand All @@ -140,6 +167,7 @@ export class AdminService {
tx,
),
);
queuedActions.push('add audit for description change');
}
}

Expand Down Expand Up @@ -171,6 +199,9 @@ export class AdminService {
payment.payment_status !== PaymentStatus.ON_HOLD_ADMIN &&
payment.payment_status !== PaymentStatus.PAID
) {
this.logger.warn(
`Invalid attempt to set OWED for payment ${payment.payment_id} when not on hold admin or paid`,
);
throw new BadRequestException(
"cannot put a payment back to owed unless it is on hold by an admin, or it's been paid",
);
Expand All @@ -180,13 +211,19 @@ export class AdminService {
break;

default:
this.logger.warn(
`Invalid payment status provided: ${body.paymentStatus}`,
);
throw new BadRequestException('invalid payment status provided');
}

if (
errMessage &&
payment.payment_status === PaymentStatus.PROCESSING
) {
this.logger.warn(
`Rejected status change for ${payment.payment_id}: ${errMessage}`,
);
throw new BadRequestException(errMessage);
}

Expand All @@ -201,11 +238,17 @@ export class AdminService {
tx,
),
);
queuedActions.push(
`update status ${payment.payment_status} -> ${body.paymentStatus}`,
);

paymentStatus = body.paymentStatus as PaymentStatus;

if (body.paymentStatus === PaymentStatus.OWED) {
needsReconciliation = true;
this.logger.log(
Copy link

@github-actions github-actions bot Dec 3, 2025

Choose a reason for hiding this comment

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

[⚠️ maintainability]
Consider using this.logger.debug instead of this.logger.log for this message. Since the message is about internal state changes and not critical information, debug level might be more appropriate to avoid cluttering logs with non-essential information.

`Payment ${payment.payment_id} marked OWED; will trigger reconciliation later`,
);
}

if (payment.installment_number === 1) {
Expand All @@ -218,6 +261,7 @@ export class AdminService {
tx,
),
);
queuedActions.push('add audit for status change');
}
}

Expand All @@ -232,6 +276,9 @@ export class AdminService {
PaymentStatus.ON_HOLD_ADMIN,
].includes(paymentStatus)
) {
this.logger.warn(
`Cannot update release date for payment ${payment.payment_id} in status ${paymentStatus}`,
);
throw new BadRequestException(
`Cannot update release date for payment unless it's in one of the states: ${[
PaymentStatus.OWED,
Expand All @@ -251,6 +298,9 @@ export class AdminService {
tx,
),
);
queuedActions.push(
`update release_date ${payment.release_date?.toISOString()} -> ${newReleaseDate.toISOString()}`,
);

if (payment.installment_number === 1) {
transactions.push((tx) =>
Expand All @@ -262,6 +312,7 @@ export class AdminService {
tx,
),
);
queuedActions.push('add audit for release date change');
}
}

Expand Down Expand Up @@ -297,6 +348,10 @@ export class AdminService {
tx,
),
);

queuedActions.push(
`update amounts -> ${body.paymentAmount.toFixed(2)} (installment 1)`,
);
} else {
transactions.push((tx) =>
this.updatePaymentAmount(
Expand All @@ -310,17 +365,35 @@ export class AdminService {
tx,
),
);
queuedActions.push(
`update amounts -> total ${body.paymentAmount.toFixed(2)} (installment ${payment.installment_number})`,
);
}
}

this.logger.log(
Copy link

@github-actions github-actions bot Dec 3, 2025

Choose a reason for hiding this comment

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

[⚠️ maintainability]
Changing this.logger.debug to this.logger.log may result in a loss of granularity in log levels. If log is not equivalent to debug, consider whether this change might affect the ability to filter logs appropriately.

`Queued ${queuedActions.length} action(s) for payment ${payment.payment_id}: ${queuedActions.join(
' ; ',
)}`,
);
});

this.logger.log(
Copy link

@github-actions github-actions bot Dec 3, 2025

Choose a reason for hiding this comment

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

[⚠️ maintainability]
The change from this.logger.info to this.logger.log could affect the log level semantics. Ensure that log provides the same level of detail and filtering capability as info if that's the intended behavior.

`Executing ${transactions.length} transaction step(s) for winningsId=${winningsId}`,
);

// Run all transaction tasks in a single prisma transaction
await this.prisma.$transaction(async (tx) => {
for (const transaction of transactions) {
await transaction(tx);
for (let i = 0; i < transactions.length; i++) {
this.logger.log(`Executing transaction ${i + 1}/${transactions.length}`);
Copy link

@github-actions github-actions bot Dec 3, 2025

Choose a reason for hiding this comment

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

[⚠️ maintainability]
Switching from this.logger.debug to this.logger.log might impact the log level used for transaction execution details. Verify that this change aligns with the desired logging strategy and does not reduce the ability to filter logs by severity.

await transactions[i](tx);
}
});

this.logger.log(
Copy link

@github-actions github-actions bot Dec 3, 2025

Choose a reason for hiding this comment

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

[⚠️ maintainability]
Replacing this.logger.info with this.logger.log could alter the intended log level. Confirm that log provides the necessary level of detail and filtering as info for successful transaction execution messages.

`Successfully executed transactions for winningsId=${winningsId}`,
);

if (needsReconciliation) {
const winning = await this.prisma.winnings.findFirst({
select: {
Expand All @@ -332,16 +405,32 @@ export class AdminService {
});

if (winning?.winner_id) {
this.logger.log(
Copy link

@github-actions github-actions bot Dec 3, 2025

Choose a reason for hiding this comment

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

[⚠️ maintainability]
Consider using a more specific logging level than log, such as info or debug, to maintain consistency and clarity in log severity. This helps in filtering logs based on their importance.

`Triggering payments reconciliation for user ${winning.winner_id}`,
);
await this.paymentsService.reconcileUserPayments(winning.winner_id);
this.logger.log(
Copy link

@github-actions github-actions bot Dec 3, 2025

Choose a reason for hiding this comment

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

[⚠️ maintainability]
Consider using a more specific logging level than log, such as info or debug, to maintain consistency and clarity in log severity. This helps in filtering logs based on their importance.

`Reconciliation triggered for user ${winning.winner_id}`,
);
} else {
this.logger.warn(
`Needs reconciliation but no winner_id found for winningsId=${winningsId}`,
);
}
}

result.data = 'Successfully updated winnings';
this.logger.log(
Copy link

@github-actions github-actions bot Dec 3, 2025

Choose a reason for hiding this comment

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

[⚠️ maintainability]
Consider using this.logger.info instead of this.logger.log to maintain consistency with the logging level used throughout the codebase. This ensures that logs are appropriately categorized and filtered based on their importance.

`updateWinnings completed for winningsId=${winningsId}: ${result.data}`,
);
} catch (error) {
if (
error instanceof NotFoundException ||
error instanceof BadRequestException
) {
this.logger.warn(
`updateWinnings validation error for winningsId=${winningsId}: ${error.message}`,
);
throw error;
}
this.logger.error('Updating winnings failed', error);
Expand Down
42 changes: 38 additions & 4 deletions src/api/webhooks/trolley/handlers/tax-form.handler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,10 @@ export class TaxFormHandler {
recipient: trolley_recipient,
taxFormData: TaxFormStatusUpdatedEventData,
) {
this.logger.log(
`Processing tax form '${taxFormId}' for user '${recipient.user_id}' (recipient trolley id: '${recipient.trolley_id}')`,
);

const existingFormAssociation =
await this.prisma.user_tax_form_associations.findFirst({
where: {
Expand All @@ -71,42 +75,72 @@ export class TaxFormHandler {
},
});

if (existingFormAssociation) {
this.logger.log(
Copy link

@github-actions github-actions bot Dec 3, 2025

Choose a reason for hiding this comment

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

[⚠️ maintainability]
Changing the logging level from debug to log may result in more verbose output in production environments. Ensure that this change aligns with the logging strategy and does not expose sensitive information or overwhelm log storage.

`Found existing association id='${existingFormAssociation.id}' status='${existingFormAssociation.tax_form_status}' date_filed='${existingFormAssociation.date_filed}'`,
);
} else {
this.logger.log('No existing tax form association found');
Copy link

@github-actions github-actions bot Dec 3, 2025

Choose a reason for hiding this comment

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

[⚠️ maintainability]
Similar to the previous comment, changing the logging level from debug to log could increase log verbosity. Verify that this change is intentional and appropriate for the environment where this code will run.

}

const taxFormStatus = this.getTaxFormStatus(
existingFormAssociation,
taxFormData,
);
this.logger.log(`Determined tax form status: '${taxFormStatus}'`);

// voided forms associations are removed from DB
if (
taxFormData.status === TrolleyTaxFormStatus.Voided &&
existingFormAssociation
) {
return this.prisma.user_tax_form_associations.deleteMany({
this.logger.log(
`Tax form '${taxFormId}' marked Voided — removing association(s) for user '${recipient.user_id}'`,
);
const result = await this.prisma.user_tax_form_associations.deleteMany({
where: {
user_id: recipient.user_id,
tax_form_id: taxFormId,
},
});
this.logger.log(
`Deleted ${result.count ?? 0} association(s) for user '${recipient.user_id}' taxFormId '${taxFormId}'`,
);
return result;
}

if (!existingFormAssociation) {
return this.prisma.user_tax_form_associations.create({
this.logger.log(
`Creating tax form association for user '${recipient.user_id}' taxFormId '${taxFormId}'`,
);
const created = await this.prisma.user_tax_form_associations.create({
data: {
user_id: recipient.user_id,
tax_form_status: taxFormStatus,
date_filed: taxFormData.signedAt,
tax_form_id: taxFormId,
},
});
this.logger.log(
`Created association id='${created.id}' tax_form_status='${created.tax_form_status}'`,
);
return created;
}

return this.prisma.user_tax_form_associations.update({
where: { id: existingFormAssociation?.id },
this.logger.log(
`Updating association id='${existingFormAssociation.id}' for user '${recipient.user_id}'`,
);
const updated = await this.prisma.user_tax_form_associations.update({
where: { id: existingFormAssociation.id },
data: {
tax_form_status: taxFormStatus,
date_filed: taxFormData.signedAt,
},
});
this.logger.log(
`Updated association id='${updated.id}' tax_form_status='${updated.tax_form_status}'`,
);
return updated;
}

/**
Expand Down
Loading
Loading