diff --git a/CHANGELOG.md b/CHANGELOG.md index 5956126e..8aae7a87 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,27 @@ ADR-007 and ADR-002. ### Fixed +- **Every report offers to email itself, not just two of them.** `POST + /api/reports/email` has always taken any `{columns, rows}`, but the button was + on the toner and warranty pages only - because those two happen to be their + own routed pages, and the reports page had no button at all. It is now beside + Export CSV for every report the page can build a table from, which is all of + them: the plugin reports through their declared columns, the core reports + through the flat list they return, and the asset inventory summary by + flattening its three tables into one with the grouping named in a column. + Presentation-only fields - a status colour, a link id - are left out, since a + column of hex codes is worse than no column. The button is hidden without the + `reports.export` permission the endpoint enforces, on all three pages: offering + somebody a button that can only answer 403 is worse than not offering it. + +- **A long drive name pushed Take and Return out of the card.** On the USB kiosk + the name is a flex item, which defaults to `min-width: auto`, so it kept its + full intrinsic width and shoved the button through the side of the card - + leaving a drive nobody could take. Names now wrap, including the serial-style + ones that carry no spaces at all, and a description is clamped to two lines so + one long name cannot make every card in its row tall. The locker line is never + clipped: it is how somebody finds the thing. + - **Muted text on the USB kiosk and in guides was not muted.** Both named a `--textmuted` variable this app has never defined, so the browser dropped the declaration and the hints, subtitles and locker lines rendered at full diff --git a/frontend/src/utils/reportTable.js b/frontend/src/utils/reportTable.js new file mode 100644 index 00000000..41e207d3 --- /dev/null +++ b/frontend/src/utils/reportTable.js @@ -0,0 +1,69 @@ +// Turning a report payload into the one table shape everything else speaks. +// +// POST /api/reports/email has always taken {columns, rows} from any caller, but +// only two pages ever offered an Email report button - the two that happen to +// be their own routed pages. Every other report was reachable only from the +// reports page, which had no button at all. This builds that table from +// whatever shape the open report returned, so the offer is the same everywhere. + +export function titleCase (text) { + return String(text || '') + .split(/[\s_-]+/) + .map(word => word.charAt(0).toUpperCase() + word.slice(1)) + .join(' ') +} + +// Presentation-only keys. A colour swatch and a row id mean nothing in an +// email, and a column of hex codes is worse than no column at all. +const EMAIL_SKIP = ['color', 'linkid', 'linkurl', 'id'] + +function emailcolumns (keys) { + return keys.filter(key => !EMAIL_SKIP.includes(key)) + .map(key => ({ key, label: titleCase(key) })) +} + +export function emailTableFrom (data) { + if (!data) return null + + // Any report that speaks the {columns, rows} contract - every plugin report, + // and any core one that adopts it. Declared columns win; otherwise take them + // from the first row, the same way the on-screen table does. + if (Array.isArray(data.rows) && data.rows.length) { + const keys = (Array.isArray(data.columns) && data.columns.length) + ? data.columns + : Object.keys(data.rows[0]) + return { columns: emailcolumns(keys), rows: data.rows } + } + + // The core reports, which return a flat list under `data`. + if (Array.isArray(data.data) && data.data.length) { + return { columns: emailcolumns(Object.keys(data.data[0])), rows: data.data } + } + + // Asset inventory is three tables. One email, so they become one table with + // the grouping named in a column - sending a third of it would be worse. + if (data.data && typeof data.data === 'object') { + const rows = [] + for (const [section, entries] of Object.entries(data.data)) { + if (!Array.isArray(entries)) continue + for (const entry of entries) { + rows.push({ + section: titleCase(section), + name: entry.type || entry.status || entry.location || '', + count: entry.count, + }) + } + } + if (!rows.length) return null + return { + columns: [{ key: 'section', label: 'Grouping' }, + { key: 'name', label: 'Name' }, + { key: 'count', label: 'Count' }], + rows, + } + } + + // An empty report has nothing to send. The button hides rather than mailing + // somebody a table with no rows in it. + return null +} diff --git a/frontend/src/utils/reportTable.spec.js b/frontend/src/utils/reportTable.spec.js new file mode 100644 index 00000000..a63167f2 --- /dev/null +++ b/frontend/src/utils/reportTable.spec.js @@ -0,0 +1,84 @@ +// Every shape a report on this app actually returns, because the Email report +// button now has to build a table out of all of them rather than only the two +// that had their own page. + +import { describe, it, expect } from 'vitest' + +import { emailTableFrom, titleCase } from './reportTable' + +describe('emailTableFrom', () => { + it('has nothing to send for a report that has not run', () => { + expect(emailTableFrom(null)).toBe(null) + expect(emailTableFrom({})).toBe(null) + }) + + it('uses the declared columns of a {columns, rows} report', () => { + const table = emailTableFrom({ + columns: ['timestamp', 'action', 'device_id'], + rows: [{ timestamp: 't', action: 'check-out', device_id: 'USB-3' }], + }) + expect(table.columns).toEqual([ + { key: 'timestamp', label: 'Timestamp' }, + { key: 'action', label: 'Action' }, + { key: 'device_id', label: 'Device Id' }, + ]) + expect(table.rows).toHaveLength(1) + }) + + it('falls back to the first row when a report declares no columns', () => { + const table = emailTableFrom({ rows: [{ item: 'a', takes: 2 }] }) + expect(table.columns.map(c => c.key)).toEqual(['item', 'takes']) + }) + + it('builds a table from a core report that returns a flat list', () => { + const table = emailTableFrom({ + report: 'machines_by_type', + total: 231, + data: [{ machinetype: 'Lathe', description: 'd', count: 12 }], + }) + expect(table.columns.map(c => c.key)).toEqual(['machinetype', 'description', 'count']) + expect(table.rows[0].count).toBe(12) + }) + + it('drops the keys that only mean something on screen', () => { + const table = emailTableFrom({ + data: [{ status: 'In Use', color: '#28a745', count: 572 }], + }) + expect(table.columns.map(c => c.key)).toEqual(['status', 'count']) + + const kb = emailTableFrom({ + data: [{ linkid: 4, linkurl: 'http://x', shortdescription: 'How to', clicks: 9 }], + }) + expect(kb.columns.map(c => c.key)).toEqual(['shortdescription', 'clicks']) + }) + + it('flattens the three tables of asset inventory into one, named', () => { + const table = emailTableFrom({ + data: { + bytype: [{ type: 'computer', count: 289 }], + bystatus: [{ status: 'In Use', color: '#0f0', count: 572 }], + bylocation: [{ location: 'Cell A', count: 3 }], + }, + }) + expect(table.columns.map(c => c.key)).toEqual(['section', 'name', 'count']) + expect(table.rows).toEqual([ + { section: 'Bytype', name: 'computer', count: 289 }, + { section: 'Bystatus', name: 'In Use', count: 572 }, + { section: 'Bylocation', name: 'Cell A', count: 3 }, + ]) + }) + + it('has nothing to send for a report that came back empty', () => { + expect(emailTableFrom({ data: [], total: 0 })).toBe(null) + expect(emailTableFrom({ columns: ['a'], rows: [] })).toBe(null) + expect(emailTableFrom({ data: { bytype: [], bystatus: [] } })).toBe(null) + }) +}) + +describe('titleCase', () => { + it('turns a column key into a heading', () => { + expect(titleCase('locker_location')).toBe('Locker Location') + expect(titleCase('machines-by-type')).toBe('Machines By Type') + expect(titleCase('')).toBe('') + }) +}) diff --git a/frontend/src/views/reports/ReportsIndex.vue b/frontend/src/views/reports/ReportsIndex.vue index 89c62c32..0298ad05 100644 --- a/frontend/src/views/reports/ReportsIndex.vue +++ b/frontend/src/views/reports/ReportsIndex.vue @@ -43,6 +43,9 @@

{{ currentReport.name }}

+
@@ -266,9 +269,18 @@ import { ref, computed, onMounted, watch } from 'vue' import { useRouter, useRoute } from 'vue-router' import api, { reportsApi, businessunitsApi, assetsApi, locationsApi, applicationsApi } from '@/api' import { apiError } from '@/utils/apiError' +import EmailReportButton from '@/components/EmailReportButton.vue' +import { emailTableFrom, titleCase } from '@/utils/reportTable' +import { useAuthStore } from '@/stores/auth' const router = useRouter() const route = useRoute() +const authStore = useAuthStore() + +// Emailing a report is the reports.export permission, the same one the endpoint +// enforces. Offering the button to somebody who will only get a 403 is worse +// than not offering it. +const canemail = computed(() => authStore.hasPermission('reports.export')) const reports = ref([]) const search = ref('') @@ -321,6 +333,12 @@ const genericColumns = computed(() => { return data.rows?.length ? Object.keys(data.rows[0]) : [] }) +// The table to email, built from whatever the open report returned. Emailing +// was available on two pages and nowhere else, purely because those two were +// built as their own routed pages - the endpoint has always taken any +// {columns, rows}. Lives in its own module so the shapes can be tested. +const emailtable = computed(() => emailTableFrom(reportData.value)) + const activeFilterFields = computed(() => currentReport.value ? (reportFilterFields[currentReport.value.id] || []) : [] ) @@ -391,12 +409,6 @@ async function loadReports() { } } -function titleCase(text) { - return String(text || '') - .split(/[\s_-]+/) - .map(word => word.charAt(0).toUpperCase() + word.slice(1)) - .join(' ') -} // Filter by name/description/category, then group by category in the fixed order. const groupedReports = computed(() => { diff --git a/plugins/printers/frontend/views/TonerReport.vue b/plugins/printers/frontend/views/TonerReport.vue index 8af88d49..22126aad 100644 --- a/plugins/printers/frontend/views/TonerReport.vue +++ b/plugins/printers/frontend/views/TonerReport.vue @@ -4,7 +4,7 @@

Toner Report

- Back to Reports
@@ -133,9 +133,16 @@ import { ref, computed, onMounted } from 'vue' import { printersApi } from '@/api' import EmailReportButton from '@/components/EmailReportButton.vue' +import { useAuthStore } from '@/stores/auth' import LocationMapTooltip from '@/components/LocationMapTooltip.vue' import { getPrinterHostnameTemplate } from '@/utils/siteSettings' +// Emailing a report is the reports.export permission the endpoint enforces. +// Offering the button to somebody who will only get a 403 is worse than not +// offering it. +const authStore = useAuthStore() +const canemail = computed(() => authStore.hasPermission('reports.export')) + const emailColumns = [ { key: 'printer', label: 'Printer' }, { key: 'hostname', label: 'Hostname' }, diff --git a/plugins/usb/frontend/views/USBKiosk.vue b/plugins/usb/frontend/views/USBKiosk.vue index 626e93b3..f43b1fb6 100644 --- a/plugins/usb/frontend/views/USBKiosk.vue +++ b/plugins/usb/frontend/views/USBKiosk.vue @@ -50,7 +50,7 @@ @click="act(drive, 'checkin')"> {{ drive.device_id }} - {{ drive.device_desc }} + {{ drive.device_desc }} Return @@ -83,7 +83,7 @@ @click="act(drive, 'checkout')"> {{ drive.device_id }} - {{ drive.device_desc }} + {{ drive.device_desc }} Locker {{ drive.locker_location }} Take @@ -112,7 +112,7 @@ @click="act(drive, 'checkin', true)"> {{ drive.device_id }} - {{ drive.holdername || 'somebody else' }} + {{ drive.holdername || 'somebody else' }} Return @@ -436,8 +436,29 @@ function reset () { cursor: pointer; } .drive-row:disabled { opacity: 0.5; } -.drive-name { display: flex; flex-direction: column; gap: 0.15rem; } +/* The name has to YIELD. A flex item defaults to min-width:auto, so a long + label kept its intrinsic width and shoved Take or Return out through the side + of the card - a drive nobody could take. `anywhere` breaks the labels that + carry no spaces at all, which a serial-style name often does not. */ +.drive-name { + display: flex; + flex-direction: column; + gap: 0.15rem; + flex: 1 1 auto; + min-width: 0; + overflow-wrap: anywhere; +} .drive-name small { color: var(--text-light); font-size: 0.85rem; } +/* Two lines of description is enough to tell two drives apart; more just makes + one card tall and every card in its row tall with it. The locker line below + is never clipped - it is how somebody finds the thing. */ +.drive-desc { + display: -webkit-box; + -webkit-line-clamp: 2; + line-clamp: 2; + -webkit-box-orient: vertical; + overflow: hidden; +} .drive-action { flex: 0 0 auto; padding: 0.5rem 1.1rem; diff --git a/plugins/warranty/frontend/views/WarrantyReport.vue b/plugins/warranty/frontend/views/WarrantyReport.vue index 634db510..bfa09a83 100644 --- a/plugins/warranty/frontend/views/WarrantyReport.vue +++ b/plugins/warranty/frontend/views/WarrantyReport.vue @@ -4,7 +4,7 @@

Warranty Report

- Back to Reports
@@ -65,8 +65,15 @@ import { ref, computed, onMounted } from 'vue' import { warrantyApi } from '@/api' import EmailReportButton from '@/components/EmailReportButton.vue' +import { useAuthStore } from '@/stores/auth' import MachineMapChip from '../components/MachineMapChip.vue' +// Emailing a report is the reports.export permission the endpoint enforces. +// Offering the button to somebody who will only get a 403 is worse than not +// offering it. +const authStore = useAuthStore() +const canemail = computed(() => authStore.hasPermission('reports.export')) + const loading = ref(true) const counts = ref({}) const buckets = ref({})