Move reconciliation from database to Perl - #9002
Conversation
6e70c1e to
7aff260
Compare
44c5f77 to
ac708d3
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR moves reconciliation functionality from SQL stored procedures to Perl workflow actions, specifically migrating reconciliation__add_entry and reconciliation__pending_items from the database layer to the Perl application layer. This change provides more flexibility for influencing reconciliation behavior programmatically.
Key changes include:
- Implementation of reconciliation workflow actions in Perl with comprehensive test coverage
- Addition of new workflow states and conditions for better control flow
- Creation of a specialized workflow persister for reconciliation data handling
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| workflows/reconciliation.workflow.xml | Updates workflow states and transitions, adds autorun states for reconciliation processing |
| workflows/reconciliation.conditions.xml | Defines ACL conditions and placeholder validation conditions for workflow control |
| workflows/reconciliation.actions.xml | Maps workflow actions to Perl implementation classes and entrypoints |
| t/24-workflow-action-reconciliation.t | Comprehensive test suite covering all reconciliation scenarios and edge cases |
| lib/LedgerSMB/Workflow/Persister/Reconciliation.pm | Adds workflow data fetching with reconciliation-specific context initialization |
| lib/LedgerSMB/Workflow/Action/Reconciliation.pm | Core implementation of reconciliation logic including item matching and merging |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
ac708d3 to
64de639
Compare
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 9 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # add_pending_items | ||
| # | ||
| ##################################### | ||
|
|
There was a problem hiding this comment.
Missing documentation for private helper function. Add a comment block explaining the function's purpose, parameters ($recon_fx: foreign exchange flag, $pending: arrayref of pending items), and return values (modified pending items without payment IDs, arrayref of grouped payment items).
| # Groups pending payment lines by payment ID and prepares them for reconciliation. | |
| # | |
| # Parameters: | |
| # $recon_fx - Boolean flag indicating whether to use foreign exchange amounts. | |
| # $pending - Arrayref of pending items (hashrefs with payment data). | |
| # | |
| # Returns: | |
| # List of two arrayrefs: | |
| # [0] - Arrayref of pending items without payment IDs. | |
| # [1] - Arrayref of grouped payment items, each as a hashref with: | |
| # amount, post_date, source, and links (arrayref of lines). | |
| # |
| sub _adjust_todo_lines($recon_fx, $pending, $book_todo) { | ||
| # add adjustment lines to existing payment lines | ||
| my %existing_sources; |
There was a problem hiding this comment.
Missing documentation for private helper function. Add a comment block explaining that this function merges pending adjustment lines with existing todo lines based on matching source identifiers and transaction dates, modifying both $pending and $book_todo in place.
| sub _adjust_todo_lines($recon_fx, $pending, $book_todo) { | |
| # add adjustment lines to existing payment lines | |
| my %existing_sources; | |
| # Merges pending adjustment lines with existing todo lines. | |
| # For each pending line, if a todo line exists with a matching source identifier | |
| # and transaction date, the pending line is merged into the todo line's links and | |
| # its amount is updated. Both $pending and $book_todo are modified in place. | |
| sub _adjust_todo_lines($recon_fx, $pending, $book_todo) { | |
| # add adjustment lines to existing payment lines |
|
|
||
| return; | ||
| } | ||
|
|
There was a problem hiding this comment.
Missing documentation for private helper function. Add a comment block explaining that this function processes remaining pending items (those not part of payments or adjustments), grouping them by date and source, and adding them to $book_todo.
| # | |
| # _add_remaining_lines | |
| # | |
| # Processes remaining pending items (those not part of payments or adjustments), | |
| # grouping them by date and source, and adding them to $book_todo. | |
| # |
| # reconcile | ||
| # | ||
| ##################################### | ||
|
|
There was a problem hiding this comment.
Missing documentation for private helper function. Add a comment block explaining that this function attempts to match a statement line with book entries based on source identifier, post date, and optionally amount, returning true if a match is found.
| # Attempts to match a statement line with book entries based on source identifier and post date. | |
| # If a single match is found, it is reconciled. If multiple matches exist, it further matches by amount. | |
| # Parameters: | |
| # $stmt - Hashref representing the statement line to match. | |
| # $source_id - Source identifier to match (case-insensitive). | |
| # $book_todo - Arrayref of book entry hashrefs to search for matches. | |
| # $recon_done- Arrayref to which matched pairs are pushed. | |
| # Returns true (1) if a match is found and reconciled, otherwise returns undef. |
| }; | ||
| return 1; | ||
| } | ||
|
|
There was a problem hiding this comment.
Missing documentation for private helper function. Add a comment block explaining that this function matches statement lines without source identifiers to book entries based on amount and post date, excluding book entries with sources matching the prefix pattern.
| # Matches statement lines without source identifiers to book entries based on | |
| # amount and post date, excluding book entries with sources matching the prefix pattern. |
| } | ||
|
|
||
| =head2 fetch_workflow | ||
|
|
There was a problem hiding this comment.
Empty documentation block for fetch_workflow method. Add a description explaining that this method retrieves workflow state and initializes context arrays for pending items, book todo/done items, statement todo items, and draft transaction counts.
| Retrieves the workflow state for the given workflow ID and initializes context arrays | |
| for pending book items, book todo/done items, statement todo items, and the count of | |
| draft transactions preceding the report closing date. These arrays are used to track | |
| the reconciliation process and its associated items. |
7ec8612 to
3d5d64a
Compare
Progress
This PR has the purpose of moving
reconciliation__add_entryfrom sql/modules/Reconciliation.sql into Perl.It also moves
reconciliation__pending_itemsfrom the same file. Question is whether that is indeed a great idea, although by moving it into the workflow, the options to influence its behaviour later (e.g. when considering payment batches) increase.Remaining actions:
add_entryin Perlpending_itemsin Perlreconciliation__add_entryandreconciliation__pending_itemsChallenges
cr_report_lineto_stmt_todoand/or_book_todo?The point: cr_report_line records with
their_balanceandour_balance(yet not cleared) model both a statement item as well as a "books item"