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

Fix api usage with custom name environment variable #56

Conversation

pedropombeiro
Copy link
Collaborator

Closes #55

@pedropombeiro pedropombeiro changed the base branch from master to development October 11, 2024 16:22
@pedropombeiro pedropombeiro force-pushed the pedropombeiro/fix-api-usage-with-custom-NAME-env-var branch 2 times, most recently from ceaa182 to 2bde31e Compare October 11, 2024 16:33
@pedropombeiro pedropombeiro force-pushed the pedropombeiro/fix-api-usage-with-custom-NAME-env-var branch from 2bde31e to 73a5be4 Compare October 11, 2024 16:40
@PhilippMundhenk
Copy link
Owner

We can also move this to master directly, as it fixes a small issue. No need to go via development, I think

@pedropombeiro
Copy link
Collaborator Author

I'll let you know when it is working. I'm still having a problem with env var expansion.

@pedropombeiro
Copy link
Collaborator Author

pedropombeiro commented Oct 11, 2024

@PhilippMundhenk I can now scan through a curl command (even from outside the container), but nothing happens when I do it from the web UI (the button stays pressed but no scanner activity).

@PhilippMundhenk
Copy link
Owner

PhilippMundhenk commented Oct 11, 2024

Could it be a Javascript issue? Have you tried calling the /scan.php URL from browser? Does this trigger (this is effectively what the JS does)? Any Javascript issues showing up in the browser console (F12)?

@pedropombeiro
Copy link
Collaborator Author

Ah yes:

image image

@PhilippMundhenk
Copy link
Owner

uhm. I'm lost. This looks like perfectly good JS to me. I would put a { there... Are you testing on master?
@vgarcia007: You have more experience with JS. Any ideas?

@pedropombeiro
Copy link
Collaborator Author

I'm testing on this branch. It did work from the browser if I put the final URL there.

@PhilippMundhenk
Copy link
Owner

Could you try another browser?

@pedropombeiro
Copy link
Collaborator Author

Could you try another browser?

That's the first thing I tried (Safari on mobile, after Edge on desktop)

@PhilippMundhenk
Copy link
Owner

PhilippMundhenk commented Oct 11, 2024

oooooh, the linting! elseif vs else if. The latter is js, the earlier the correction after linting: 729851c

@PhilippMundhenk
Copy link
Owner

This should do it

@pedropombeiro
Copy link
Collaborator Author

That worked! 👍

@StarTakko
Copy link

Hi, when will it be moved to master? 😁

@PhilippMundhenk
Copy link
Owner

I'll let you know when it is working. I'm still having a problem with env var expansion.

Hi @pedropombeiro, did you manage to fix this? Is there anything missing to merge this?

@pedropombeiro
Copy link
Collaborator Author

@PhilippMundhenk it is working for me, after your fix of the else if.

@PhilippMundhenk PhilippMundhenk changed the base branch from development to master October 20, 2024 08:14
@PhilippMundhenk PhilippMundhenk changed the base branch from master to development October 20, 2024 08:15
@PhilippMundhenk
Copy link
Owner

Ok, lets put this to develpoment and I extracted your fix to apply directly to master as well, in #58.

@PhilippMundhenk PhilippMundhenk merged commit 93e6f9c into development Oct 20, 2024
1 check passed
@PhilippMundhenk
Copy link
Owner

PhilippMundhenk commented Oct 20, 2024

@StarTakko : This PR is going through our development branch first, since the changes are based on the Python rewrite, but a fix for the custom naming should be in master today, via #58 . Thank you for the hint!

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

Successfully merging this pull request may close these issues.

Web server non-functional, no logs
3 participants