Add group management

[DL6ER]

By submitting this pull request, I confirm the following:

• I have read and understood the contributors guide, as well as this entire template.
• I have made only one major change in my proposed changes.
• I have commented my proposed changes within the code.
• I have tested my proposed changes.
• I am willing to help maintain this change if there are issues with it later.
• I give this submission freely and claim no ownership.
• It is compatible with the EUPL 1.2 license
• I have squashed any insignificant commits. (git rebase)
• I have Signed Off all commits. (git commit --signoff)

---

What does this PR aim to accomplish?:

Add group management for the per-client blocking

How does this PR accomplish the above?:

Add four new pages where users can configure

• groups,
• clients,
• domains, and
• adlists

What documentation changes (if any) are needed to support this PR?:

We should write a tutorial or at least some documentation. Even if I tried to make it as easy as possible to use.

[liamsorsby]

I've had a little bit of a review but it seems that the coding standards used is completely different to that of the project. It might be worth running it through a linter to get it to fix the formatting.

[DL6ER]

@liamsorsby This code is still a draft more than anything else. Even if it is a completely working draft. This time, I used vscode as text editor as I was hoping for some better autocompletion support. This editor, however, was doing some strange/wrong assumptions when it comes to indentations. I may not have manually set the indentation of 4 spaces per level for each file.

Do you have recommendations for a linter?

It is no problem that the code is following a different style than the rest of the project as this web interface is about to get phased out in favor of our new React-based web interface. It turned out to be easier to code like I did now.

If your following the PSR-2 guidelines you should be formatting your if statements with a space before the conditional and the brace on the same line

As said, I'm not following any guideline here and do not see it beneficial to enforce this now in the last few months of lifetime of this project. As long as the code works - there will be not much maintenance to invest here. If you, however, can recommend a linter that would put all my code into the expected for automatically, then I'm obviously not against using this.

[liamsorsby]

Do you have recommendations for a linter?

If your using vscode I think there is a plugin for phpcs which should do most of the work for you.
If you are however moving to a react frontend then it may not be worth the effort as it could just block the initial PR.

[DL6ER]

I develop on a coding terminal that has no local PHP installation. And, as I do not want to be tied to vscode as editor in any way, and since I prefer to not install a lot of additional stuff on this machine (composer, ...), I chose to install an isolated version of phpcs on my server. This worked and formatted all the files according to PSR-12 standard. In addition, I used prettier to also enforce a common coding style for the JS files.

[DL6ER]

I implemented Client host name displaying implemented as I suggested earlier.

Selection menu:

Table:

[DL6ER]

From the mentioned ways of how we could do it, I currently prefer my idea "Auto Save".

[pralor-bot]

This pull request has been mentioned on Pi-hole Userspace. There might be relevant details there:

https://discourse.pi-hole.net/t/dev-bad-handling-of-capitalization-of-domain-names/25660/40

[XhmikosR]

Not sure I get some of the changes.

1. Why was DataTables updated in this branch and what are the exact build options used? I have Update DataTables to v1.10.21 #1116 and although untested, I used the default options + Bootstrap 3 which should be enough. If not, the options should have been at least mentioned in the commit message.

2. Why do we need a 3K file for animations? Do we really need all of them?

Ideally, we shouldn't even add unminified 3rd-party files either, but this would require a build step, since not all of the current 3rd-party libraries provide minified dist files, unfortunately.

Regardless, we shouldn't just add so many 3rd-party libraries so easy IMHO. Also, not cleaning the patches up before the merge doesn't help git history.

Just my 2 cents. I haven't even tried the devel branch, I'm just expressing my opinion.

[XhmikosR]

Also forgot to mention, there's so many duplicated code. I know abstracting can be hard, but this is exactly what causes maintenance issues in the longterm. I've attempted to move some common code in 25cea0f but there's a lot more. For example tailog.js and tailog-FTL.js, and some of the group*.js files logic could be abstracted to common functions.

[pralor-bot]

This pull request has been mentioned on Pi-hole Userspace. There might be relevant details there:

https://discourse.pi-hole.net/t/make-some-advance-api/26786/12

[DL6ER]

@XhmikosR Sorry, I missed your comments, previously. Here are my replies:

Why was DataTables updated in this branch and what are the exact build options used?

There were issues with the multiselect environment used for group management on the client, domains, and adlist pages (not rendering properly). These issues disappeared after the update. I have not build DataTables myself but used the pre-built variant from here: http://cdn.datatables.net/1.10.20/js/jquery.dataTables.min.js
I assume this is the minimal build with no external libraries included.

Why do we need a 3K file for animations? Do we really need all of them?

This file is needed by bootstrapToggle. I left it unmodified so it can easily be updated in the future by simply replacing it with a newer version (if needed). 3K should not be all that much of an issue nowadays, especially given browser caching. However, this is of course not a general excuse for adding more and more stuff if this is really not needed.

Ideally, we shouldn't even add unminified 3rd-party files either, but this would require a build step, since not all of the current 3rd-party libraries provide minified dist files, unfortunately.

I agree. This will change with the NG web interface. However, I don't think all that many micro-optimizations should still go into this web interface.

Regardless, we shouldn't just add so many 3rd-party libraries so easy IMHO. Also, not cleaning the patches up before the merge doesn't help git history.

What do you mean by "add so many 3rd-party libraries so easy". Do you suggest to write more functionality ourselves instead of just using ready external libraries for it? Or extracting only what we need from them? This implementation already took me several days as it was not only writing the code, also getting how conceptually it can be implemented was difficult. If there is some external library that allows me to add a function in a matter of seconds, we should always do this. There is enough pain involved with coding stuff that is completely new. Pre-mature optimization will only cost time and probably won't result in anything. Later optimization should be thought about carefully as well: Do we want to invest the time into making optimizations the user will never see? Or shall we rather invest our time into adding new features, e.g. for the NG web repo? I prefer the latter.

Also forgot to mention, there's so many duplicated code. I know abstracting can be hard, but this is exactly what causes maintenance issues in the longterm.

I know. And I'll right away admit that the lack of abstraction comes due to a certain level of laziness, not because it is hard. I already said above that it took me several (!) hours to get this features conceptualized, implemented, and tested. All four pages are similar, however there are some few, partially very subtle, differences. The plan always was that there will be no "in the long term" for this web interface and it was necessary to get this feature added before the beta which was initially thought to start in mid-December.

My main contribution to Pi-hole is code for new features and bug fixes. Optimizations are fine, however, when I can use an hour to implement a new feature or to micro-optimize something, I prefer to do the former. It is something our users will benefit from in a much more direct manner.

All this doesn't mean I do not value optimizations, however, we should be clear that the faster we can get NG web done, the better as it will remove any support burden on this PHP-driven web interface.

[XhmikosR]

There were issues with the multiselect environment used for group management on the client, domains, and adlist pages (not rendering properly). These issues disappeared after the update. I have not build DataTables myself but used the pre-built variant from here: http://cdn.datatables.net/1.10.20/js/jquery.dataTables.min.js
I assume this is the minimal build with no external libraries included.

I have #1116 which is I plan to document if everything still works.

This file is needed by bootstrapToggle. I left it unmodified so it can easily be updated in the future by simply replacing it with a newer version (if needed). 3K should not be all that much of an issue nowadays, especially given browser caching. However, this is of course not a general excuse for adding more and more stuff if this is really not needed.

I'd suggest that we don't add so many useless stuff from now on. We can check what is needed and only include the needed stuff.

What do you mean by "add so many 3rd-party libraries so easy". Do you suggest to write more functionality ourselves instead of just using ready external libraries for it?

I definitely don't mean reinventing the wheel. I just want o add only what we need and not extra stuff; see my point above.

I know. And I'll right away admit that the lack of abstraction comes due to a certain level of laziness, not because it is hard. I already said above that it took me several (!) hours to get this features conceptualized, implemented, and tested. All four pages are similar, however there are some few, partially very subtle, differences. The plan always was that there will be no "in the long term" for this web interface and it was necessary to get this feature added before the beta which was initially thought to start in mid-December.

The problem is that one can easily identify duplicate JS code. I'm not blaming you or anyone, I;m just saying we should try to reuse code instead. One example is devel...js-tweaks.

We could definitely combine tailog.js and tailog-FTL.js too.

I know this isn't an easy task but we need to take this stuff into consideration because otherwise we'll just keep bloating the codebase.

[pralor-bot]

This pull request has been mentioned on Pi-hole Userspace. There might be relevant details there:

https://discourse.pi-hole.net/t/show-100-entries-instead-of-10/44479/2