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

Web invoice does not show logo #1012

Closed
jmclaren7 opened this issue Jan 5, 2024 · 2 comments · Fixed by #1013
Closed

Web invoice does not show logo #1012

jmclaren7 opened this issue Jan 5, 2024 · 2 comments · Fixed by #1013
Assignees
Milestone

Comments

@jmclaren7
Copy link
Contributor

jmclaren7 commented Jan 5, 2024

Expected Behavior

The invoice's web view uses invoice_logo() in invoice_helper.php, this function should provide a URL to the IP configured logo

Current Behavior

invoice_logo() produces an invalid URL based on an absolute file system path, it seems this change was only intended for invoice_logo_pdf()

Possible Solution

#1013

@nielsdrost7
Copy link
Contributor

nielsdrost7 commented Jan 5, 2024

Thanks for the bug report, man!
Can you help me out with an example of this one:

  • The invoice's web view uses invoice_logo() in invoice_helper.php
    And basically this one as well:
  • invoice_logo() produces an invalid URL based on an absolute file system path

I'm looking to reproduce the problem and then of course merge your fix.

What I want to prevent is that after merging some other logo's won't show again, if you know what I mean.

Related:
https://github.com/InvoicePlane/InvoicePlane/pull/964/files
Issue: #898
and (original)
https://github.com/InvoicePlane/InvoicePlane/pull/897/files

@nielsdrost7 nielsdrost7 self-assigned this Jan 5, 2024
@nielsdrost7 nielsdrost7 added this to the 1.6.2 milestone Jan 5, 2024
@nielsdrost7 nielsdrost7 linked a pull request Jan 5, 2024 that will close this issue
@jmclaren7
Copy link
Contributor Author

jmclaren7 commented Jan 5, 2024

Just to restate the issue: when viewing an invoice using the web view, the logo will not load. When viewing what URL was generated from the template we can see that it's a file system path, in my case:

image

In #898 / #964 the stated issue was related to PDF's only but both invoice_logo() invoice_logo_pdf() were modified. As far as I can tell the main difference between the two functions is that:

  • invoice_logo_pdf() should return a file system path so mpdf can load the logo for server side pdf generation
  • invoice_logo() should return a public URL that can be used by a visitors browser to load the logo

#964 seems to have modified both functions when working on an issue related specifically to PDFs.

In #897 the issue was related to PDF and only invoice_logo_pdf() was modified so it wont be related to my issue.

You can see in the default PDF template, the logo is pulled with invoice_logo_pdf() so as long as that function isn't modified those referenced issues/pulls should not be impacted unless someone used invoice_logo() in a custom pdf template by mistake.

I just made an additional commit because I made a mistake and had an extra slash, I didn't see that base_url() already includes a trailing slash (although I'm not sure if it will when IP is in a sub directory).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants