Add Group overview to group management

yubiuser · April 30, 2020, 7:03am

With the current implementation of the group management it is easy to assign clients/domains/adlists to groups. Depending on users needs the group management might be fairly complex. At the moment there is no easy way to answer the questions

What clients/adlists/domains are in group XY?"

Users need to go through each subsection (clients, adlists, domains) to answer the question. It would be extremly helpful to have a tool to look at a group from the opposite direction. I could image a table with each group one can expand and sees which client/adlist/domains belong to that group. For easy handling one could also remove items from that group overview directely.

Feel to move this to Feature Requeste or Beta v5.1

jfb · April 30, 2020, 2:07pm

There is a simple (basic, rudimentary) table showing this in the V5 debug log. Not all in section of the log though.

yubiuser · April 30, 2020, 2:30pm

Makes a good start.

DL6ER · April 30, 2020, 3:00pm

This is the very same output as seen on the front end.

Am I right assuming that the output should rather be something like the following?

Group        | Client
-------------------------
Unassociated | Client1
(enabled)    | Client2
             | Client3
             | Client4
             | Client6
-------------|----------
IoT-devices  | Client2
(disabled)   | Client4
-------------|----------
Whatever     | Client5
...          | ...

We should clarify what should be contained and how it could be formatted.

yubiuser · April 30, 2020, 3:26pm

I imagined something like

Group
-------------------------
Unassociated | 
(enabled)  
-------------------------
            Clients
            ----------
            Client1        | Remove
            Client2        | Remove
            Client3        | Remove
            Client4        | Remove
            Client6        | Remove
            
            Adlists
            ----------
            Adlist1        | Remove
            Adlist2        | Remove
            Adlist3        | Remove

            Exact Whitelist
            -------------
            
            Regex Whitelist
            ---------------
            Regex 1        | Remove
            Regex 15       | Remove
          
            Exact Blacklist
            -------------
            Domain 1       | Remove
            
            Regex Blacklist
            ---------------
            Regex 23       | Remove
            Regex 15       | Remove

-------------------------
IoT-devices  
(disabled)
-------------------------
            Clients
            ----------
            Client11        | Remove
            Client21        | Remove
            Client31        | Remove
            Client41        | Remove
            Client61        | Remove
            
            Adlists
            ----------
            Adlist1        | Remove
            Adlist2        | Remove
            Adlist3        | Remove

            Exact Whitelist
            -------------
            
            Regex Whitelist
            ---------------
            Regex 1        | Remove
            Regex 15       | Remove
          
            Exact Blacklist
            -------------
            Domain 1       | Remove
            
            Regex Blacklist
            ---------------
            Regex 23       | Remove
            Regex 15       | Remove

-------------

Bucking_Horn · May 1, 2020, 12:26pm

That looks a lot like the side menu structure with all items expanded at once.

While quite uniform and tidy when using abstract names, that may become not very appealing, even fidgety, once you start populating that with actual properties for the different categories, for two reasons:
Items of different categories will have different properties, e.g. IP address(es) and hostname(s) for a client or name, last update timestamp and description for an adlist.
And as we are dealing with n-to-m-relationships for the most part, an item may appear in several places (as you have included in your sample structure).

For "five millions blocked domains" users, this may become a bit unwieldy.

The main advantage I see in that approach is you do show everything in one place.

I personally don't like scrolling long lists that much, and I'd also guess that most of the time, a user would focus on managing same type items from each of the sections (of course, that's an assumption only).

So I would probably want to select a specific group (maybe from a dropdown), displaying some basic info and allowing me to switch the item of interest (i.e. Clients, Adlists, etc.), which in return prompts display of the associated list.

sample layout

|---------------------------------------------------|
| Group: [Unassociated [V]]                         |
|                                                   |
| Description: Default group                        |
|                                                   |
|       State: Active                               |
|                                                   |
|                                                   |
|                 [Deactivate Group] [Remove Group] |
|                                                   |
|---------------------------------------------------|
|  >>10 Clients<< |   5 Adlists     |   2 RegEx     |
|---------------------------------------------------|
|                                                   |
| id   HOST              IP address                 |
| --   ----------------  ------------               |
| #1 laptop            192.168.0.32        [Remove] |
| #2 iphone            192.168.0.64        [Remove] |
| #3 elma-sniddles-t.. 192.168.0.96        [Remove] |
| #4 big-box-multi-vm  192.168.9.99        [Remove] |
|                                                   |
|---------------------------------------------------|

The main disadvantage of this approach is that it focuses on a single group.
To mitigate this, you would also need a dedicated search that would allow you to find the groups of your interest.

Anyway, it won't be easy to decide on any layout without knowing or assuming how that information will be used most often.
Again, that's me assuming that a user would normally look at data to initate some kind of action, rather than just for the sake of looking.

yubiuser · May 1, 2020, 5:55pm

I wouldn't include that all. Only client (host/IP), domain and adlist URL with corresponding comment. So it would be only 2-3 columns per (sub) entry.

Me neither, that's why I suggested

I thought of something like this (datatables Child row)

But I see the charm of your design. It's much cleaner and I liked the "boxed" design. The challenge would be the upstream choise/design of which/how a specific group is selected.

I'm not sure if this is a disadvantage when the user can switch fast and easily between groups.
I assume typical user questions would be something like

"Are all my kid's device in Group "Kids"?"
"Did I add all malware lists to Group "IoT?"
"Did I add all Whitelist entries to Group "Samsung TV"?"

Bucking_Horn · May 1, 2020, 6:10pm

I must confess I haven't looked at v5 group management that intensely until now.

Think of questions like "A given client is a member or which groups?" or "Which groups do not contain a given adlist yet?", and a search may prove useful or even necessary, if the information sought is not available elsewhere.

yubiuser · May 1, 2020, 6:15pm

Both can be answered already easily with the current implementation.

DL6ER · June 4, 2020, 2:37pm

I did some work on this, so far, I implemented the necessary back end changes.

Screenshots:

Generated data (to be pretty-parsed into some table in the future):

{
  "groups": [
    {
      "name": "Default",
      "enabled": true,
      "clients": [
        {
          "name": "Default",
          "client": "192.168.0.102",
          "date_added": 1588916040,
          "date_modified": 1588916040,
          "comment": null
        },
        {
          "name": "Default",
          "client": "192.168.0.103",
          "date_added": 1588916041,
          "date_modified": 1588916041,
          "comment": null
        },
        {
          "name": "Default",
          "client": "localhost",
          "date_added": 1589807760,
          "date_modified": 1589807760,
          "comment": null
        }
      ],
      "adlists": [
        {
          "name": "Default",
          "adlist": "https://raw.githubusercontent.com/StevenBlack/hosts/master/hosts",
          "enabled": true,
          "date_added": 1579806006,
          "date_modified": 1588916570,
          "comment": "Migrated from /etc/pihole/adlists.list"
        },
        {
          "name": "Default",
          "adlist": "https://mirror1.malwaredomains.com/files/justdomains",
          "enabled": true,
          "date_added": 1579806006,
          "date_modified": 1580682939,
          "comment": "Migrated from /etc/pihole/adlists.list"
        },
        {
          "name": "Default",
          "adlist": "http://sysctl.org/cameleon/hosts",
          "enabled": true,
          "date_added": 1579806006,
          "date_modified": 1588872347,
          "comment": "Migrated from /etc/pihole/adlists.list"
        },
        {
          "name": "Default",
          "adlist": "https://s3.amazonaws.com/lists.disconnect.me/simple_tracking.txt",
          "enabled": true,
          "date_added": 1579806006,
          "date_modified": 1580682939,
          "comment": "Migrated from /etc/pihole/adlists.list"
        },
        {
          "name": "Default",
          "adlist": "https://s3.amazonaws.com/lists.disconnect.me/simple_ad.txt",
          "enabled": true,
          "date_added": 1579806006,
          "date_modified": 1580682939,
          "comment": "Migrated from /etc/pihole/adlists.list"
        }
      ],
      "domains": [
        {
          "name": "Default",
          "type": 1,
          "domain": "blacklisted.com",
          "enabled": true,
          "date_added": 1591089723,
          "date_modified": 1591089723,
          "comment": ""
        },
        {
          "name": "Default",
          "type": 1,
          "domain": "doubleclick.net",
          "enabled": true,
          "date_added": 1591090417,
          "date_modified": 1591090417,
          "comment": ""
        }
      ]
    },
    {
      "name": "Group 1",
      "enabled": true,
      "clients": [
        {
          "name": "Group 1",
          "client": "192.168.0.101",
          "date_added": 1588916040,
          "date_modified": 1588916270,
          "comment": null
        }
      ],
      "adlists": [
        {
          "name": "Group 1",
          "adlist": "https://raw.githubusercontent.com/StevenBlack/hosts/master/hosts",
          "enabled": true,
          "date_added": 1579806006,
          "date_modified": 1588916570,
          "comment": "Migrated from /etc/pihole/adlists.list"
        }
      ],
      "domains": []
    },
    {
      "name": "Group 2",
      "enabled": true,
      "clients": [
        {
          "name": "Group 2",
          "client": "192.168.0.102",
          "date_added": 1588916040,
          "date_modified": 1588916040,
          "comment": null
        }
      ],
      "adlists": [],
      "domains": []
    },
    {
      "name": "Group 3",
      "enabled": true,
      "clients": [
        {
          "name": "Group 3",
          "client": "192.168.0.103",
          "date_added": 1588916041,
          "date_modified": 1588916041,
          "comment": null
        }
      ],
      "adlists": [],
      "domains": []
    }
  ]
}

I think this contains anything we will need herein, what do you think?

edit: Pushed to branch new/group_overview

yubiuser · June 4, 2020, 4:04pm

Great. But leave something for 5.2

Group description is missing.

Is it necessary for all items in a group to contain the name of the group again, like "name": "Group 2"?

Empty comments for domains are "comment": "" and for everything else "comment": null

DL6ER · June 5, 2020, 7:21am

This will not be in v5.1, the entire frontend is still missing and v5.1 is (loosely) scheduled for release in 1-2 weeks. It's almost time to go into a short code freeze phase so we can test and verify the new changes.

Good catch, the SQL query is too verbose here, I will optimize it.

Yes, however, this is what is in the database. I will check this out separately and push a fix.

yubiuser · June 5, 2020, 7:23am

That's what I thought. I just reported as it might be a problem for later parsing.

You noted?

DL6ER · June 5, 2020, 10:14am

Issue 1 will be resolved by

https://github.com/pi-hole/AdminLTE/pull/1442

We will still take both possibilities "" and NULL for empty comments into account as users may have been interacting with the database directly and this does not necessarily need to adhere with how we anticipate things.

Updated output is:

{
  "groups": [
    {
      "enabled": true,
      "name": "Default",
      "date_added": 1588874261,
      "date_modified": 1591087656,
      "description": "The default group",
      "clients": [
        {
          "client": "192.168.0.102",
          "date_added": 1588916040,
          "date_modified": 1588916040,
          "comment": null
        },
        {
          "client": "192.168.0.103",
          "date_added": 1588916041,
          "date_modified": 1588916041,
          "comment": null
        },
        {
          "client": "localhost",
          "date_added": 1589807760,
          "date_modified": 1589807760,
          "comment": null
        }
      ],
      "adlists": [
        {
          "address": "https://raw.githubusercontent.com/StevenBlack/hosts/master/hosts",
          "enabled": true,
          "date_added": 1579806006,
          "date_modified": 1588916570,
          "comment": "Migrated from /etc/pihole/adlists.list"
        },
        {
          "address": "https://mirror1.malwaredomains.com/files/justdomains",
          "enabled": true,
          "date_added": 1579806006,
          "date_modified": 1580682939,
          "comment": "Migrated from /etc/pihole/adlists.list"
        },
        {
          "address": "http://sysctl.org/cameleon/hosts",
          "enabled": true,
          "date_added": 1579806006,
          "date_modified": 1588872347,
          "comment": "Migrated from /etc/pihole/adlists.list"
        },
        {
          "address": "https://s3.amazonaws.com/lists.disconnect.me/simple_tracking.txt",
          "enabled": true,
          "date_added": 1579806006,
          "date_modified": 1580682939,
          "comment": "Migrated from /etc/pihole/adlists.list"
        },
        {
          "address": "https://s3.amazonaws.com/lists.disconnect.me/simple_ad.txt",
          "enabled": true,
          "date_added": 1579806006,
          "date_modified": 1580682939,
          "comment": "Migrated from /etc/pihole/adlists.list"
        }
      ],
      "domains": [
        {
          "type": 1,
          "domain": "blacklisted.com",
          "enabled": true,
          "date_added": 1591089723,
          "date_modified": 1591089723,
          "comment": ""
        },
        {
          "type": 1,
          "domain": "doubleclick.net",
          "enabled": true,
          "date_added": 1591090417,
          "date_modified": 1591090417,
          "comment": ""
        },
        {
          "type": 0,
          "domain": "a",
          "enabled": true,
          "date_added": 1591350319,
          "date_modified": 1591350319,
          "comment": null
        }
      ]
    },
    {
      "enabled": true,
      "name": "Group 1",
      "date_added": 1588915897,
      "date_modified": 1588915897,
      "description": null,
      "clients": [
        {
          "client": "192.168.0.101",
          "date_added": 1588916040,
          "date_modified": 1588916270,
          "comment": null
        }
      ],
      "adlists": [
        {
          "address": "https://raw.githubusercontent.com/StevenBlack/hosts/master/hosts",
          "enabled": true,
          "date_added": 1579806006,
          "date_modified": 1588916570,
          "comment": "Migrated from /etc/pihole/adlists.list"
        }
      ],
      "domains": []
    },
    {
      "enabled": true,
      "name": "Group 2",
      "date_added": 1588915897,
      "date_modified": 1588915897,
      "description": null,
      "clients": [
        {
          "client": "192.168.0.102",
          "date_added": 1588916040,
          "date_modified": 1588916040,
          "comment": null
        }
      ],
      "adlists": [],
      "domains": []
    },
    {
      "enabled": true,
      "name": "Group 3",
      "date_added": 1588915898,
      "date_modified": 1588915898,
      "description": null,
      "clients": [
        {
          "client": "192.168.0.103",
          "date_added": 1588916041,
          "date_modified": 1588916041,
          "comment": null
        }
      ],
      "adlists": [],
      "domains": []
    }
  ]
}

yubiuser · June 5, 2020, 11:15am

Looks good to me.