diff --git a/frontend/src/api/index.js b/frontend/src/api/index.js index 89029cc0..6c7a0da0 100644 --- a/frontend/src/api/index.js +++ b/frontend/src/api/index.js @@ -405,6 +405,12 @@ export const vendorsApi = { list(params = {}) { return api.get('/vendors', { params }) }, + // Every vendor, paged. perpage is capped at 100 server-side, so a picker + // asking for 500 silently showed the first 100 and dropped the rest - with a + // 200, so nothing looked wrong. + listAll(params = {}) { + return fetchAllPages('/vendors', params) + }, get(id) { return api.get(`/vendors/${id}`) }, @@ -667,6 +673,10 @@ export const businessunitsApi = { list(params = {}) { return api.get('/businessunits', { params }) }, + // See vendorsApi.listAll: the cap is 100 whatever the caller asks for. + listAll(params = {}) { + return fetchAllPages('/businessunits', params) + }, get(id) { return api.get(`/businessunits/${id}`) }, diff --git a/plugins/computers/frontend/views/ConfigurationPoliciesList.vue b/plugins/computers/frontend/views/ConfigurationPoliciesList.vue index 24c37ab4..8e014be7 100644 --- a/plugins/computers/frontend/views/ConfigurationPoliciesList.vue +++ b/plugins/computers/frontend/views/ConfigurationPoliciesList.vue @@ -145,8 +145,8 @@ async function loadData() { // 500'd, which looked like data loss. const [pol, types, appList, actions, monitorList] = await Promise.allSettled([ computersApi.policies.configuration.list(), - computersApi.types.list({ perpage: 200 }), - applicationsApi.listAll({ perpage: 200 }), + computersApi.types.listAll(), + applicationsApi.listAll(), computersApi.policies.actions(), endpointsApi.monitors.list() ]) diff --git a/plugins/computers/frontend/views/PCTypesList.vue b/plugins/computers/frontend/views/PCTypesList.vue index 453a011f..5afef7e4 100644 --- a/plugins/computers/frontend/views/PCTypesList.vue +++ b/plugins/computers/frontend/views/PCTypesList.vue @@ -107,7 +107,7 @@ onMounted(() => loadData()) async function loadData() { loading.value = true try { - const response = await computersApi.types.list({ perpage: 200, active: false }) + const response = await computersApi.types.listAll({ active: false }) pcTypes.value = response.data.data || [] } catch (err) { console.error('Error loading PC types:', err) diff --git a/plugins/machines/frontend/views/MachineForm.vue b/plugins/machines/frontend/views/MachineForm.vue index 6350c449..bce1693a 100644 --- a/plugins/machines/frontend/views/MachineForm.vue +++ b/plugins/machines/frontend/views/MachineForm.vue @@ -581,10 +581,10 @@ onMounted(async () => { const [typesRes, statusRes, vendorRes, locRes, allModels, buRes, pcsRes, relTypesRes] = await Promise.all([ machinesApi.types.list(), assetsApi.statuses.list(), - vendorsApi.list({ perpage: 500 }), + vendorsApi.listAll(), locationsApi.listAll(), // backend caps perpage at 100; page through all modelsApi.listAll({ category: 'Equipment' }), // backend caps perpage at 100; page through all - businessunitsApi.list({ perpage: 500 }), + businessunitsApi.listAll(), computersApi.listAll(), // backend caps perpage at 100; page through all assetsApi.types.list() // Used for relationship types, will fix below ]) diff --git a/plugins/machines/frontend/views/MachineTypesList.vue b/plugins/machines/frontend/views/MachineTypesList.vue index bc69909b..d19f5eeb 100644 --- a/plugins/machines/frontend/views/MachineTypesList.vue +++ b/plugins/machines/frontend/views/MachineTypesList.vue @@ -95,7 +95,7 @@ onMounted(loadData) async function loadData() { loading.value = true try { - const response = await machinesApi.types.list({ perpage: 200, active: false }) + const response = await machinesApi.types.list({ perpage: 100, active: false }) items.value = response.data.data || [] } catch (err) { console.error('Error loading machine types:', err) diff --git a/plugins/measuringtools/frontend/views/MeasuringToolForm.vue b/plugins/measuringtools/frontend/views/MeasuringToolForm.vue index bb26a61b..5b18eb76 100644 --- a/plugins/measuringtools/frontend/views/MeasuringToolForm.vue +++ b/plugins/measuringtools/frontend/views/MeasuringToolForm.vue @@ -179,10 +179,10 @@ const businessunits = ref([]) onMounted(async () => { try { const [typesResponse, statusResponse, locResponse, buResponse, assetTypesResponse] = await Promise.all([ - measuringtoolsApi.types.list({ perpage: 200 }), + measuringtoolsApi.types.list({ perpage: 100 }), assetsApi.statuses.list(), locationsApi.listAll(), // backend caps perpage at 100; page through all - businessunitsApi.list({ perpage: 500 }), + businessunitsApi.listAll(), assetsApi.types.list(), ]) types.value = typesResponse.data.data || [] diff --git a/plugins/measuringtools/frontend/views/MeasuringToolTypesList.vue b/plugins/measuringtools/frontend/views/MeasuringToolTypesList.vue index b16919c1..559f7b40 100644 --- a/plugins/measuringtools/frontend/views/MeasuringToolTypesList.vue +++ b/plugins/measuringtools/frontend/views/MeasuringToolTypesList.vue @@ -107,7 +107,7 @@ onMounted(() => loadData()) async function loadData() { loading.value = true try { - const response = await measuringtoolsApi.types.list({ perpage: 200, active: false }) + const response = await measuringtoolsApi.types.list({ perpage: 100, active: false }) types.value = response.data.data || [] } catch (err) { console.error('Error loading measuring tool types:', err) diff --git a/plugins/measuringtools/frontend/views/MeasuringToolsList.vue b/plugins/measuringtools/frontend/views/MeasuringToolsList.vue index 14be7716..52d18bf5 100644 --- a/plugins/measuringtools/frontend/views/MeasuringToolsList.vue +++ b/plugins/measuringtools/frontend/views/MeasuringToolsList.vue @@ -134,7 +134,7 @@ function formatDate(d) { return new Date(d + 'T00:00:00').toLocaleDateString() } onMounted(async () => { try { - const typesResponse = await measuringtoolsApi.types.list({ perpage: 200 }) + const typesResponse = await measuringtoolsApi.types.list({ perpage: 100 }) types.value = typesResponse.data.data || [] } catch (err) { console.error('Error loading types:', err) diff --git a/plugins/network/frontend/views/NetworkTypesList.vue b/plugins/network/frontend/views/NetworkTypesList.vue index cb430bab..85ee1e2c 100644 --- a/plugins/network/frontend/views/NetworkTypesList.vue +++ b/plugins/network/frontend/views/NetworkTypesList.vue @@ -112,7 +112,7 @@ onMounted(loadData) async function loadData() { loading.value = true try { - const response = await networkApi.types.list({ perpage: 200, active: false }) + const response = await networkApi.types.list({ perpage: 100, active: false }) items.value = response.data.data || [] } catch (err) { console.error('Error loading network device types:', err) diff --git a/plugins/printers/frontend/views/PrinterTypesList.vue b/plugins/printers/frontend/views/PrinterTypesList.vue index 40c4241b..aad5b1d7 100644 --- a/plugins/printers/frontend/views/PrinterTypesList.vue +++ b/plugins/printers/frontend/views/PrinterTypesList.vue @@ -95,7 +95,7 @@ onMounted(loadData) async function loadData() { loading.value = true try { - const response = await printersApi.types.list({ perpage: 200, active: false }) + const response = await printersApi.types.list({ perpage: 100, active: false }) items.value = response.data.data || [] } catch (err) { console.error('Error loading printer types:', err) diff --git a/shopdb/core/api/auditlogs.py b/shopdb/core/api/auditlogs.py index 544fa78f..55ccf238 100644 --- a/shopdb/core/api/auditlogs.py +++ b/shopdb/core/api/auditlogs.py @@ -5,7 +5,8 @@ from flask_jwt_extended import jwt_required from shopdb.core.models import AuditLog from shopdb.extensions import db -from shopdb.utils.responses import success_response, error_response, ErrorCodes +from shopdb.utils.responses import (success_response, error_response, + paginated_response, ErrorCodes) from shopdb.utils.authz import require_permission auditlogs_bp = Blueprint('auditlogs', __name__) @@ -123,15 +124,12 @@ def list_auditlogs(): for r in rows: r['userfullname'] = fullnames.get(str(r.get('username'))) - return success_response( - rows, - meta={ - 'page': page, - 'perpage': perpage, - 'total': pagination.total, - 'pages': pagination.pages - } - ) + # paginated_response, like every other list in the product. This one built + # its own meta with the fields FLAT and named 'pages', so a client written + # against the documented meta.pagination.{page,perpage,total,totalpages} + # found nothing where it looked - on the one list whose whole purpose is to + # be read by somebody checking what happened. + return paginated_response(rows, page, perpage, pagination.total) @auditlogs_bp.route('/entity//', methods=['GET']) diff --git a/tests/test_core/test_perpage_cap.py b/tests/test_core/test_perpage_cap.py new file mode 100644 index 00000000..45f63dcf --- /dev/null +++ b/tests/test_core/test_perpage_cap.py @@ -0,0 +1,79 @@ +"""No client may ask for a page bigger than the server will give. + +MAX_PAGE_SIZE caps perpage at 100. The cap is deliberate, and it is applied +SILENTLY: a request for perpage=500 returns the first 100 rows with a 200, so +nothing anywhere reports a problem. A picker built that way shows the first 100 +options and drops the rest, and the person using it has no way to tell - the +missing vendor simply is not in the list. + +That shape has been found and fixed more than once, which is what makes it worth +a gate rather than a comment. Paging through is what fetchAllPages/listAll are +for. + +The check reads the frontend sources rather than the running app, because the +defect is in what the caller ASKS for: the response is a valid 200 either way, +so no runtime assertion can see it. +""" +import re +from pathlib import Path + +REPO = Path(__file__).resolve().parents[2] + +# Where a client is written. The staged copy under frontend/src/.plugins-staged +# is generated from plugins/*/frontend at build time, so checking the source +# avoids reporting the same line twice. +SOURCE_DIRS = [ + REPO / 'frontend' / 'src', + *sorted((REPO / 'plugins').glob('*/frontend')), +] + +SKIP_PARTS = {'.plugins-staged', 'node_modules', 'dist'} + +# perpage: , in an object literal or a query string. +PERPAGE = re.compile(r'perpage[\'"]?\s*[:=]\s*[\'"]?(\d+)', re.IGNORECASE) + +MAX_PAGE_SIZE = 100 + + +def _sources(): + for root in SOURCE_DIRS: + if not root.exists(): + continue + for path in root.rglob('*'): + if path.suffix not in ('.vue', '.js'): + continue + if set(path.parts) & SKIP_PARTS: + continue + if path.name.endswith('.spec.js'): + continue + yield path + + +def test_the_sweep_actually_reads_something(): + """A gate that matches nothing passes forever.""" + files = list(_sources()) + assert len(files) > 100, len(files) + assert any(PERPAGE.search(path.read_text()) for path in files), \ + 'no perpage usage found at all - the pattern is probably wrong' + + +def test_no_client_asks_for_more_rows_than_the_server_returns(): + offenders = [] + for path in _sources(): + for lineno, line in enumerate(path.read_text().splitlines(), start=1): + # A comment explaining the cap is not a caller asking for 1000 + # rows. Crude but sufficient: this is a line-oriented check over + # source that follows one style. + stripped = line.strip() + if stripped.startswith('//') or stripped.startswith('*'): + continue + for match in PERPAGE.finditer(line): + if int(match.group(1)) > MAX_PAGE_SIZE: + offenders.append('%s:%d %s' + % (path.relative_to(REPO), lineno, + line.strip()[:90])) + assert not offenders, ( + 'perpage is capped at %d server-side and the excess is dropped ' + 'silently, so these show a truncated list with no error. Use listAll() ' + '/ fetchAllPages() instead:\n %s' + % (MAX_PAGE_SIZE, '\n '.join(offenders)))