GEWIS / sudosos-backend

SudoSOS is a Node.js-based Bar and POS system made for study association GEWIS.
https://sudosos.gewis.nl
GNU Affero General Public License v3.0
4 stars 3 forks source link

[Refactor] Major invoice changes: view invoices as deposits and replace credit invoices with SellerPayouts #260

Closed Yoronex closed 1 week ago

Yoronex commented 4 weeks ago

What would you like?

Over the last few days/weeks, multiple issues with the existing invoice implementation came to light, like https://github.com/GEWIS/sudosos-backend/issues/255, https://github.com/GEWIS/sudosos-backend/issues/249. Furthermore, automatic invoices (https://github.com/GEWIS/sudosos-backend/issues/197) were one of the desired features to implement over the summer, yet they were not so trivial to implement (https://github.com/GEWIS/sudosos-backend/issues/197#issuecomment-2258780616). Even then, the invoice implementation already started to crack.

To clearly distinguish SudoSOS balance from BAC bank balance, hereafter I will use "SudoSOS Coin" as the currency of SudoSOS.

Problem

All these issues with invoices boil down to two major design flaws:

  1. Even though it was one of the design philosophies of SudoSOS, SudoSOS Coin is not always transfered between two users. For transactions this is the case: the user trades SudoSOS Coin for physical goods. However, to get SudoSOS Coin, the user has to spend EUR. These euros are send to the BAC bank account and in exchange, the user gets SudoSOS Coin. You would expect that the SudoSOS Coin is then taken from the BAC and placed into the user's account, but this is not the case. With the current implementation, these SudoSOS Coins are taken from the void, i.e. created out of nothing. All transfers behave this way (as a SudoSOS Coin exchange from/to the void), except for one, namely invoices. These are always an exchange of SudoSOS Coin between two users. This creates an edge case that has to be handled correctly in for example the transaction reports.
  2. "Credit" invoices are not actually credit invoices. They are used as an overview of all sold products within a time frame, which the BAC uses to get SudoSOS Coin out of SudoSOS for sellers. For these "invoices" another edge case is created, namely that "credit" invoices also only exchange with the void (and not with another user like with regular invoices). This causes problems https://github.com/GEWIS/sudosos-backend/issues/249#issuecomment-2282294319.

The whole thing is that invoices are handled as transactions out of SudoSOS, even though they are registered within SudoSOS. The idea was that the sold products are paid for via the BAC bank account and not SudoSOS, so they should not be included in the SudoSOS overviews. However, this edge case adds a lot of complexity.

Solution

After a long, long discussion yesterday evening, we settled on a solution that could greatly reduce the complexity of the invoicing module and also make it more robust for new additions like automatic invoices. For this, we will solve the aforementioned two problems.

Invoices as deposits

We need to view invoices as deposits, as this removes the aforementioned edge case. The change is relatively easy, as we only need to remove the opposite side of the transfer (in other words, the user should receive money from the void instead of a seller). However, we need to be cautious with the actual invoicing and Multivers bookkeeping, because it is easy to now sell products twice. If we get rid of the invoices-are-not-SudoSOS-transactions, they are included in the sales reports. However, they will also be listed in the invoice a user receives. This can be fixed by the BAC treasurer viewing the invoice as a deposit and not as an "actual" payment. Then, this bank transaction should be included in the pile of Stripe deposits. Money is now correctly put in SudoSOS. In case of custom invoice entries (like "Tappersvergoeding"), we need to explicitly separate these from SudoSOS products on the invoice, as these products should be handled like actual sold products (with VAT and all that related stuff).

If SudoSOS Coin is no longer taken from the seller by the invoice, this means that sellers will suddenly have a lot more balance. We can rectify this during migration by manually creating a transfer with the same amount of SudoSOS Coin that has been added.

Introduce SellerPayouts

Right now, we use "credit" invoices as a means to get SudoSOS Coin out of SudoSOS and onto a bank account (like that of GEWIS when I.V.V for example sells their wines). It is nothing more than a list of everything that is sold within a given timespan and what the profits were (in SudoSOS Coin). Then, this total amount of SudoSOS Coin is taken from the account and an equal amount of EUR is transferred to a different bank account or put into the books as sales. However, this looks a lot like the PayoutRequests we already know and love. However, instead of a regular SudoSOS user requesting its remaining balance back, it's an organ who is requesting all its balance back.

We do not want to explicitly link transactions to such a SellerPayout, because that seems stupid and very repetitive. To simplify the process, we can just use the createdAt or updatedAt attributes of transactions to see when they were created. For SellerPayouts, we then also track the timespan this payout should cover. A transfer is then created for the total value of all these transactions within the timespan. The transaction report of the given timespan is then used by the BAC treasurer for the Multivers bookkeeping.

However, at SudoSOS we are great at making changes to the past, for example by deleting a transaction that should not have existed, or changing for whom the transaction was made (because names are hard). By using these timespans, these cases are also covered. Take this example. The BAC treasurer creates a SellerPayout for month x. The balance on the BAC account should now be zero at the end of month x. Then, stuff happens and a transaction is deleted in month x. The BAC treasurer unfortunately forgets about updating the SellerPayout. But, when a SellerPayout is now created for month x + 1, the balance is not zero at the end of month x + 1. Something has changed in the past! We should explicitly require the BAC treasurer to manually update the SellerPayout with the correct amount taken from the transaction report, as this forces the BAC treasurer to notice these changes and thus also apply them in Multivers.

Consequences of these changes

Unfortunately, these design changes will not go without any consequences. The biggest problem is that this "new" implementation is not backwards compatible with the "old" design, as transaction reports will now include invoice transactions while this was not the case before. We prefer to not maintain two implementations and also to not completely redo the bookkeeping of 2022-2023 and 2023-2024, so the latter should be finished and closed as soon as possible. @JustSamuel and I prefer to use this new design as soon as possible, so the treasurer of 2024-2025 does not have to switch bookkeeping methodology during the year.

Furthermore, these changes are quite drastic, as they fundamentally change some important workflows of SudoSOS. Therefore, rigorous manual testing (especially around the migrations) is required.

Why is this needed?

No response

How could it be implemented?

No response

Other information

No response

JustSamuel commented 4 weeks ago

The Zettle-like Solution

After giving it some thought, I believe I have a description, rationale, and explanation that should summarize all the changes and make it extremely transparent why these changes exist and how they will be implemented in the bookkeeping and in SudoSOS. This should also clarify the previously mentioned points in the issue.

The solution, like most effective solutions, is inspired by real-world working examples. In this case, I take Zettle as inspiration, but the same principles would apply to Stripe or any other payment provider.

SudoSOS Coins

As stated in the description, the balance on SudoSOS should be considered its own currency, SudoSOS Coins. You buy SudoSOS Coins with money and 'top-up' your balance with them. These coins function like 'multi-purpose' vouchers (MPV).

A MPV is not taxable upon issuance and (re)sale, however, it is taxable upon redemption.

This is already implemented when we look at the UserPayouts. A user can exchange their coins back for money, and no VAT will apply since the MPV was not redeemed.

Sellers

Sellers in SudoSOS are entities (BAC, I.V.V, B.O.O.M., etc.) that offer goods that users can buy using SudoSOS Coins. When a user buys from a seller, the MPV is redeemed.

This is where the Zettle analogy comes into play. SudoSOS does not make any sales itself; sellers do. It only keeps track of SudoSOS Coins and how they are redeemed. Similarly, Zettle does not make any sales when GEWIS sells ties; they only keep track of how many ties were sold. Here, SudoSOS acts as a middleman, and the MPV (SudoSOS Coins) is redeemed at the seller. In other words:

When a user buys goods from a seller, they are redeeming their MPV with the seller, not with SudoSOS.

Seller Payouts

This is where the reasoning slightly changes from the issue description. Instead of the following:

However, instead of a regular SudoSOS user requesting its remaining balance back, it's an organ that is requesting all its balance back

Consider Seller Payouts as an overview of the amount of SudoSOS Coins redeemed at a seller within a given time span, similar to a Zettle Sales Report. (I won't insert an actual overview since @tomudding and GDPR will kill me).

On the first page of a Zettle report, it shows the amount of money being transferred to the seller. This is not a VAT-liable transaction; neither the seller nor Zettle pays taxes on the actual transferring of the stored balance. On the second page of the Zettle report, it lists what the seller has actually sold, which is VAT-liable. The seller then inserts this in their bookkeeping and pays taxes on the sales.

This is also how we should consider Seller Payouts: a payout consists of the exchange of SudoSOS Coins into euros (not VAT-liable) and an overview of the MPV redeemed at the seller (VAT-liable). Essentially, a seller does not request a payout so much as they are given a payout. A SudoSOS Seller Payout is nothing more than a Zettle Sales Overview.

Thus, the first page of a SudoSOS Seller Payout should state the amount of transferred cash, and the second should provide an overview of the sold products.

The BAC Account in SudoSOS is Just Another Regular SudoSOS Seller

A side note that helped me clear some confusion is to consider the BAC account in SudoSOS no different than any other seller account. The only tricky part is that the bookkeeping of SudoSOS as a "company" is the same as that of the BAC. However, if you imagine that SudoSOS had its own separate bank account, it becomes easier to see how a Seller Payout would work for the BAC.

The BAC receives a SudoSOS Seller Payout and records it as sales in the BAC bookkeeping. At the same time, SudoSOS will transfer the money to the BAC bank account, which will then be matched in the BAC bookkeeping to the Seller Payout. In the SudoSOS bookkeeping, this transaction would not be VAT-liable as SudoSOS itself did not make any sales.

Therefore, the BAC will receive SudoSOS Seller Payouts just like any other seller. The only caveat is that SudoSOS does not have a separate bank account, so this transfer of money will be done in the form of a journal entry instead.

The added benefit of this approach is that the balance of any seller in SudoSOS should be zero, including that of the BAC. The only reason it wouldn't be zero is if a Seller Payout is still in process (i.e., payouts are at the end of the month). Notice how this explanation aligns closely with how @Yoronex described it for other sellers. And just like @Yoronex mentioned, if someone changes any of the BAC's transactions in the past, this will immediately be clear as the BAC seller balance will not be zero!

Invoices as Deposits (Continued)

Invoice users are unusual in the sense that they have "no" SudoSOS coins but can still redeem them for goods at a seller. This is because having 0 SudoSOS coins doesn't actually mean you have no SudoSOS coins. If I have 0 euros and go to a bakery, I won't be able to give any euros to the baker in exchange for bread. However, in the context of SudoSOS, this is possible. You have a "phantom" balance of coins you can spend. When an invoice user (or a user without a balance) goes to a seller, they are still redeeming MPV SudoSOS coins in exchange for goods. Again, it is not known what an invoice user will buy with their SudoSOS coins before they actually go out and spend them.

With this in mind, SudoSOS invoices are actually SudoSOS Top-up "Requests." Since spending SudoSOS coins without having any means you implicitly borrowed coins from SudoSOS, SudoSOS naturally wants you to pay off this debt and requests you to top up your balance with a Top-up "Request." This again highlights that this is still considered MPV, as SudoSOS has no knowledge of what a user will use the borrowed coins for.

A SudoSOS Top-up "Request" is therefore not VAT-liable as it is no different from a user buying new SudoSOS coins. The redeeming of these coins by an (invoice) user is, of course, VAT-liable. This follows what @Yoronex wrote in the description.

Therefore, a SudoSOS Top-up "Request" can be seen as two separate parts. One part is the request to buy more SudoSOS Coins (not VAT-liable), and the other is the "proof" that a user borrowed SudoSOS coins in the form of a receipt showing how they redeemed these borrowed coins. The first page of a SudoSOS Top-up "Request" should therefore state the amount of coins the user borrowed, which they have to buy/top-up. The second page of the Top-up "Request" is a summary of all the receipts, which serves to provide proof that the requested amount of SudoSOS coins to be topped up is correct.

When SudoSOS enters this Top-up "Request" into their bookkeeping, it will do so as not VAT-liable since it is the same as a deposit. The receipts are actually snippets of what a seller sold, and therefore the VAT for that is in the seller's bookkeeping, completely independent of SudoSOS.

The main consideration should be how you present this Top-up Request so that it is clear for both the user and the tax administration. I personally think that a first page listing only the debt to be paid and the remaining pages showing the receipts (user transaction history) would be clear enough, but this might be unusual to send to companies. (@rinkp, any ideas?)

Cool Story, What Does This Change Exactly?

I never thought you'd ask! The main change involves how we migrate the database so that this concept is also applied to the history.

As stated by @Yoronex, we remove the opposite side of all the "invoice" transfers. This essentially turns them into Top-up Requests as described before.

For simplicity, consider all old "credit invoices" to be deleted.

Given that the BAC SudoSOS account is just another SudoSOS Seller:

In the case of other sellers, we create SudoSOS Seller Payouts that match the time intervals of the old credit invoices since most other sellers will prefer an activity-based payout instead of a recurring monthly payout.

As a small note, I suggest keeping the booking of 2022-2023 as is since we cannot change it anymore. There won't be an actual difference; we just need to ensure that we can still "recover" the reports as they are currently present in the bookkeeping, as @Yoronex mentioned.

Because of some small mistakes in the code regarding how the credit invoices used to work (see the bugs at the top of the discussion), there might be some small fixes needed in the database for 2022-2023, but I have already created an overview of these.

In conclusion, this will not only fix SudoSOS but also make it extremely apparent what should be entered in the bookkeeping with VAT and what should not since it will either be present on the document or not. Just like when booking in a Zettle report, you go to the second page and enter the VAT information. I will create a more practical flowchart-like guide to bookkeeping later and go over it with the relevant parties to ensure it is clear.

Food for though:

rinkp commented 4 weeks ago

I think it is very good to discuss this change extensively before implementation. Certain decisions regarding the scope of the project must be made, especially for reasons of expectation management.

SCOPE OF THE PROJECT

Currently, I consider SudoSOS a cash register system (kassasysteem) with some advanced features such as remote payments and invoices for transactions you do in-store. This part is very much comparable to Zettle's cash register software + payment service provider features:

The significant difference here is that SudoSOS is internal to GEWIS, which allows gift card balances to be used across multiple sellers. For that reason, you cannot apply the same logic to both.

In my opinion, the scope of SudoSOS shall not be expanded to turn it into accounting software. Zettle does not offer functionality to make invoices other than through a POS transaction, Zettle also does not offer options to create invoices that will not settle through the system. Furthermore, for tracking whether such invoices have been paid, we are already using Multivers. If it considers invoices related to bar expenses, we can simply create a container with products that can be charged by the BAC treasurer. This uses existing functionality reducing maintain complexity.
However, such functionality is desired. The main question is whether it should be included in SudoSOS or in a separate tool that can also be used by the intro treasurer, the GEWIS treasurer, and committee treasurers, etc.

THE IMPLEMENTATION

As I have already discussed with three of you, I agree with @JustSamuel on most items. I propose replacing a top-up request with a collective invoice, so that customers will have only one document to work with rather than 15 tiny invoices, one for each transaction, and then a payment overview. It will then boil down to the following:

Then, some implementation thing:

Yoronex commented 1 week ago

Implemented in version 0.6.0 https://github.com/GEWIS/sudosos-backend/releases/tag/v0.6.0.