Merge pull request #207 from ipnet-mesh/chore/member-operator-fixes

fix: member count badge and operator-only filter dropdowns
2026-08-07 17:33:20 +02:00 · 2026-05-11 21:33:47 +01:00
parent 92f56fa24b 6385f22ac3
commit 069afca74c
12 changed files with 281 additions and 13 deletions
@@ -160,7 +160,9 @@ Toast/flash messages after successful operations:
 | `all_types` | All Types | "All types" filter option |
 | `all_channels` | All Channels | "All channels" filter option |
 | `all_members` | All Members | "All members" filter option (only shown when OIDC is enabled) |
+| `all_operators` | All Operators | "All operators" filter option for operator-only filter dropdowns |
 | `filter_member_label` | Member | Label for member filter dropdown |
+| `filter_operator_label` | Operator | Label for operator filter dropdown (used on Nodes, Advertisements, Map pages) |
 | `node_type` | Node Type | Node type field |
 | `show` | Show | Show/display action |
 | `search_placeholder` | Search by name, ID, or public key... | Search input placeholder |
@@ -0,0 +1,189 @@
+# Plan: Members Count Fix & Operator-Only Filters
+
+**Date:** 2026-05-11
+**Status:** Draft
+
+## Problem
+
+Two related issues with member/operator display in the UI:
+
+1. **Members page count mismatch** — The badge in the upper-right corner of the Members page shows a higher count than the total of operators and members actually listed. Profiles that have neither the `operator` nor `member` role (e.g., admins with only the `admin` role, or users with no roles at all) are counted in the badge but not displayed in any group.
+
+2. **Filter dropdowns show all members instead of operators only** — The "Member" filter on Nodes, Advertisements, and Map pages lists all non-test profiles. Since only operators can adopt nodes, this filter should only show operators and the label should read "Operator" not "Member".
+
+## Root Cause Analysis
+
+### Issue 1: Members Page Count
+
+In `members.js`:
+
+- Line 74: Fetches profiles from `/api/v1/user/profiles` (excludes test users by default)
+- Line 77: Client-side defense-in-depth filter removes remaining test-role profiles
+- Line 100: Badge shows `profiles.length` — **all** non-test profiles
+- Lines 92-95: Only `operators` (have operator role) and `members` (have member role but NOT operator) are rendered as groups
+
+A profile with `roles = ["admin"]` or `roles = []` passes the test filter, is counted in the badge, but is invisible on the page — it appears in neither the Operators nor Members group.
+
+### Issue 2: Filter Dropdowns
+
+In `nodes.js` (line 60), `advertisements.js` (line 67), and `map.js` (line 121/126):
+
+- All fetch `/api/v1/user/profiles` with `limit: 500`
+- All render every profile in the dropdown using label `filter_member_label` ("Member") and all-option `all_members` ("All Members")
+- The `adopted_by` parameter filters nodes by the adopting profile's UUID — only operators can adopt nodes, so showing non-operators in the dropdown is misleading (selecting a non-operator who hasn't adopted any nodes returns zero results)
+
+## Approach
+
+### Fix 1: Members Page Count — Only Count Displayed Profiles
+
+**File:** `src/meshcore_hub/web/static/js/spa/pages/members.js`
+
+Change the badge to show the sum of operators + members actually displayed, not the total of all profiles:
+
+```javascript
+// Before (line 100):
+${t('common.count_entity', { count: profiles.length, entity: t('entities.members').toLowerCase() })}
+
+// After:
+${t('common.count_entity', { count: operators.length + members.length, entity: t('entities.members').toLowerCase() })}
+```
+
+This makes the count reflect exactly what is rendered on the page. The `operators` and `members` variables are already computed at lines 92-95, before the `litRender` call at line 97. No structural change needed — only the expression on line 100 changes.
+
+**Change:** Replace `profiles.length` with `operators.length + members.length` on line 100.
+
+### Fix 2: Operator-Only Filter Dropdowns
+
+For all three pages (Nodes, Advertisements, Map), filter the profiles list to only show operators and update labels.
+
+#### 2a. Add i18n Keys
+
+**Files:** `src/meshcore_hub/web/static/locales/en.json`, `nl.json`, `docs/i18n.md`
+
+Add new keys under `common`:
+
+```json
+"all_operators": "All Operators",
+"filter_operator_label": "Operator"
+```
+
+These replace the usage of `all_members` and `filter_member_label` in the filter dropdowns.
+
+Dutch translations:
+```json
+"all_operators": "Alle Operators",
+"filter_operator_label": "Operator"
+```
+
+Update `docs/i18n.md` with the new keys following the existing format.
+
+#### 2b. Nodes Page
+
+**File:** `src/meshcore_hub/web/static/js/spa/pages/nodes.js`
+
+After fetching profiles (line 64), filter to operators only:
+
+```javascript
+const operatorRole = config.role_names?.operator || 'operator';
+const profiles = config.oidc_enabled
+    ? (results[1]?.items || []).filter(p => p.roles && p.roles.includes(operatorRole))
+    : [];
+```
+
+Update the filter dropdown (lines 149-166):
+- Change label from `t('common.filter_member_label')` to `t('common.filter_operator_label')`
+- Change all-option from `t('common.all_members')` to `t('common.all_operators')`
+
+#### 2c. Advertisements Page
+
+**File:** `src/meshcore_hub/web/static/js/spa/pages/advertisements.js`
+
+Same pattern as Nodes. After fetching profiles (line 72), filter to operators:
+
+```javascript
+const operatorRole = config.role_names?.operator || 'operator';
+const profiles = config.oidc_enabled
+    ? (results[2]?.items || []).filter(p => p.roles && p.roles.includes(operatorRole))
+    : [];
+```
+
+Update the filter dropdown (lines 199-216) with the same label changes.
+
+#### 2d. Map Page
+
+**File:** `src/meshcore_hub/web/static/js/spa/pages/map.js`
+
+The map page gets profiles from `/map/data` (server-side fetch). Two options:
+
+**Option A (recommended): Client-side filtering** — Filter `profiles` after receiving from `/map/data`:
+
+```javascript
+const operatorRole = config.role_names?.operator || 'operator';
+const operatorProfiles = profiles.filter(p => p.roles && p.roles.includes(operatorRole));
+```
+
+Then use `operatorProfiles` in the dropdown instead of `profiles`. Update the dropdown label (line 222) and all-option (line 224).
+
+**Option B: Server-side filtering** — Add a `?operators_only=true` parameter to `/map/data`. More robust but requires changes to the web app route handler.
+
+Chosen: **Option A** — consistent with the other pages, simpler, and the profiles data is already available in the frontend with role information.
+
+> **Note:** The `/map/data` endpoint currently fetches profiles without explicit `exclude_test=false`, so test users are already excluded by the API default. No server-side change needed for test exclusion. However, the profiles returned by `/map/data` only include `id`, `name`, and `callsign` (lines 723-727 of `web/app.py`) — they do NOT include `roles`. This means client-side filtering by role is **not possible** on the map page without a server-side change.
+
+**Updated approach for Map page:** Modify the `/map/data` endpoint in `web/app.py` to include `roles` in the profile data, OR filter server-side to only return operator profiles. The simpler fix is to include `roles` in the profile dict:
+
+```python
+# web/app.py line 723-727, add roles:
+profiles_by_id[profile["id"]] = {
+    "id": profile.get("id"),
+    "name": profile.get("name"),
+    "callsign": profile.get("callsign"),
+    "roles": profile.get("roles", []),  # ADD
+}
+```
+
+Then the frontend can filter by role just like the other pages.
+
+### Summary of Changes
+
+| File | Change |
+|------|--------|
+| `src/meshcore_hub/web/static/js/spa/pages/members.js` | Badge counts `operators.length + members.length` instead of `profiles.length` |
+| `src/meshcore_hub/web/static/js/spa/pages/nodes.js` | Filter profiles to operators; update dropdown labels |
+| `src/meshcore_hub/web/static/js/spa/pages/advertisements.js` | Filter profiles to operators; update dropdown labels |
+| `src/meshcore_hub/web/static/js/spa/pages/map.js` | Filter profiles to operators; update dropdown labels |
+| `src/meshcore_hub/web/app.py` | Include `roles` in profiles dict returned by `/map/data` |
+| `src/meshcore_hub/web/static/locales/en.json` | Add `all_operators` and `filter_operator_label` keys |
+| `src/meshcore_hub/web/static/locales/nl.json` | Add Dutch translations for new keys |
+| `docs/i18n.md` | Document new `all_operators` and `filter_operator_label` keys |
+
+### Tests to Update
+
+| Test File | Change |
+|-----------|--------|
+| `tests/test_common/test_i18n.py` | Add tests for new `all_operators` and `filter_operator_label` keys |
+| `tests/test_web/` | Verify map data includes roles in profiles; verify operator filtering |
+
+### Edge Cases
+
+- **No operators exist** — Filter dropdown should not appear (same as current "no profiles" guard)
+- **User has both operator and admin roles** — Should appear in the operator dropdown (has operator role)
+- **User has member role only** — Should NOT appear in the operator filter dropdown
+- **Map page profile filtering** — Must work even though map profiles have minimal data (just id/name/callsign/roles)
+
+### Out of Scope
+
+- API-level operator filtering (e.g., `?role=operator` parameter on `/api/v1/user/profiles`) — client-side filtering is sufficient
+- Refactoring the filter dropdown into a shared component
+- Changes to the Members page grouping logic (admin-only users still won't appear in either group)
+
+## Implementation Order
+
+1. Add `all_operators` and `filter_operator_label` to `en.json`, `nl.json`, and `docs/i18n.md`
+2. Fix Members page count badge (`members.js`)
+3. Filter to operators in Nodes page (`nodes.js`) and update labels
+4. Filter to operators in Advertisements page (`advertisements.js`) and update labels
+5. Add `roles` to map data profiles (`web/app.py`)
+6. Filter to operators in Map page (`map.js`) and update labels
+7. Update i18n tests
+8. Run `pre-commit run --all-files` and `pytest`
@@ -0,0 +1,53 @@
+# Tasks: Members Count Fix & Operator-Only Filters
+
+## Implementation
+
+- [ ] **T1: Add i18n keys** (`src/meshcore_hub/web/static/locales/en.json`, `nl.json`, `docs/i18n.md`)
+  - Add to `en.json` under `common`: `"all_operators": "All Operators"`, `"filter_operator_label": "Operator"`
+  - Add to `nl.json` under `common`: `"all_operators": "Alle Operators"`, `"filter_operator_label": "Operator"`
+  - Document both keys in `docs/i18n.md` following existing format
+
+- [ ] **T2: Fix Members page count badge** (`src/meshcore_hub/web/static/js/spa/pages/members.js`)
+  - Line 100: Change `profiles.length` to `operators.length + members.length` in the count badge
+
+- [ ] **T3: Filter to operators in Nodes page** (`src/meshcore_hub/web/static/js/spa/pages/nodes.js`)
+  - After fetching profiles (~line 64), filter to operators only:
+    ```javascript
+    const operatorRole = config.role_names?.operator || 'operator';
+    const profiles = config.oidc_enabled
+        ? (results[1]?.items || []).filter(p => p.roles && p.roles.includes(operatorRole))
+        : [];
+    ```
+  - Update filter dropdown (~lines 149-166): change `filter_member_label` → `filter_operator_label`, `all_members` → `all_operators`
+
+- [ ] **T4: Filter to operators in Advertisements page** (`src/meshcore_hub/web/static/js/spa/pages/advertisements.js`)
+  - After fetching profiles (~line 72), filter to operators only:
+    ```javascript
+    const operatorRole = config.role_names?.operator || 'operator';
+    const profiles = config.oidc_enabled
+        ? (results[2]?.items || []).filter(p => p.roles && p.roles.includes(operatorRole))
+        : [];
+    ```
+  - Update filter dropdown (~lines 199-216): change `filter_member_label` → `filter_operator_label`, `all_members` → `all_operators`
+
+- [ ] **T5: Add `roles` to map data profiles** (`src/meshcore_hub/web/app.py`)
+  - In `/map/data` endpoint (~lines 723-727), add `"roles": profile.get("roles", [])` to the `profiles_by_id` dict
+
+- [ ] **T6: Filter to operators in Map page** (`src/meshcore_hub/web/static/js/spa/pages/map.js`)
+  - After receiving profiles from `/map/data`, filter to operators:
+    ```javascript
+    const operatorRole = config.role_names?.operator || 'operator';
+    const operatorProfiles = profiles.filter(p => p.roles && p.roles.includes(operatorRole));
+    ```
+  - Use `operatorProfiles` in the dropdown instead of `profiles`
+  - Update dropdown (~lines 220-232): change `filter_member_label` → `filter_operator_label`, `all_members` → `all_operators`
+
+## Tests
+
+- [ ] **T7: Add i18n tests** (`tests/test_common/test_i18n.py`)
+  - Test `common.all_operators` key exists and resolves correctly
+  - Test `common.filter_operator_label` key exists and resolves correctly
+
+- [ ] **T8: Run quality checks**
+  - `pre-commit run --all-files`
+  - `pytest tests/test_common/ tests/test_web/`
@@ -472,7 +472,11 @@ def create_app(
    app.state.network_welcome_text = (
        network_welcome_text or settings.network_welcome_text
    )
-    raw_announcement = network_announcement or settings.network_announcement
+    raw_announcement = (
+        network_announcement
+        if network_announcement is not None
+        else settings.network_announcement
+    )
    if raw_announcement:
        import markdown

@@ -724,6 +728,7 @@ def create_app(
                        "id": profile.get("id"),
                        "name": profile.get("name"),
                        "callsign": profile.get("callsign"),
+                        "roles": profile.get("roles", []),
                    }

            # Fetch all nodes from API
@@ -69,7 +69,10 @@ ${displayContent}`, container);
            const results = await Promise.all(fetches);
            const data = results[0];
            const nodesData = results[1];
-            const profiles = config.oidc_enabled ? (results[2]?.items || []) : [];
+            const operatorRole = config.role_names?.operator || 'operator';
+            const profiles = config.oidc_enabled
+                ? (results[2]?.items || []).filter(p => p.roles && p.roles.includes(operatorRole))
+                : [];

            const advertisements = data.items || [];
            const total = data.total || 0;
@@ -200,10 +203,10 @@ ${displayContent}`, container);
                filterFields.push(() => html`
            <div class="flex flex-col gap-1 max-w-56">
                <label class="flex items-center py-1">
-                    <span class="opacity-80 text-sm">${t('common.filter_member_label')}</span>
+                    <span class="opacity-80 text-sm">${t('common.filter_operator_label')}</span>
                </label>
                <select name="adopted_by" class="select select-bordered select-sm" @change=${autoSubmit}>
-                    <option value="" ?selected=${!adopted_by}>${t('common.all_members')}</option>
+                    <option value="" ?selected=${!adopted_by}>${t('common.all_operators')}</option>
                    ${profiles.sort((a, b) => {
                        const na = a.name || a.callsign || '';
                        const nb = b.name || b.callsign || '';
@@ -124,6 +124,8 @@ export async function render(container, params, router) {
        const adoptedCenter = data.adopted_center || null;
        const debug = data.debug || {};
        const profiles = data.profiles || [];
+        const operatorRole = config.role_names?.operator || 'operator';
+        const operatorProfiles = profiles.filter(p => p.roles && p.roles.includes(operatorRole));

        const isMobilePortrait = window.innerWidth < 480;
        const isMobile = window.innerWidth < 768;
@@ -217,12 +219,12 @@ export async function render(container, params, router) {
                    <option value="room">${t('node_types.room')}</option>
                </select>
            </div>
-            ${config.oidc_enabled && profiles.length > 0 ? html`
+            ${config.oidc_enabled && operatorProfiles.length > 0 ? html`
            <div class="fieldset">
-                <label class="fieldset-label">${t('common.filter_member_label')}</label>
+                <label class="fieldset-label">${t('common.filter_operator_label')}</label>
                <select id="member-filter" class="select select-bordered select-sm" @change=${applyFilters}>
-                    <option value="">${t('common.all_members')}</option>
-                    ${profiles.sort((a, b) => {
+                    <option value="">${t('common.all_operators')}</option>
+                    ${operatorProfiles.sort((a, b) => {
                        const na = a.name || a.callsign || '';
                        const nb = b.name || b.callsign || '';
                        return na.localeCompare(nb);
@@ -97,7 +97,7 @@ export async function render(container, params, router) {
        litRender(html`
 <div class="flex items-center justify-between mb-6">
    <h1 class="text-3xl font-bold">${t('entities.members')}</h1>
-    <span class="badge badge-lg">${t('common.count_entity', { count: profiles.length, entity: t('entities.members').toLowerCase() })}</span>
+    <span class="badge badge-lg">${t('common.count_entity', { count: operators.length + members.length, entity: t('entities.members').toLowerCase() })}</span>
 </div>

 ${renderGroup(t('members_page.operators'), operators, html`<span class="text-primary">${iconAntenna('h-6 w-6')}</span>`, router)}
@@ -61,7 +61,10 @@ ${displayContent}`, container);
            }
            const results = await Promise.all(fetches);
            const data = results[0];
-            const profiles = config.oidc_enabled ? (results[1]?.items || []) : [];
+            const operatorRole = config.role_names?.operator || 'operator';
+            const profiles = config.oidc_enabled
+                ? (results[1]?.items || []).filter(p => p.roles && p.roles.includes(operatorRole))
+                : [];

            const nodes = data.items || [];
            const total = data.total || 0;
@@ -150,10 +153,10 @@ ${displayContent}`, container);
                filterFields.push(() => html`
            <div class="flex flex-col gap-1 max-w-56">
                <label class="flex items-center py-1">
-                    <span class="opacity-80 text-sm">${t('common.filter_member_label')}</span>
+                    <span class="opacity-80 text-sm">${t('common.filter_operator_label')}</span>
                </label>
                <select name="adopted_by" class="select select-bordered select-sm" @change=${autoSubmit}>
-                    <option value="" ?selected=${!adopted_by}>${t('common.all_members')}</option>
+                    <option value="" ?selected=${!adopted_by}>${t('common.all_operators')}</option>
                    ${profiles.sort((a, b) => {
                        const na = a.name || a.callsign || '';
                        const nb = b.name || b.callsign || '';
@@ -81,7 +81,9 @@
        "all_types": "All Types",
        "all_channels": "All Channels",
        "all_members": "All Members",
+        "all_operators": "All Operators",
        "filter_member_label": "Member",
+        "filter_operator_label": "Operator",
        "filter_observer_label": "Observer",
        "node_type": "Node Type",
        "show": "Show",
@@ -96,7 +96,9 @@
        "from": "Van",
        "close": "sluiten",
        "unnamed": "Naamloos",
-        "unnamed_node": "Naamloos knooppunt"
+        "unnamed_node": "Naamloos knooppunt",
+        "all_operators": "Alle Operators",
+        "filter_operator_label": "Operator"
    },
    "links": {
        "website": "Website",
@@ -121,6 +121,11 @@ class TestEnJsonCompleteness:
        assert t("common.no_entity_yet", entity="test") == "No test yet"
        assert t("common.page_not_found") == "Page not found"

+    def test_common_operator_filter_keys(self):
+        """Test that operator filter keys exist and resolve correctly."""
+        assert t("common.all_operators") == "All Operators"
+        assert t("common.filter_operator_label") == "Operator"
+
    def test_entity_keys(self):
        """Entity keys are all present."""
        assert t("entities.home") != "entities.home"
@@ -322,6 +322,7 @@ def web_app(mock_http_client: MockHttpClient, monkeypatch: pytest.MonkeyPatch) -
    monkeypatch.setenv("WEB_DATETIME_LOCALE", "en-US")
    monkeypatch.setenv("COLLECTOR_INCLUDE_TEST_CHANNEL", "true")
    monkeypatch.setenv("OIDC_ENABLED", "false")
+    monkeypatch.setenv("NETWORK_ANNOUNCEMENT", "")
    app = create_app(
        api_url="http://localhost:8000",
        api_key="test-api-key",
@@ -366,6 +367,7 @@ def web_app_with_oidc(
    monkeypatch.setenv("OIDC_SESSION_SECRET", "test-session-secret")
    monkeypatch.setenv("WEB_DATETIME_LOCALE", "en-US")
    monkeypatch.setenv("COLLECTOR_INCLUDE_TEST_CHANNEL", "true")
+    monkeypatch.setenv("NETWORK_ANNOUNCEMENT", "")

    app = create_app(
        api_url="http://localhost:8000",