RSC-Labs / medusa-documents

Medusa plugin which generates documents (e.g. invoices) in PDF
MIT License
43 stars 5 forks source link

unable to set invoice settings #7

Closed sashahilton00 closed 3 months ago

sashahilton00 commented 3 months ago

When installing this on a new instance of medusa (1.20.6), I am unable to set the invoice settings - upon clicking the invoice settings button one gets a cryptic e is null error, along with a useless stacktrace due to bundling. Happy to try and help out in diagnosing this, but have hit a roadblock thus far.

radoslaw-sz commented 3 months ago

hi @sashahilton00 - which version of this plugin you are using? There was a recent update which migrate to medusa 1.20.6 - 0.7.1. Or maybe it was happen because you migrated to mentioned version? I know that this bundling thing is often useless, but if you could paste it anyway - sometimes it is helpful.

sashahilton00 commented 3 months ago

I actually saw the commit for 0.7.1 occur as I was browsing the repo - I created a fresh instance of 1.20.6 and installed 0.7.1 with the above issue.

I’ll paste the trace once I’m back at my desk, it indicated it was something in _virtual_entry.js from what I remember

On Friday, May 10, 2024 at 3:31 PM, Radosław @. @.)> wrote:

hi @sashahilton00 (https://github.com/sashahilton00) - which version of this plugin you are using? There was a recent update which migrate to medusa 1.20.6 - 0.7.1. Or maybe it was happen because you migrated to mentioned version? I know that this bundling thing is often useless, but if you could paste it anyway - sometimes it is helpful.

— Reply to this email directly, view it on GitHub (https://github.com/RSC-Labs/medusa-documents/issues/7#issuecomment-2104715402), or unsubscribe (https://github.com/notifications/unsubscribe-auth/AA752EQW32YOJ4THIDFSJIDZBTK3HAVCNFSM6AAAAABHP2DGJWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMBUG4YTKNBQGI). You are receiving this because you were mentioned.Message ID: @.***>

sashahilton00 commented 3 months ago

As promised:

TypeError
e is null

Call Stack
 aU
  vendors-node_modules_rsc-labs_medusa-documents_dist_admin__virtual_entry_js.chunk.js:253:111877
 le
  main.bundle.js:510:55326
 31557/c7/<
  main.bundle.js:510:109743
 c7
  main.bundle.js:510:111085
 g
  main.bundle.js:498:48300
 r3
  main.bundle.js:498:48625
 nt
  main.bundle.js:498:49578
 eh
  main.bundle.js:510:170988
 fF
  main.bundle.js:510:156931
 31557/fU/<
  main.bundle.js:510:156654

Also, it may sound slightly pathetic but I haven't worked out how to get medusa to hot reload local dependencies - if you've got a pointer on that I'm happy to try and load this as a local dependency to poke around and try to resolve the issue.

radoslaw-sz commented 3 months ago

omg, indeed impossible to figure out something 😞 Was this plugin working for you before? I mean with previous Medusa version and previous version of the plugin? Maybe you would be to able to try with some variations, like previous Medusa version and current plugin version and vice versa (to narrow the problem). Is this problem only visible when you click Settings in Invoice? What about address settings? It works for you? The only thing which happens when you click settings in invoice card is to show Focus Modal (https://docs.medusajs.com/ui/components/focus-modal). Please also (but I guess you already did it) try to clean up and make "yarn build" (after install) and then start the backend.

Regarding reloading local dependencies - what configuration you exactly mean? For instance, when you install the plugin into clean backend, then virtual_entry is generated inside of the plugin package so under node_modules/@rsc-labs/medusa-documents/dist/admin. You can in theory change there the code, putting some console logs etc. You can also clone the repository and put src of the plugin to src of the backend and check if it works.

sashahilton00 commented 3 months ago

Hi,

The problem is only when I click invoice settings. Address settings works fine. My theory is that if no settings have been initialised, the endpoint returns null, and then causes this issue. It's just a guess though.

I've just tried a fresh install, added the plugin, run yarn install and yarn build, still the same error.

From what I can see this plugin has the deveDependency "@medusajs/medusa": "1.19.0" and the peerDependency "@medusajs/medusa": "1.20.1" - have you tried running the plugin on 1.20.6 on your side?

radoslaw-sz commented 3 months ago

hi @sashahilton00 - did you run maybe migrations before? (I am just guessing what could be the issue). Following your suggestion I have updated versions of medusajs to 1.20.6. Such updated is available in version 0.7.2 of plugin - please kindly check if it helps.

sashahilton00 commented 3 months ago

Ok so I figured out how to fix it, though I haven't found the issue in the code yet. It was as I suspected - because there were no existing settings configured (i.e. the document_invoice_settings table in the DB had zero rows), the code was trying to access a null object and crashing. This was confirmed by the preview windows not working either.

To fix it, I configured the address settings via the UI, copied the id beginning with docset_ from the document_settings table, and created a row in the document_invoice_settings table with the same ID. It was then possible to configure everything else via the UI, which created a new entry in the table with the id docinvset_ prefix.

So my guess is that somewhere in the code a null check is missing.

radoslaw-sz commented 3 months ago

hi @sashahilton00 - thank you very much for this analysis! Indeed it could be a reason. Let me check the code again and see if I can improve the code.

EDIT: Disregard below - probably it can happen - I need to check. Thanks again for the analysis!

However, please be aware that such situation shall not happen. I mean if you had already document_settings table with the row, then after running migrations you shall get also document_invoice_settings. Unfortunately it comes from the fact that document_settings was before the only table, but we needed to migrate some information into new table called document_invoice_settings. As I mentioned, I will keep this issue open to see what I can improve, but running migrations shall solve it - precisely this migration: https://github.com/RSC-Labs/medusa-documents/blob/main/src/migrations/1712753213091-DocumentInvoiceSettings.ts

sashahilton00 commented 3 months ago

No problem. What you said above does make sense, though your answer pointed to the problem - in the migration script the check is:

if (latestDocumentSettings.length > 0) {

which is false for a fresh install where no settings have been configured.

radoslaw-sz commented 3 months ago

yeah, that's true - I found the issue in the code and also found similar bugs - I will fix it in the next days.

radoslaw-sz commented 3 months ago

hi @sashahilton00 - fix landed in version 0.8.1 - let me know if you are still able to reproduce this issue and check if fix is working or you fixed it and you can't check it anymore.

sashahilton00 commented 3 months ago

Just installed the latest one, wiped the DB and ran migrations, no issues 👍 thanks for the quick fix.