feat(plugins): search, filter and sort for Installed Plugins, on a shared ListFilter helper (#540)

* feat(plugins): add search, filter and sort to Installed Plugins, on a shared helper

The Installed Plugins grid had no way to narrow it down: no search, no way to
see only what's enabled, disabled, or out of date. On a rig with a couple dozen
plugins that means scrolling the whole grid to find one.

The two sections below it already solved this, twice, independently — the
Plugin Store and Starlark Apps carried a copy-paste fork of the same ~600 lines
(filter state, apply-filters-and-sort, page renderer, pagination strip,
active-filter badge, listener wiring). Rather than add a third copy, this
extracts the shared machinery and builds the new toolbar on it.

New: web_interface/static/v3/js/plugins/list_filter.js — ListFilter.create()
owns debounced search, filter axes, sort, the active-filter count, Clear, and
optional pagination/persistence. Callers keep their own card markup via a
`render` callback. Three control types cover every axis the page uses: pills
(new), select (store category, starlark author) and cycle (the tri-state
All -> Installed -> Not Installed button).

Installed Plugins gets a compact toolbar: search box, one-click All / Enabled /
Disabled / Updates pills, and a sort dropdown (A-Z, Z-A, updates first,
recently updated, category). Filters reset on load, so you never come back to a
mysteriously short list. No new CSS — this is the first consumer of the
.filter-pill rules already sitting unused in app.css.

renderInstalledPlugins() is split so it still publishes canonical state while
renderInstalledCards() draws only the visible subset; the filtered list is
never assigned to window.installedPlugins, which the toggle handler,
isStorePluginInstalled(), runUpdateAllPlugins() and the Alpine config tabs all
read as their source of truth. Toggling a plugin while filtered pins its card
so it doesn't vanish from under the cursor.

The Store and Starlark migrations are behaviour-preserving: same element ids,
same localStorage keys (storeSort/storePerPage, starlarkSort/starlarkPerPage),
same tri-state button markup, same pagination. Verified by differential tests
that run the old and new implementations side by side against identical
fixtures and compare every observable after each interaction. The only visible
change is the pagination attribute (data-store-page/data-starlark-page ->
data-list-page), which nothing outside its own click handler referenced.

Net -156 lines in plugins_manager.js while adding a feature.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9

* fix(plugins): keep raw search text, and stop the store search refetching

Two review findings from CodeRabbit on #540.

Do not write the trimmed search value back into the input. setSearch() trimmed
before storing, and syncControls() then copied that trimmed value back over what
the user had typed. Pausing longer than the debounce after typing a space
deleted the space (and reset the caret), making multi-word terms effectively
untypable. The raw text is now kept alongside the trimmed one: filtering and
activeCount() still use the trimmed value, while the input keeps exactly what
was typed.

Remove the legacy #plugin-search / #plugin-category listeners in
initializePlugins(). They bound searchPluginStore as the event handler, so the
DOM event arrived as its `fetchCommitInfo` argument — always truthy, which
skipped the cached-filter fast path and refetched /api/v3/plugins/store/list
with commit info on every keystroke burst and category change. The store's
ListFilter controller already filters the cached list, which is what those two
controls should do. This double-binding predates this PR (the old code guarded
with _listenerSetup and _storeFilterInit, two different flags, so both sets
stayed live); it is fixed here because the refactor owns that wiring now.

Both fixes are covered by tests that fail without them: the trailing-space
regressions in the installed-plugins DOM suite, and a new whole-file jsdom test
that counts fetches while typing (1 request at init, 0 thereafter; previously
1 -> 2 -> 3 -> 5).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9

* fix(plugins): build pagination via DOM APIs, drop computed member access

Addresses the five Codacy security findings, all in list_filter.js.

Pagination no longer assembles an HTML string (3 findings: 2 critical + 1 high,
"unsafe assignment to innerHTML"). The interpolated values were only page
integers and local class constants, so there was no injection path, but
concatenating markup into innerHTML is the pattern the scanners flag and
createElement is no less clear. Each button now also owns its click listener
directly instead of the container being re-queried afterwards, and the strip is
cleared with textContent = '' rather than by assigning empty markup. No
innerHTML assignment remains in the file.

haystack() now walks Object.entries(item) and keeps the configured fields,
instead of reading item[field] per field ("generic object injection sink").
Field order no longer drives the haystack order, which is irrelevant to the
substring test. matches() iterates controls with for...of instead of an index
("variable assigned to object injection sink").

The rendered pagination is unchanged: same buttons, labels, page numbers,
disabled states and classes. The old-vs-new differential tests now compare
pagination structurally (tag, text, page, disabled, sorted class list) rather
than as an HTML string, since building nodes legitimately serialises
differently — «/» as characters rather than &laquo;/&raquo;, disabled="" rather
than a bare attribute. That comparison is stronger than the string one it
replaces, and the real-DOM suite still drives the actual page buttons.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9

* fix(plugins): keep configured field order when building the search haystack

The previous commit swapped item[field] for Object.entries(item) to clear a
static-analysis object-injection warning, and in doing so changed the order of
the haystack: entries follow the object's own key insertion order, not the
configured `fields` order. Since the values are concatenated, that order decides
which values end up adjacent, so a multi-word query spanning a field boundary
matched differently. For store fields [name, description, author, id, ...] and
API objects keyed {id, name, description, author, ...}, "bob plugin-01" matched
before and stopped matching after.

That contradicted the behaviour-preservation claim for the store and starlark
migrations, and the differential tests missed it because every fixture query was
a single word.

Values now come out of a Map built from Object.entries, iterated in `fields`
order: the original haystack is restored, and there is still no computed member
access for the analyser to flag.

Regression coverage for the ordering itself, at both levels:
  - unit: phrases spanning name->id and category->tags, plus the reverse
    (object-key) order asserted NOT to match
  - differential: the same class of query compared old-vs-new, with a guard that
    the phrase actually matches something so a mutual zero-result cannot pass
    vacuously

Verified both fail without this fix (3 unit, 2 differential) and pass with it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9

* test(web): add JS suites for ListFilter and the plugin-manager grids

No JS toolchain exists in this repo, so these are plain node scripts with no
framework: each prints ok/FAIL lines and exits non-zero. `node test/js/run_all.js`
runs everything, skipping the DOM suites (rather than failing) when jsdom is
absent or nothing is listening, so it stays useful in a bare checkout.

  unit/test_list_filter.js    ListFilter search/filter/sort/count/sticky, driven
                              through the installed-plugins config eval'd
                              verbatim out of plugins_manager.js so the test
                              cannot drift from the real configuration
  unit/test_render_cards.js   renderInstalledCards markup, both empty states,
                              and escaping of hostile plugin metadata
  dom/test_installed_dom.js   the toolbar in a real DOM, including the HTMX
                              partial re-swap and a getComputedStyle check that
                              .filter-pill[data-active] matches what we emit
  dom/test_store_dom.js       store pagination, per-page, category, tri-state
                              Installed button, persistence across a re-boot
  dom/test_no_double_fetch.js loads the whole plugins_manager.js and counts
                              requests, so a keystroke cannot refetch the store

The DOM suites deliberately fetch the partial and the plugin data from a running
web interface instead of using fixtures, so a renamed element id or a changed
payload shape fails them loudly. Point them at a rig with a full plugin set when
it matters (BASE=http://host:5000); a dev box with two plugins installed passes
while exercising very little.

Several assertions exist to stop specific bugs recurring: trailing spaces
surviving the search debounce, a query spanning two adjacent search fields
(haystack field order is load-bearing), and window.installedPlugins staying at
full length while the grid is filtered. Others guard against passing vacuously —
counting only non-skeleton cards, and checking a search phrase matches something
before comparing two result sets.

The old-vs-new differential suites that verified the store and starlark
migrations are not included: they compared against the pre-refactor code, which
now exists only in git history.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Chuck
2026-09-08 20:04:47 -04:00
committed by GitHub
co-authored by Claude Opus 5
parent 26769ee37f
commit 4423ec33d5
13 changed files with 2160 additions and 617 deletions
@@ -0,0 +1,484 @@
/**
* ListFilter — shared search / filter / sort controller for the card-grid
* sections of the Plugin Manager.
*
* The Installed Plugins, Plugin Store and Starlark Apps sections all need the
* same machinery: debounced text search over a few fields, a handful of filter
* axes, a sort dropdown, an active-filter count with a Clear button, and a
* re-render. This owns that machinery; the caller keeps ownership of its own
* card markup via the `render` callback.
*
* Usage:
*
* const ctl = ListFilter.create({
* getItems: () => window.installedPlugins || [],
* render: (visible, total) => renderCards(visible, total),
* search: { el: 'my-search', fields: ['name', 'id', 'tags'] },
* sort: { el: 'my-sort', default: 'a-z', comparators: { 'a-z': fn } },
* controls: [{ type: 'pills', el: '#my-pills', attr: 'data-my-filter',
* key: 'filter', default: 'all', test: (item, v) => true }],
* clearEl: 'my-clear',
* });
* ctl.bind(); // idempotent — safe to call after every HTMX partial swap
* ctl.apply(); // filter + sort + render
*
* Optional `pagination` slices the result set and renders page controls; the
* `render` callback then receives just the current page. Optional `persist`
* takes read/write callbacks so the caller — not this helper — owns its
* storage keys.
*
* Element references are DOM ids, except `controls[].el` which is a CSS
* selector for the pill container.
*/
const ListFilter = (function () {
'use strict';
function debounce(fn, wait) {
let timer = null;
return function (...args) {
clearTimeout(timer);
timer = setTimeout(() => fn.apply(this, args), wait);
};
}
function byId(id) {
return id ? document.getElementById(id) : null;
}
// Filter axes compare against their default to decide "is this axis active",
// so null/undefined/'' must not be conflated with a real selection.
function sameValue(a, b) {
if (a === b) return true;
if (a === null || a === undefined) return b === null || b === undefined;
return false;
}
// Build the lowercased search haystack. Array fields (e.g. tags) are
// flattened in, matching the existing store/starlark search behaviour.
//
// Values are read out of a Map rather than via item[field], which keeps
// static analysers from flagging a computed member access as an
// object-injection sink. Iteration follows `fields`, NOT the object's own
// key order: the fields are concatenated, so their order decides which
// values end up adjacent, and a multi-word query can span a field boundary.
function haystack(item, fields) {
if (!item) return '';
const values = new Map(Object.entries(item));
const parts = [];
(fields || []).forEach(field => {
const value = values.get(field);
if (Array.isArray(value)) {
value.forEach(v => { if (v) parts.push(String(v)); });
} else if (value) {
parts.push(String(value));
}
});
return parts.join(' ').toLowerCase();
}
function create(config) {
const cfg = config || {};
const searchCfg = cfg.search || null;
const sortCfg = cfg.sort || null;
const controls = Array.isArray(cfg.controls) ? cfg.controls : [];
const pageCfg = cfg.pagination || null;
const persistCfg = cfg.persist || null;
const idOf = typeof cfg.idOf === 'function' ? cfg.idOf : (item => item && item.id);
// Defaults double as the "inactive" value for each axis.
const defaults = {};
if (searchCfg) {
defaults.search = ''; // trimmed — what filtering and activeCount use
defaults.searchRaw = ''; // exactly what the user typed — what the input shows
}
if (sortCfg) defaults.sort = sortCfg.default !== undefined ? sortCfg.default : 'a-z';
controls.forEach(c => {
defaults[c.key] = c.default !== undefined ? c.default : null;
});
const state = Object.assign({}, defaults);
// page/perPage sit outside `defaults` on purpose: Clear Filters returns
// to page 1 but must NOT reset a per-page size the user chose.
if (pageCfg) {
state.page = 1;
state.perPage = pageCfg.defaultPerPage || 12;
}
// Seed persisted values. The caller supplies read()/write() so storage
// keys stay where they always were.
if (persistCfg && typeof persistCfg.read === 'function') {
const saved = persistCfg.read() || {};
if (sortCfg && saved.sort !== undefined && saved.sort !== null) state.sort = saved.sort;
if (pageCfg && saved.perPage) state.perPage = saved.perPage;
}
function persist() {
if (persistCfg && typeof persistCfg.write === 'function') persistCfg.write(state);
}
// Ids that stay visible even when they no longer match the active
// filters. Populated by the caller when the user acts on a card (e.g.
// toggling a plugin off while filtering by Enabled) so the card they
// just clicked doesn't vanish underneath the cursor. Cleared as soon as
// the user touches the toolbar.
const sticky = new Set();
function activeCount() {
let n = 0;
if (searchCfg && state.search) n++;
if (sortCfg && !sameValue(state.sort, defaults.sort)) n++;
controls.forEach(c => {
if (!sameValue(state[c.key], defaults[c.key])) n++;
});
return n;
}
function matches(item) {
if (searchCfg && state.search) {
if (!haystack(item, searchCfg.fields).includes(state.search.toLowerCase())) {
return false;
}
}
for (const c of controls) {
const value = state[c.key];
if (sameValue(value, defaults[c.key])) continue; // axis inactive
if (typeof c.test === 'function' && !c.test(item, value)) return false;
}
return true;
}
function compute() {
const all = (typeof cfg.getItems === 'function' ? cfg.getItems() : null) || [];
const total = all.length;
const list = all.filter(item => {
if (sticky.size > 0 && sticky.has(idOf(item))) return true;
return matches(item);
});
if (sortCfg && sortCfg.comparators) {
// An unrecognised sort key falls back to the default comparator,
// matching the switch-with-default the store code used.
const cmp = sortCfg.comparators[state.sort] || sortCfg.comparators[defaults.sort];
if (typeof cmp === 'function') list.sort(cmp);
}
return { list: list, total: total };
}
// Reflect current state back onto the controls, so programmatic changes
// and a fresh partial swap both land on a correctly-lit toolbar.
function syncControls() {
if (searchCfg) {
// The toolbar markup is rebuilt on every HTMX partial swap while
// this controller (and its state) survives — put the text back.
const el = byId(searchCfg.el);
const text = state.searchRaw !== undefined ? state.searchRaw : state.search;
if (el && el.value !== text) el.value = text;
}
if (sortCfg) {
const el = byId(sortCfg.el);
if (el && el.value !== state.sort) el.value = state.sort;
}
if (pageCfg && pageCfg.perPageEl) {
const el = byId(pageCfg.perPageEl);
if (el && el.value !== String(state.perPage)) el.value = String(state.perPage);
}
controls.forEach(c => {
const value = state[c.key];
if (c.type === 'pills') {
const container = document.querySelector(c.el);
if (!container) return;
container.querySelectorAll('[' + c.attr + ']').forEach(btn => {
const on = btn.getAttribute(c.attr) === String(value);
btn.setAttribute('data-active', on ? 'true' : 'false');
btn.setAttribute('aria-pressed', on ? 'true' : 'false');
});
} else if (c.type === 'select') {
const el = byId(c.el);
if (el && el.value !== (value === null ? '' : value)) {
el.value = value === null ? '' : value;
}
} else if (c.type === 'cycle') {
const btn = byId(c.el);
// The caller renders cycle buttons so each section keeps its
// own label/icon/class treatment.
if (btn && typeof c.render === 'function') c.render(btn, value);
}
});
}
function updateChrome(list, total) {
const n = activeCount();
const countEl = byId(cfg.countEl);
if (countEl && typeof cfg.countFormat === 'function') {
countEl.textContent = cfg.countFormat(list.length, total, n > 0);
}
const activeEl = byId(cfg.activeCountEl);
if (activeEl) {
activeEl.classList.toggle('hidden', n === 0);
activeEl.textContent = n + ' filter' + (n !== 1 ? 's' : '') + ' active';
}
const clearEl = byId(cfg.clearEl);
if (clearEl) clearEl.classList.toggle('hidden', n === 0);
if (searchCfg && searchCfg.clearEl) {
const searchClear = byId(searchCfg.clearEl);
if (searchClear) searchClear.classList.toggle('hidden', !state.search);
}
syncControls();
if (typeof cfg.onChrome === 'function') cfg.onChrome(state, list, total);
}
// Page-number strip with leading/trailing ellipsis, producing the same
// controls the plugin store has always rendered.
//
// Built with createElement rather than by concatenating an HTML string.
// Nothing interpolated here is user-controlled — only page integers and
// these class constants — but assembling markup into innerHTML is the
// pattern static analysers flag as an XSS sink, and building nodes is no
// less clear. It also lets each button own its listener directly instead
// of re-querying the container afterwards.
const PAGE_BTN_CLASS = 'px-3 py-1 text-sm rounded-md border transition-colors';
const PAGE_ACTIVE_CLASS = 'bg-blue-600 text-white border-blue-600';
const PAGE_NORMAL_CLASS = 'bg-white text-gray-700 border-gray-300 hover:bg-gray-100 cursor-pointer';
const PAGE_DISABLED_CLASS = 'bg-gray-100 text-gray-400 border-gray-200 cursor-not-allowed';
function renderPagination(containerId, totalPages, currentPage) {
const container = byId(containerId);
if (!container) return;
// textContent = '' drops the previous strip without parsing markup.
container.textContent = '';
if (totalPages <= 1) return;
const goTo = target => {
if (target >= 1 && target <= totalPages && target !== currentPage) {
state.page = target;
// Page moves re-slice only; filters and sort are unchanged.
apply(true);
const grid = byId(pageCfg && pageCfg.scrollToEl);
if (grid && typeof grid.scrollIntoView === 'function') {
grid.scrollIntoView({ behavior: 'smooth', block: 'start' });
}
}
};
const addPageButton = (label, target, variant) => {
const btn = document.createElement('button');
btn.className = PAGE_BTN_CLASS + ' ' + (
variant === 'active' ? PAGE_ACTIVE_CLASS
: variant === 'disabled' ? PAGE_DISABLED_CLASS
: PAGE_NORMAL_CLASS);
btn.setAttribute('data-list-page', String(target));
if (variant === 'disabled') btn.disabled = true;
btn.textContent = label;
btn.addEventListener('click', () => goTo(target));
container.appendChild(btn);
};
addPageButton('\u00ab', currentPage - 1, currentPage <= 1 ? 'disabled' : 'normal');
const pages = [];
pages.push(1);
if (currentPage > 3) pages.push('...');
for (let i = Math.max(2, currentPage - 1); i <= Math.min(totalPages - 1, currentPage + 1); i++) {
pages.push(i);
}
if (currentPage < totalPages - 2) pages.push('...');
if (totalPages > 1) pages.push(totalPages);
pages.forEach(entry => {
if (entry === '...') {
const gap = document.createElement('span');
gap.className = 'px-2 py-1 text-sm text-gray-400';
gap.textContent = '\u2026';
container.appendChild(gap);
} else {
addPageButton(String(entry), entry, entry === currentPage ? 'active' : 'normal');
}
});
addPageButton('\u00bb', currentPage + 1, currentPage >= totalPages ? 'disabled' : 'normal');
}
function apply(skipPageReset) {
const result = compute();
if (!pageCfg) {
updateChrome(result.list, result.total);
if (typeof cfg.render === 'function') cfg.render(result.list, result.total);
return result;
}
if (!skipPageReset) state.page = 1;
const total = result.list.length;
const totalPages = Math.max(1, Math.ceil(total / state.perPage));
if (state.page > totalPages) state.page = totalPages;
const start = (state.page - 1) * state.perPage;
const end = Math.min(start + state.perPage, total);
const pageItems = result.list.slice(start, end);
const info = total > 0
? (typeof pageCfg.infoFormat === 'function'
? pageCfg.infoFormat(start + 1, end, total)
: `Showing ${start + 1}\u2013${end} of ${total}`)
: (pageCfg.emptyText || 'No results match your filters');
[pageCfg.infoEl, pageCfg.infoBottomEl].forEach(id => {
const el = byId(id);
if (el) el.textContent = info;
});
renderPagination(pageCfg.topEl, totalPages, state.page);
renderPagination(pageCfg.bottomEl, totalPages, state.page);
updateChrome(result.list, result.total);
if (typeof cfg.render === 'function') cfg.render(pageItems, result.total);
return result;
}
function setSearch(value) {
// Keep the raw text so syncControls can put it back verbatim. Writing
// the trimmed value into the input would eat a trailing space (and
// reset the caret) mid-word, which makes multi-word terms untypable.
state.searchRaw = value || '';
state.search = state.searchRaw.trim();
sticky.clear();
apply();
}
function reset() {
// Only the filter axes reset; a chosen page size is a preference,
// not a filter, so it survives Clear Filters.
Object.assign(state, defaults);
if (pageCfg) state.page = 1;
sticky.clear();
if (searchCfg) {
const el = byId(searchCfg.el);
if (el) el.value = '';
}
persist();
syncControls();
apply();
}
function bind() {
if (searchCfg) {
const input = byId(searchCfg.el);
if (input && !input._listFilterInit) {
input._listFilterInit = true;
const run = debounce(() => setSearch(input.value), searchCfg.debounceMs || 300);
input.addEventListener('input', run);
input.addEventListener('keydown', e => {
if (e.key === 'Escape') {
input.value = '';
setSearch('');
}
});
}
const searchClear = searchCfg.clearEl ? byId(searchCfg.clearEl) : null;
if (searchClear && !searchClear._listFilterInit) {
searchClear._listFilterInit = true;
searchClear.addEventListener('click', () => {
const el = byId(searchCfg.el);
if (el) el.value = '';
setSearch('');
});
}
}
if (sortCfg) {
const el = byId(sortCfg.el);
if (el && !el._listFilterInit) {
el._listFilterInit = true;
el.addEventListener('change', function () {
state.sort = this.value;
sticky.clear();
persist();
apply();
});
}
}
controls.forEach(c => {
if (c.type === 'pills') {
const container = document.querySelector(c.el);
if (!container || container._listFilterInit) return;
container._listFilterInit = true;
// Delegated, so the pills survive any markup re-render.
container.addEventListener('click', event => {
const btn = event.target.closest('[' + c.attr + ']');
if (!btn || !container.contains(btn)) return;
state[c.key] = btn.getAttribute(c.attr);
sticky.clear();
apply();
});
} else if (c.type === 'select') {
const el = byId(c.el);
if (!el || el._listFilterInit) return;
el._listFilterInit = true;
el.addEventListener('change', function () {
state[c.key] = this.value;
sticky.clear();
apply();
});
} else if (c.type === 'cycle') {
const btn = byId(c.el);
if (!btn || btn._listFilterInit) return;
btn._listFilterInit = true;
const values = Array.isArray(c.values) ? c.values : [null];
btn.addEventListener('click', () => {
const at = values.findIndex(v => sameValue(v, state[c.key]));
state[c.key] = values[(at + 1) % values.length];
sticky.clear();
apply();
});
}
});
if (pageCfg && pageCfg.perPageEl) {
const el = byId(pageCfg.perPageEl);
if (el && !el._listFilterInit) {
el._listFilterInit = true;
el.addEventListener('change', function () {
state.perPage = parseInt(this.value) || (pageCfg.defaultPerPage || 12);
persist();
apply();
});
}
}
const clearEl = byId(cfg.clearEl);
if (clearEl && !clearEl._listFilterInit) {
clearEl._listFilterInit = true;
clearEl.addEventListener('click', reset);
}
}
return {
state: state,
sticky: sticky,
activeCount: activeCount,
bind: bind,
apply: apply,
reset: reset,
setSearch: setSearch,
syncControls: syncControls,
};
}
return { create: create };
})();
// Export
if (typeof module !== 'undefined' && module.exports) {
module.exports = ListFilter;
} else {
window.ListFilter = ListFilter;
}
File diff suppressed because it is too large Load Diff