umbragit/UmbraTrans
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix the add-node page: nothing happened because three bugs, all silent

Browse Source 
Reported as "I entered info, clicked create, and nothing happened". Three separate
faults, each of which fails quietly, so the page looked like it simply did not work.

**1. The toast element had the wrong id.** api.js's showToast() looks for
`id="toast"` and returns silently when it is absent. The page had
`id="toast-region"`. Every message - success and failure alike - was appended to
nothing. This is the direct cause of "nothing happened": the request may well have
succeeded and the answer was thrown away. The working console page uses id="toast";
mine should have matched it.

**2. Seven `el()` calls passed a className where api.js wants a props object.**
`el(tag, props, children)` assigns `props.class`. `el('td', 'table-note')` therefore
set an attribute literally named "table-note" and no class at all, so the node list
would have rendered unstyled. Caught by reading the helper instead of assuming its
signature from the name.

**3. A platform administrator could not use the page at all.** The compute routes
refuse to infer an organization - they raise `organization_required` and ask for
`?organization_id=...`. The first version read an `organization_id` out of
/api/uadmin/summary, which has no such field, so it was always empty and every create
was a 422.

The page now loads organizations from /api/uadmin/organizations and presents a picker,
sending the chosen id as a **query parameter**. It went in the JSON body at first,
where the route declares it as a Query - a body field of the same name is silently
ignored, which looks exactly like it worked. An organization administrator has exactly
one and sees it preselected rather than being asked to choose.

Also added `defer` to both script tags, as the working console page has, so api.js is
defined before nodes.js boots.

Verified the corrected request path end to end: the same payload the browser now sends
produces a code, and that code pairs a node on 238 through the `curl | sh` flow.

525 passed, 27 skipped.
This commit is contained in:
umbra audio
2026-10-03 11:44:35 -07:00
parent c3a3e506ec
commit a75959c2e9
2 changed files with 92 additions and 31 deletions
Download Patch File Download Diff File Expand all files Collapse all files

100
web/assets/nodes.js
View File

@@ -32,11 +32,11 @@
/* ------------------------------------------------------------------ list */
function statusBadge(status, schedulable, reason) {
var badge = el('span', 'status-badge status-badge--' + (status || 'unknown'));
var badge = el('span', { class: 'status-badge status-badge--' + (status || 'unknown') });
setText(badge, status || 'unknown');
badge.title = reason || '';
if (status === 'online' && schedulable === false) {
var note = el('span', 'status-badge__note');
var note = el('span', { class: 'status-badge__note' });
setText(note, 'not schedulable');
badge.appendChild(note);
}
@@ -100,7 +100,7 @@
setText(name, node.name);
nameCell.appendChild(name);
if (node.stored_status && node.stored_status !== node.status) {
var stale = el('small', 'table-note table-note--warn');
var stale = el('small', { class: 'table-note table-note--warn' });
setText(stale, 'stored as ' + node.stored_status);
nameCell.appendChild(stale);
}
@@ -122,7 +122,7 @@
setText(seenCell, lastSeenText(node));
row.appendChild(seenCell);
var reasonCell = el('td', 'table-note');
var reasonCell = el('td', { class: 'table-note' });
setText(reasonCell, node.status_reason || '');
row.appendChild(reasonCell);
@@ -154,7 +154,7 @@
}
function actionButton(label, handler) {
var button = el('button', 'button button--quiet button--small');
var button = el('button', { class: 'button button--quiet button--small' });
button.type = 'button';
setText(button, label);
button.addEventListener('click', handler);
@@ -169,13 +169,13 @@
clear(container);
if (!nodes.length) {
var empty = el('p', 'empty-state');
var empty = el('p', { class: 'empty-state' });
setText(empty, 'No nodes yet. Add one, then run the command it gives you on that machine.');
container.appendChild(empty);
return;
}
var table = el('table', 'admin-table');
var table = el('table', { class: 'admin-table' });
var head = el('thead');
var headRow = el('tr');
['Node', 'Status', 'Where', 'Devices', 'Last seen', 'Why', ''].forEach(function (title) {
@@ -195,9 +195,12 @@
}
function loadNodes() {
var path = ORGANIZATION_ID
? '/api/compute/nodes?organization_id=' + encodeURIComponent(ORGANIZATION_ID)
: '/api/compute/nodes';
// Always names the organization. The compute routes refuse to infer one for a
// platform administrator, so a bare /api/compute/nodes is a 422 waiting to happen.
var path = '/api/compute/nodes';
if (ORGANIZATION_ID) {
path += '?organization_id=' + encodeURIComponent(ORGANIZATION_ID);
}
return api.request(path).then(
function (nodes) {
renderNodes(nodes);
@@ -284,13 +287,33 @@
return;
}
var organizationField = query('#node-organization');
var organizationId = String(
organizationField && organizationField.value ? organizationField.value : ''
).trim();
if (!organizationId) {
say('Choose the organization this node belongs to.', 'error');
if (organizationField) {
organizationField.focus();
}
return;
}
var localityField = query('#node-locality');
var domainField = query('#node-trust-domain');
var controllerField = query('#node-controller');
var rootField = query('#node-as-root');
api.disable(submit, true);
api.request('/api/compute/pairing-script', {
// organization_id goes in the query string, not the body. The route declares it as
// a Query parameter, and a JSON field of the same name is silently ignored - which
// is how the first version looked correct and then failed with 422
// `organization_required` because the server never saw one.
var path =
'/api/compute/pairing-script?organization_id=' + encodeURIComponent(organizationId);
api.request(path, {
method: 'POST',
body: {
node_name: name,
@@ -414,24 +437,45 @@
});
}
// The organization comes from the session rather than a picker, because a node is
// owned by exactly one tenant and picking the wrong one would put a machine in
// somebody else's inventory.
api
.request('/api/uadmin/summary')
.then(
function (summary) {
if (summary && summary.organization_id) {
ORGANIZATION_ID = summary.organization_id;
}
return loadNodes();
},
function () {
// A platform administrator may have no single organization. The list route
// falls back to the caller's own scope, so carry on.
return loadNodes();
loadOrganizations();
}
/*
* The organization a node belongs to.
*
* A platform administrator has no organization of their own, and the compute routes
* refuse to guess: they raise `organization_required` and ask for
* `?organization_id=…`. The first version read an organization_id out of
* /api/uadmin/summary, which does not have that field, so it was always empty and
* every create failed with a 422. So the organization is chosen here, explicitly.
*
* This is also the correct shape for an organization administrator: they have exactly
* one, and the picker shows it without making them choose.
*/
function loadOrganizations() {
return api.request('/api/uadmin/organizations').then(
function (data) {
var items = (data && data.items) || [];
var select = query('#node-organization');
if (!select) {
return;
}
);
clear(select);
var placeholder = el('option', { value: '', text: 'Choose an organization…' });
select.appendChild(placeholder);
items.forEach(function (org) {
select.appendChild(el('option', { value: org.id, text: org.name }));
});
if (items.length === 1) {
select.value = items[0].id;
ORGANIZATION_ID = items[0].id;
}
return loadNodes();
},
function (error) {
say(api.describeError(error), 'error');
}
);
}
if (document.readyState === 'loading') {

23
web/uadmin/nodes.html
View File

@@ -60,6 +60,20 @@
</p>
<form id="add-node-form" novalidate>
<div class="field">
<label for="node-organization">Organization</label>
<select id="node-organization" name="organization_id" required
aria-describedby="node-organization-hint">
<option value="">Choose an organization…</option>
</select>
<small id="node-organization-hint" class="field-hint">
A node belongs to exactly one organization, so the node list and every
job it takes are scoped by it. A platform administrator has no
organization of their own and must choose one — that is why this is a
field rather than something inferred.
</small>
</div>
<div class="field">
<label for="node-name">Node name</label>
<input id="node-name" name="node_name" type="text" autocomplete="off"
@@ -173,9 +187,12 @@
</section>
</main>
<div class="toast-region" id="toast-region" aria-live="polite"></div>
<!-- showToast() in api.js looks for id="toast". With a different id every message -
success and failure alike - is appended to nothing and returns silently, which
looks exactly like the button not working. -->
<div id="toast" class="toast" role="status" aria-live="polite"></div>
<script src="../assets/api.js"></script>
<script src="../assets/nodes.js"></script>
<script src="../assets/api.js" defer></script>
<script src="../assets/nodes.js" defer></script>
</body>
</html>