crater-invoice / crater

Open Source Invoicing Solution for Individuals & Businesses
https://craterapp.com
GNU Affero General Public License v3.0
7.85k stars 1.57k forks source link

PDF URLs #1142

Open chestnutbak opened 1 year ago

chestnutbak commented 1 year ago

Describe the bug We have an issue where the incorrect PDF gets loaded when trying to view specific invoices. Here are the links to the different invoices: https://example.com/invoices/pdf/7PVveOmgj7pqgrNLRG16 https://example.com/invoices/pdf/7PVveOmgj7PqgrNLRG16

Notice how there is only a capital P or p difference in the URL... I can only assume this is causing the issue because I can't see what else it might be. When we check the local storage both PDFs are there with their unique numbers. In this instance the invoice we are loading is number 709 but the one that shows up is 685. Our numbering is: Receipt - 2022-23/00685

Also... This is not the first time this has happened but I cannot find the other numbers and URLs that it happened with as they were deleted from the system.

Expected behavior The correct PDF gets loaded when viewing every invoice.

Screenshots image

Please complete the following information:

pepa65 commented 1 year ago

This is a real bug, that I think should not be hard to fix. Today it happened 3 times, and the only thing we can tell Accounting is to make the same invoice again (hoping this time the uppercase/lowercase id does not clash...).

Are the clashes happening so frequent because the first twenty or so characters of the receipt name are the same??

pepa65 commented 1 year ago

To clarify the bug:

The bug is where the pdf retrieval function (maybe in Vue?) ignores case, and the match then finds the oldest document that matches.

mohitpanjwani commented 1 year ago

Thanks for the feedback & suggestion @chestnutbak & @pepa65.

We are using a package called laravel-hashids in order to generate unique ids for the invoices but we noticed just now that it has many repeating characters on the hashids.php file which is causing the issue.

Removing the repeating characters from the configuration should fix the problem. We will also create a new migration in order to update existing invoices with the same unique_hash to fix existing duplicates.

@radhika587 is working on creating a PR for this asap.

pepa65 commented 1 year ago

But how about the uppercase/lowercase issue? If you use this URL to download an invoice: https://example.com/invoices/pdf/8V1DGOqxnY1RxnjNmpYR you will get https://example.com/invoices/pdf/8V1DGOqxnY1RxnjNmpYr (see the final char) if it is the unique_hash of an earlier pdf.

You an easily confirm this bug by changing the hashid of an older invoice in the database to differ only in case from a newer invoice.

pepa65 commented 1 year ago

@radhika587 If I patch this by hand, is there anything else I need to do with composer/artisan to make sure it's being applied?

mohitpanjwani commented 1 year ago

@pepa65 No need to do anything except patching these changes.. but please note that it will not fix or update the old invoices/estimates/payments that are already created.

In order to fix that, we will need to create a separate one-off command or something that will run a query for the duplicate records and regenerate the hash ids for it.

pepa65 commented 1 year ago

In our case, the accounting department had to issue the invoice, customer waiting, so they generated another one. The "bad" ones just got deleted...