Update add.php

[ghost]

Added removal from blacklist if adding to whitelist
Added removal from whitelist if we add to the blacklist
Added removal option from both lists black & white option "none"

[PromoFaux]

I was looking over this PR on saturday, then life kind of got in the way. I am not sure if the change is a necessary one, at least in the case of removing from one list when adding to the other.

We already made a change in database.php which does REPLACE INTO the list, so it will automatically switch it from one to the other:

https://github.com/pi-hole/AdminLTE/blob/devel/scripts/pi-hole/php/database.php#L139

[ghost]

If this has been implemented at the database level, that's fine. Have you tested it on sample requests?

[PromoFaux]

It's implemented in the method add_to_table (on the dev branch at least, not master currently).

All I'm saying is that a call to remove_from_table before calling add_to_table is redundant because of the REPLACE INTO. Say you want to whitelist domain.com, and it is already on the blacklist, what REPLACE INTO is effectively saying is:

Insert domain.com to whitelist, and if domain.com is already there as a blacklist item, change the list type so that it is on the whitelist now.

This has been tested from the web interface:

[ghost]

I understand it makes sense. In that case, it could be useful to reset (cleaning the domain from all lists). I tried to solve this by adding "none"

[PromoFaux]

Yeah, removing from probably makes sense. I believe add.php and database.php have only stayed around for interaction with the query log, as most other functionality (including removing from lists) has been created in groups.php (@DL6ER would know better here, he did this part)

[ghost]

I think api is really needed. However, there is probably a need to rewrite.

[PromoFaux]

For sure. I think for the purposes of this PR, you should be able to get away with only having the case for none, and remove the two remove_from_table lines.

Although, can you test that removing those two lines still has the desired effects for the extension this is for?

[ghost]

You mean remove changes for case white and black? Because this is implemented with datebase query?

[PromoFaux]

Sorry, I wasn't clear! Yes, those two are redundant

[ghost]

I can delete but I have no way to check on dev branch (I'm using a docker instance)

[badsgahhl]

Hello,

i checked out the dev branch currently. And actually the replace feature which is done in the database.php is doing exactly the correct thing. Adding a domain to the whitelist/blacklist that is already on a list gets replaced. Perfect! But i think the none option is also nice to have, removing a domain from any list.

[PromoFaux]

Closing in favour of #1387

Thank you, nonetheless, for your contribution!