CIVIMM-349: Add Credit Note Transaction Details To Sage50 Report #31
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the Sage50 CSV export by including credit note details—number, line item description, and financial type—whenever credit notes are present.
- Extend the SELECT query to pull credit note fields and join the relevant tables conditionally.
- Map the new credit note values into the Details, Reference, and Project Refn columns.
- Only apply the additional joins and columns when
$supportCreditNoteis enabled.
Comments suppressed due to low confidence (2)
CRM/Financial/BAO/ExportFormat/DataProvider/Sage50CSVProvider.php:255
- New behavior for credit note details should be covered by automated tests. Add or update a test case to verify that credit note descriptions override standard item descriptions in the CSV export.
$item[self::DETAILS_LABEL] = !empty($exportResultDao->credit_note_description) ? $exportResultDao->credit_note_description
CRM/Financial/BAO/ExportFormat/DataProvider/Sage50CSVProvider.php:87
- The SELECT list is missing a comma after
credit_note_financial_type, which will cause a SQL syntax error. Add a trailing comma before the FROM clause.
. ($supportCreditNote ? "ftycn.name" : "''") . " AS credit_note_financial_type
| AND (fii.financial_account_id IN ($taxAccounts))) | ||
| LEFT JOIN civicrm_line_item li ON (li.id = fi.entity_id AND fi.entity_table = 'civicrm_line_item') | ||
| " . ($supportCreditNote ? "LEFT JOIN financeextras_credit_note_line fcli ON (fcli.id = fi.entity_id AND fi.entity_table = 'financeextras_credit_note_line')" : "") . " | ||
| " . ($supportCreditNote ? "LEFT JOIN financeextras_credit_note_line fcli ON (fcli.id = fi.entity_id AND fi.entity_table = 'financeextras_credit_note_line') LEFT JOIN financeextras_credit_note fcn ON (fcli.credit_note_id = fcn.id) LEFT JOIN civicrm_financial_type ftycn ON fcli.financial_type_id = ftycn.id" : "") . " |
There was a problem hiding this comment.
[nitpick] Combining multiple JOIN clauses into one long concatenated string reduces readability. Consider splitting each JOIN into its own concatenated segment or formatting them on separate lines.
Member
@shahrukh-compuco can u check the copilot comment especially. |
Contributor
Author
@erawat I think the suggestion is wrong as adding a comma before from will cause the syntax error |
erawat
approved these changes
Jul 2, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
This PR adds credit note details to the sage 50 report, mainly it adds following three files as currently these fields are empty for credit note transactions
Before
before.csv
After
after.csv