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

Regression: input multiple domains in domain input field adds them as a single domain #1621

Closed
yubiuser opened this issue Aug 8, 2023 · 7 comments · Fixed by #1629
Closed
Labels

Comments

@yubiuser
Copy link
Member

yubiuser commented Aug 8, 2023

Expected behavior is like it was in v5: domains separated by space should be added as individual entries.

@rdwebdesign
Copy link
Member

rdwebdesign commented Aug 8, 2023

I'm not sure if this issue should be handle in the web interface or in FTL.

In v5, the first step executed by PHP is to split the input into an array of domains:

https://github.com/pi-hole/AdminLTE/blob/41682f17b72d3fb83837a7a08fa68b3e37cd35b7/scripts/pi-hole/php/groups.php#L534

In this case, the expected value for the domain field ($_POST['domain']) is a string with one or more domains separated by spaces.

The current v6 web interface doesn't use a server side code here. It sends the whole text to the API. FTL doesn't seems to take care of spaces or line breaks and the whole string is used.

@yubiuser
Copy link
Member Author

yubiuser commented Aug 8, 2023

FTL doesn't seems to take care of spaces or line breaks and the whole string is used.

Exactly, FTL expects one domain to be added per request. I think the web interface needs to iterate over the input data and split it. Can this be done in JS?

@rdwebdesign
Copy link
Member

Can this be done in JS?

Yes, but this will send multiple API requests (one for each domain) and FTL will reload the list after each request.

I was thinking if this should be done in FTL, to handle multi-domain inputs (from web interface or command line) in a single step and reload the lists only once.

@yubiuser
Copy link
Member Author

yubiuser commented Aug 8, 2023

API requests (one for each domain) and FTL will reload the list after each request

I see the issue here. Agree, this should be handled by FTL

@yubiuser yubiuser transferred this issue from pi-hole/web Aug 9, 2023
@DL6ER
Copy link
Member

DL6ER commented Sep 6, 2023

#1629 adds handling in FTL. I'm currently developing away from my test environment and am not keen to break it from remote so this PR is - so far - entirely untested (you are welcomed to do this :-) ).

This will need a subsequent web change. Sending a space-separated domain seems a dirty hack. The PR above makes FTL accept either a string or an array of strings for domains, clients, group names and lists. The web interface will need to split the input itself and send a proper array to the API in this case.

@rdwebdesign
Copy link
Member

Sending a space-separated domain seems a dirty hack.

I never said we should do that. I just said this is how it is done in v5.


OK, now the web interface will accept a domain or a list of domains separated by spaces (maybe we could also accept line breaks). The javascript code will need to split the "domain" field and send the result as an array.

@github-actions
Copy link

github-actions bot commented Oct 7, 2023

This issue is stale because it has been open 30 days with no activity. Please comment or update this issue or it will be closed in 5 days.

@github-actions github-actions bot added the stale label Oct 7, 2023
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Oct 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants