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

Transaction reversal tracking #6715

Merged
merged 17 commits into from
Apr 29, 2023

Conversation

ehuelsmann
Copy link
Member

@ehuelsmann ehuelsmann commented Aug 9, 2022

The reversal of the reversed transaction is to be exact (which is what #2321 says is currently not happening). Part of this has been fixed earlier, but taxes are still being redone on reversal (for invoices), which means that the transactions cannot be exact before this PR.

Additionally, in order to be able to track what happened to transactions, it's important to see relations between transactions as they are created. To that purpose, this PR creates infrastructure to track which transactions or invoices have been reversed (voided) and which transactions are the reversing(voiding) ones. It also prevents a transaction from being voided twice.

The following functionalities are planned for this PR:

Item AR AP GL IS IR
Tracking reversals 1 ✔️ ✔️ ✔️ ✔️ ✔️
Prevent duplicate reversals (shared) ✔️ ✔️ ✔️ ✔️ ✔️
Read-only UI on reversals ✔️ ✔️ ✔️ ✔️ ✔️
Editable headers only (reference/invnumber) ✔️ ✔️ ✔️ ✔️ ✔️
Editable headers only (sequence) ✔️ ✔️ ✔️ ✔️ ✔️
Editable headers only (date[s]) ✔️ ✔️ ✔️ ✔️ ✔️
Editable headers only (description) ✔️ ✔️ ✔️ ✔️ ✔️
Editable headers only (notes) ✔️ ✔️ ✔️ ✔️ ✔️
Editable headers only (internal notes) ✔️ ✔️ 2 ✔️ ✔️

Additional functionality implemented in this PR:

  • Searching for reversed and voided GL/AR/AP transactions and AR/AP invoices

This change adds tracking to transactions like this:

2023-04-22_22-48

Footnotes

  1. A problem is that the triggering workflow advances to the next state (reversed/voided) even when the replacement transaction hasn't been posted yet. We need a mechanism to advance the transaction to VOIDED / REVERSED at the same time the reversal is posted. Until that happens, it'd be nice to help the user by listing the transaction as "in the process of being reversed.

  2. Not applicable

@ehuelsmann ehuelsmann force-pushed the features/reversed-transactions branch 2 times, most recently from 2b286f0 to dac1740 Compare August 9, 2022 21:12
@ehuelsmann ehuelsmann changed the title Transaction reversal tracking and reversal exactness Transaction reversal tracking Aug 10, 2022
@ehuelsmann
Copy link
Member Author

Since the fix for #2321 has been released for 1.7, 1.8 and 1.9, the 'exact reversal' goal of this PR has been resolved. This PR was mostly initiated to address that problem (and improve on the situation of reversal tracking while at it).

@ehuelsmann ehuelsmann force-pushed the features/reversed-transactions branch from dac1740 to 9031a0f Compare August 28, 2022 21:21
old/bin/gl.pl Outdated Show resolved Hide resolved
old/bin/aa.pl Outdated Show resolved Hide resolved
&create_links; # create_links overwrites 'reversing'
};

AA->post_transaction( \%myconfig, \%$form );
Copy link
Member Author

Choose a reason for hiding this comment

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

The unfortunate side-effect of this choice is that the transaction goes through all the logic of loading and posting a transaction instead of copying the original transaction and negating its effect. I'm looking for ideas on how to bypass the loading and saving to prevent "damage" to the exact (negated) copy.

@ehuelsmann ehuelsmann force-pushed the features/reversed-transactions branch 2 times, most recently from cddae6d to 682b80c Compare October 13, 2022 20:14
transdate = ?,
notes = ?
WHERE id = ?
QUERY
Copy link
Member Author

Choose a reason for hiding this comment

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

What's with this query? It's not being used?!

@ehuelsmann ehuelsmann force-pushed the features/reversed-transactions branch from 61ae406 to 3b80297 Compare November 2, 2022 10:44
@ehuelsmann ehuelsmann force-pushed the features/reversed-transactions branch from deba61d to cff626c Compare February 1, 2023 21:36
@ehuelsmann ehuelsmann force-pushed the features/reversed-transactions branch from f5d09dc to 4910520 Compare March 7, 2023 22:36
@ehuelsmann ehuelsmann force-pushed the features/reversed-transactions branch from 4910520 to d40160d Compare April 4, 2023 20:43
@ehuelsmann ehuelsmann force-pushed the features/reversed-transactions branch 2 times, most recently from a8bb968 to 3bbb8ec Compare April 22, 2023 21:12
@ehuelsmann ehuelsmann marked this pull request as ready for review April 22, 2023 22:01
@ehuelsmann
Copy link
Member Author

@christian-eriksson, @neilt, I can't add you to the list of reviewers, but your comments regarding this PR are highly appreciated!

@ehuelsmann
Copy link
Member Author

@neilt notes:

ehuelsmann: I tested GL and the only anomaly I found was this:

  1. Enter Journal Entry
  2. Do Transaction Approval
  3. Search for Transaction
  4. Reverse Transaction
  5. Done

Why when I have "Separate Duties" turned on do I have to approve the original GL Journal Entry, but the reversal happens immediately and does not have to be Approved?

@neilt
Copy link
Contributor

neilt commented Apr 24, 2023

When I try to reverse a AR Invoice with both a service and a part I get the following error.

Screenshot 2023-04-24 at 9 36 28 AM

I assume that it is the service that is causing the problem, but can't tell from the error message.

But it seems to have worked anyway, which was unexpected. Here is the completed reversal screen:
Screenshot 2023-04-24 at 9 43 33 AM

@neilt
Copy link
Contributor

neilt commented Apr 24, 2023

On the AR and AP search screen I would like to see a checkbox for Voided Invoices.

Screenshot 2023-04-24 at 9 46 29 AM

Not sure which lines it goes on (status or include in report). But I think being able to pull out recent voided invoices would be useful.

@ehuelsmann ehuelsmann force-pushed the features/reversed-transactions branch from c6fd6e5 to 161c747 Compare April 25, 2023 16:18
@ehuelsmann
Copy link
Member Author

@neilt notes:

ehuelsmann: I tested GL and the only anomaly I found was this:

1. Enter Journal Entry

2. Do Transaction Approval

3. Search for Transaction

4. Reverse Transaction

5. Done

Why when I have "Separate Duties" turned on do I have to approve the original GL Journal Entry, but the reversal happens immediately and does not have to be Approved?

I'm unable to reproduce. Hoping that means it's no longer a problem... Can't fix it without reproduction recipe, is my main point though.

@ehuelsmann
Copy link
Member Author

Not sure which lines it goes on (status or include in report). But I think being able to pull out recent voided invoices would be useful.

"Include in" is an output column with no influence on the selection. Your description suggests it's about filtering, which means it should go in the status column.

@ehuelsmann ehuelsmann force-pushed the features/reversed-transactions branch from 5fe3704 to 721ca32 Compare April 29, 2023 08:13
@ehuelsmann
Copy link
Member Author

When I try to reverse a AR Invoice with both a service and a part I get the following error.

Hmm. I'm unable to reproduce. Neither on master nor on the branch... I guess I'll need help or we need to declare it a heisenbug.

While at it, harmonize the AR/AP and GL search pages.
@ehuelsmann ehuelsmann force-pushed the features/reversed-transactions branch from 721ca32 to a34e60a Compare April 29, 2023 18:16
@neilt
Copy link
Contributor

neilt commented Apr 29, 2023

I'm fine with closing this PR. I'll open an issue if I can recreate the issue. I don't time to test it again right now.

@ehuelsmann ehuelsmann merged commit cdd1334 into ledgersmb:master Apr 29, 2023
@ehuelsmann ehuelsmann deleted the features/reversed-transactions branch April 29, 2023 21:31
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.

None yet

2 participants