Skip to content

fix(Invoice Ninja Node): Fix assigning an invoice to a payment #9590

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

Merged

Conversation

CodeShakingSheep
Copy link
Contributor

@CodeShakingSheep CodeShakingSheep commented Jun 2, 2024

Summary

Describe what the PR does and how to test. Photos and videos are recommended.

Currently, when creating a payment and specifying an invoice ID for it, the invoice isn't assigned to the payment. Instead, an unapplied payment is created in InvoiceNinja. This is because the body for the payment create request has changed in API v5, see https://api-docs.invoicing.co/#post-/api/v1/payments.

This PR fixes this by keeping the current request body for API version 4 and changes adds an invoices array to the request body for API version 5.

{
  <OLD_PAYMENT_CREATE_REQUEST_BODY>
  "invoices": [
    {
      "invoice_id": "{{INVOICE_ID}}",
      "amount": "{{AMOUNT}}"
    }
  ]
}

Related tickets and issues

Include links to Linear ticket or Github issue or Community forum post. Important in order to close automatically and provide context to reviewers.

None available. I just used the Invoice Ninja API node and figured that a payment is never assigned to an invoice although an invoice_id has been specified.

Review / Merge checklist

  • PR title and summary are descriptive. Remember, the title automatically goes into the changelog. Use (no-changelog) otherwise. (conventions)
  • Docs updated or follow-up ticket created.
  • Tests included.

    A bug is not considered fixed, unless a test is added to prevent it from happening again.
    A feature is not complete without tests.

@n8n-assistant n8n-assistant bot added community Authored by a community member node/improvement New feature or request in linear Issue or PR has been created in Linear for internal review labels Jun 2, 2024
@CodeShakingSheep
Copy link
Contributor Author

@Joffcom Can this PR please get a review asap? It's broken functionality that's not working with the latest InvoiceNinja software and should be fixed. I made the changes as minimal invasive as possible as not to break v4 functionality. Thank you.

Same applies to #9589. As it seems InvoiceNinja v5 isn't working for 1.5 years when I look at the history of #4807 . So, I would really appreciate having this reviewed and merged asap. If there's anything I can do to speed this up, please let me know.

@Joffcom
Copy link
Member

Joffcom commented Jun 19, 2024

Hey @CodeShakingSheep,

Sorry I had a week off so wasn't able to get to this one, This PR is on my list to check tomorrow morning which is one of 2 review sessions I do each week.

@CodeShakingSheep
Copy link
Contributor Author

Hey @Joffcom,
of course, I hope you enjoyed your week off. As the PR hasn't been reviewed yet, I added a commit to fix assigning a payment type to a payment. This didn't work either in v5 as the key has been changed from payment_type_id to type_id. See https://api-docs.invoicing.co/#post-/api/v1/Payments Though, I kept version 4 compatibility.

I made the changes in my Invoice Ninja PRs as small as possible. I'd appreciate a review of these PRs asap when you have your next review session. Thank you 🙏

Copy link
Member

@Joffcom Joffcom left a comment

Choose a reason for hiding this comment

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

Thanks for this the change looks good.

@Joffcom Joffcom merged commit 7a3c127 into n8n-io:master Jul 3, 2024
8 of 9 checks passed
Copy link

cypress bot commented Jul 4, 2024

4 flaky tests on run #5773 ↗︎

0 399 0 0 Flakiness 4

Details:

🌳 master 🖥️ browsers:node18.12.0-chrome107 🤖 PR User 🗃️ e2e/*
Project: n8n Commit: 7a3c127b2c
Status: Passed Duration: 05:12 💡
Started: Jul 4, 2024 3:11 AM Ended: Jul 4, 2024 3:17 AM
Flakiness  10-undo-redo.cy.ts • 1 flaky test

View Output Video

Test Artifacts
Undo/Redo > should undo/redo renaming node using NDV Test Replay Screenshots Video
Flakiness  5-ndv.cy.ts • 2 flaky tests

View Output Video

Test Artifacts
NDV > should not retrieve remote options when required params throw errors Screenshots Video
NDV > Stop listening for trigger event from NDV Screenshots Video
Flakiness  20-workflow-executions.cy.ts • 1 flaky test

View Output Video

Test Artifacts
Current Workflow Executions > should auto load more items if there is space and auto scroll Test Replay Screenshots Video

Review all test suite changes for PR #9590 ↗︎

@janober
Copy link
Member

janober commented Jul 10, 2024

Got released with [email protected]

@CodeShakingSheep CodeShakingSheep deleted the fix-payment-assignment-to-invoice branch July 25, 2024 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community Authored by a community member in linear Issue or PR has been created in Linear for internal review node/improvement New feature or request Released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants