Skip to content

Conversation

@shahrukh-compuco
Copy link
Contributor

@shahrukh-compuco shahrukh-compuco commented Jul 4, 2025

Overview

This PR displays the net amount as a positive value for all the payment line items always even for refund as well. This change is needed as sage50 report exports the rows in avery specific output and to denote negative amounts we have a separete column by the name of Type but the amount column for all payment rows should display a positive value.

Before

The net amount for refund lines appear as negative
before.csv

After

The net amount is always positive
after.csv

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR ensures that the net amount for all payment line items is displayed as a positive value, including for refunds.

  • Always takes the absolute value of the net amount for payment lines
  • Updates the CSV provider to use abs() on the debit total amount
  • Ensures refunds no longer appear with a negative net amount
Comments suppressed due to low confidence (3)

CRM/Financial/BAO/ExportFormat/DataProvider/Sage50CSVProvider.php:233

  • [nitpick] Add a brief comment above this line explaining that the net amount is forced positive via abs(), so maintainers understand the intent and know why negative values were removed.
    $item[self::NET_AMOUNT_LABEL] = abs($exportResultDao->debit_total_amount);

CRM/Financial/BAO/ExportFormat/DataProvider/Sage50CSVProvider.php:233

  • Add or update automated tests to verify that refund line items now output a positive net amount, ensuring this behavior is covered and preventing regressions.
    $item[self::NET_AMOUNT_LABEL] = abs($exportResultDao->debit_total_amount);

CRM/Financial/BAO/ExportFormat/DataProvider/Sage50CSVProvider.php:233

  • This change only applies abs() to debit_total_amount. If refunds use a credit amount field (e.g., credit_total_amount), consider normalizing the net amount regardless of sign by applying abs() to the overall net calculation so no case is missed.
    $item[self::NET_AMOUNT_LABEL] = abs($exportResultDao->debit_total_amount);

Copy link
Member

@erawat erawat left a comment

Choose a reason for hiding this comment

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

@shahrukh-compuco can you explain a bit why we need to display positive amount for the refund entitiy?

@shahrukh-compuco
Copy link
Contributor Author

@shahrukh-compuco can you explain a bit why we need to display positive amount for the refund entitiy?

done

@shahrukh-compuco shahrukh-compuco merged commit 6b0704a into sage50-workstream Jul 7, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants