mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-10-13 10:36:35 +00:00
fix(web): report a no-op plugin update as already up to date
update_plugin() returns True both for a real update and for "nothing to do" (a ZIP-installed monorepo plugin already at the registry version, a bundled plugin). With no git commit to compare, POST /plugins/update called every such success "updated successfully", so Check & Update All counted most official plugins as updated on every run. The route now reads what changed off the plugin itself (commit, else manifest version, else last_updated) and returns data.update_status (updated / up_to_date / local_only). The update-all toast is summarised by PluginInstallManager.summarizeUpdateResults from that status, falling back to the message for older servers. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -146,6 +146,33 @@ const noSleep = { sleep: async () => {} };
|
|||||||
ok('...and is reported as a failure', results.find(x => x.pluginId === 'static-image').success === false);
|
ok('...and is reported as a failure', results.find(x => x.pluginId === 'static-image').success === false);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
console.log('\nsummary: a no-op update is not counted as updated');
|
||||||
|
{
|
||||||
|
const answer = (update_status, message) => ({ success: true, result: { status: 'success', message, data: { update_status } } });
|
||||||
|
const results = [
|
||||||
|
answer('updated', 'Plugin a updated to version 2.8.0'),
|
||||||
|
// ZIP-installed monorepo plugin already at the registry version: the
|
||||||
|
// route used to call this "updated successfully".
|
||||||
|
answer('up_to_date', 'Plugin stock-news already up to date (version 2.8.0)'),
|
||||||
|
answer('up_to_date', 'Plugin clock already up to date (commit abcdef1)'),
|
||||||
|
answer('local_only', 'Plugin mine is managed locally and does not receive registry updates'),
|
||||||
|
{ success: false, error: { error_code: 'PLUGIN_UPDATE_FAILED' } },
|
||||||
|
];
|
||||||
|
const s = Manager.summarizeUpdateResults(results);
|
||||||
|
ok('counts come from update_status',
|
||||||
|
s.updated === 1 && s.upToDate === 2 && s.localOnly === 1 && s.failed === 1, s);
|
||||||
|
ok('toast text names each outcome',
|
||||||
|
s.text === '1 updated, 2 already up to date, 1 managed locally, 1 failed', s.text);
|
||||||
|
ok('a failure alongside an update is a warning', s.type === 'warning', s.type);
|
||||||
|
ok('an older server that only says so in the message is still up to date',
|
||||||
|
Manager.updateOutcome({ success: true, result: { message: 'Plugin x already up to date (commit 1234567)' } }) === 'up_to_date');
|
||||||
|
ok('a success without a status or telltale message counts as updated',
|
||||||
|
Manager.updateOutcome({ success: true, result: { message: 'Plugin x updated successfully' } }) === 'updated');
|
||||||
|
const allNoop = Manager.summarizeUpdateResults([answer('up_to_date', ''), answer('up_to_date', '')]);
|
||||||
|
ok('nothing to do is a success toast with no "updated"',
|
||||||
|
allNoop.type === 'success' && allNoop.text === '2 already up to date', allNoop);
|
||||||
|
}
|
||||||
|
|
||||||
console.log(`\n${pass} passed, ${fail} failed`);
|
console.log(`\n${pass} passed, ${fail} failed`);
|
||||||
process.exit(fail ? 1 : 0);
|
process.exit(fail ? 1 : 0);
|
||||||
})().catch(e => { console.error(e); process.exit(1); });
|
})().catch(e => { console.error(e); process.exit(1); });
|
||||||
|
|||||||
@@ -80,6 +80,67 @@ class TestUpdateRouteRejectsStarlarkIds:
|
|||||||
assert [c.args[0] for c in store.update_plugin.call_args_list] == ['stock-news', 'starlark-apps']
|
assert [c.args[0] for c in store.update_plugin.call_args_list] == ['stock-news', 'starlark-apps']
|
||||||
|
|
||||||
|
|
||||||
|
class TestUpdateRouteReportsNoOps:
|
||||||
|
"""update_plugin() answers True when there was nothing to do.
|
||||||
|
|
||||||
|
A ZIP-installed monorepo plugin (no .git) already at the registry version
|
||||||
|
is the common case for official plugins. The route used to call that
|
||||||
|
"updated successfully", so Check & Update All counted it as updated.
|
||||||
|
"""
|
||||||
|
|
||||||
|
def _install(self, tmp_path, pid, version, last_updated='2026-09-01'):
|
||||||
|
(tmp_path / pid).mkdir()
|
||||||
|
(tmp_path / pid / 'manifest.json').write_text(
|
||||||
|
'{"id": "%s", "version": "%s", "last_updated": "%s"}' % (pid, version, last_updated),
|
||||||
|
encoding='utf-8')
|
||||||
|
|
||||||
|
def test_a_zip_plugin_already_at_the_registry_version_is_up_to_date(self, client, store, tmp_path):
|
||||||
|
self._install(tmp_path, 'stock-news', '2.8.0')
|
||||||
|
body = client.post('/api/v3/plugins/update', json={'plugin_id': 'stock-news'}).get_json()
|
||||||
|
assert body['status'] == 'success'
|
||||||
|
assert body['data']['update_status'] == 'up_to_date'
|
||||||
|
assert 'already up to date' in body['message'], body
|
||||||
|
assert 'updated successfully' not in body['message']
|
||||||
|
|
||||||
|
def test_a_zip_plugin_reinstalled_at_a_new_version_is_updated(self, client, store, tmp_path):
|
||||||
|
self._install(tmp_path, 'stock-news', '2.6.2')
|
||||||
|
|
||||||
|
def reinstall(pid):
|
||||||
|
(tmp_path / pid / 'manifest.json').write_text(
|
||||||
|
'{"id": "%s", "version": "2.8.0", "last_updated": "2026-09-01"}' % pid,
|
||||||
|
encoding='utf-8')
|
||||||
|
return True
|
||||||
|
store.update_plugin.side_effect = reinstall
|
||||||
|
|
||||||
|
body = client.post('/api/v3/plugins/update', json={'plugin_id': 'stock-news'}).get_json()
|
||||||
|
assert body['data']['update_status'] == 'updated'
|
||||||
|
assert '2.8.0' in body['message'], body
|
||||||
|
|
||||||
|
def test_a_git_plugin_whose_commit_is_unchanged_is_up_to_date(self, client, store, tmp_path):
|
||||||
|
self._install(tmp_path, 'clock', '1.0.0')
|
||||||
|
store._get_local_git_info.return_value = {'sha': 'abcdef1234567', 'branch': 'main'}
|
||||||
|
body = client.post('/api/v3/plugins/update', json={'plugin_id': 'clock'}).get_json()
|
||||||
|
assert body['data']['update_status'] == 'up_to_date'
|
||||||
|
|
||||||
|
def test_a_git_plugin_that_moved_is_updated(self, client, store, tmp_path):
|
||||||
|
self._install(tmp_path, 'clock', '1.0.0')
|
||||||
|
store._get_local_git_info.side_effect = [
|
||||||
|
{'sha': 'aaaaaaa000', 'branch': 'main'}, # before
|
||||||
|
{'sha': 'aaaaaaa000', 'branch': 'main'}, # is-git check
|
||||||
|
{'sha': 'bbbbbbb111', 'branch': 'main'}, # after
|
||||||
|
]
|
||||||
|
body = client.post('/api/v3/plugins/update', json={'plugin_id': 'clock'}).get_json()
|
||||||
|
assert body['data']['update_status'] == 'updated', body
|
||||||
|
|
||||||
|
def test_a_local_only_plugin_says_so(self, client, store, tmp_path):
|
||||||
|
(tmp_path / 'mine').mkdir()
|
||||||
|
(tmp_path / 'mine' / 'manifest.json').write_text(
|
||||||
|
'{"id": "mine", "version": "0.1.0", "local_only": true}', encoding='utf-8')
|
||||||
|
body = client.post('/api/v3/plugins/update', json={'plugin_id': 'mine'}).get_json()
|
||||||
|
assert body['data']['update_status'] == 'local_only'
|
||||||
|
store.update_plugin.assert_not_called()
|
||||||
|
|
||||||
|
|
||||||
class TestInstalledListContract:
|
class TestInstalledListContract:
|
||||||
"""What update-all filters on is what /plugins/installed publishes."""
|
"""What update-all filters on is what /plugins/installed publishes."""
|
||||||
|
|
||||||
|
|||||||
@@ -988,6 +988,7 @@ def update_plugin():
|
|||||||
)
|
)
|
||||||
|
|
||||||
current_last_updated = None
|
current_last_updated = None
|
||||||
|
current_version = None
|
||||||
current_commit = None
|
current_commit = None
|
||||||
current_branch = None
|
current_branch = None
|
||||||
|
|
||||||
@@ -997,6 +998,7 @@ def update_plugin():
|
|||||||
with open(manifest_path, 'r', encoding='utf-8') as f:
|
with open(manifest_path, 'r', encoding='utf-8') as f:
|
||||||
manifest = json.load(f)
|
manifest = json.load(f)
|
||||||
current_last_updated = manifest.get('last_updated')
|
current_last_updated = manifest.get('last_updated')
|
||||||
|
current_version = manifest.get('version')
|
||||||
if manifest.get('local_only'):
|
if manifest.get('local_only'):
|
||||||
logger.debug("Skipping update for local-only plugin: %s", plugin_id)
|
logger.debug("Skipping update for local-only plugin: %s", plugin_id)
|
||||||
if api_v3.operation_history:
|
if api_v3.operation_history:
|
||||||
@@ -1006,7 +1008,9 @@ def update_plugin():
|
|||||||
status="skipped",
|
status="skipped",
|
||||||
details={"reason": "local_only"}
|
details={"reason": "local_only"}
|
||||||
)
|
)
|
||||||
return success_response(message=f'Plugin {plugin_id} is managed locally and does not receive registry updates')
|
return success_response(
|
||||||
|
data={'update_status': 'local_only'},
|
||||||
|
message=f'Plugin {plugin_id} is managed locally and does not receive registry updates')
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
logger.debug("Could not read local manifest for plugin: %s", e)
|
logger.debug("Could not read local manifest for plugin: %s", e)
|
||||||
|
|
||||||
@@ -1036,12 +1040,14 @@ def update_plugin():
|
|||||||
|
|
||||||
if success:
|
if success:
|
||||||
updated_last_updated = current_last_updated
|
updated_last_updated = current_last_updated
|
||||||
|
updated_version = current_version
|
||||||
try:
|
try:
|
||||||
if manifest_path.exists():
|
if manifest_path.exists():
|
||||||
import json
|
import json
|
||||||
with open(manifest_path, 'r', encoding='utf-8') as f:
|
with open(manifest_path, 'r', encoding='utf-8') as f:
|
||||||
manifest = json.load(f)
|
manifest = json.load(f)
|
||||||
updated_last_updated = manifest.get('last_updated', current_last_updated)
|
updated_last_updated = manifest.get('last_updated', current_last_updated)
|
||||||
|
updated_version = manifest.get('version', current_version)
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
logger.debug("Could not read updated manifest after update: %s", e)
|
logger.debug("Could not read updated manifest after update: %s", e)
|
||||||
|
|
||||||
@@ -1053,15 +1059,28 @@ def update_plugin():
|
|||||||
updated_commit = git_info_after.get('sha')
|
updated_commit = git_info_after.get('sha')
|
||||||
updated_branch = git_info_after.get('branch') or updated_branch
|
updated_branch = git_info_after.get('branch') or updated_branch
|
||||||
|
|
||||||
|
# update_plugin() answers True for "nothing to do" as well as for
|
||||||
|
# a real update (a ZIP-installed monorepo plugin already at the
|
||||||
|
# registry version, a bundled plugin), so what changed is read off
|
||||||
|
# the plugin itself: its git commit, else its manifest.
|
||||||
|
update_status = 'updated'
|
||||||
message = f'Plugin {plugin_id} updated successfully'
|
message = f'Plugin {plugin_id} updated successfully'
|
||||||
if current_commit and updated_commit and current_commit == updated_commit:
|
if current_commit and updated_commit and current_commit == updated_commit:
|
||||||
|
update_status = 'up_to_date'
|
||||||
message = f'Plugin {plugin_id} already up to date (commit {updated_commit[:7]})'
|
message = f'Plugin {plugin_id} already up to date (commit {updated_commit[:7]})'
|
||||||
elif updated_commit:
|
elif updated_commit:
|
||||||
message = f'Plugin {plugin_id} updated to commit {updated_commit[:7]}'
|
message = f'Plugin {plugin_id} updated to commit {updated_commit[:7]}'
|
||||||
if updated_branch:
|
if updated_branch:
|
||||||
message += f' on branch {updated_branch}'
|
message += f' on branch {updated_branch}'
|
||||||
|
elif updated_version and updated_version != current_version:
|
||||||
|
message = f'Plugin {plugin_id} updated to version {updated_version}'
|
||||||
elif updated_last_updated and updated_last_updated != current_last_updated:
|
elif updated_last_updated and updated_last_updated != current_last_updated:
|
||||||
message = f'Plugin {plugin_id} refreshed (Last Updated {updated_last_updated})'
|
message = f'Plugin {plugin_id} refreshed (Last Updated {updated_last_updated})'
|
||||||
|
elif not current_commit:
|
||||||
|
update_status = 'up_to_date'
|
||||||
|
message = f'Plugin {plugin_id} already up to date'
|
||||||
|
if updated_version:
|
||||||
|
message += f' (version {updated_version})'
|
||||||
|
|
||||||
remote_commit_short = remote_commit[:7] if remote_commit else None
|
remote_commit_short = remote_commit[:7] if remote_commit else None
|
||||||
if remote_commit_short and updated_commit and remote_commit_short != updated_commit[:7]:
|
if remote_commit_short and updated_commit and remote_commit_short != updated_commit[:7]:
|
||||||
@@ -1093,14 +1112,16 @@ def update_plugin():
|
|||||||
"version": version,
|
"version": version,
|
||||||
"previous_commit": current_commit[:7] if current_commit else None,
|
"previous_commit": current_commit[:7] if current_commit else None,
|
||||||
"commit": updated_commit[:7] if updated_commit else None,
|
"commit": updated_commit[:7] if updated_commit else None,
|
||||||
"branch": updated_branch
|
"branch": updated_branch,
|
||||||
|
"update_status": update_status
|
||||||
}
|
}
|
||||||
)
|
)
|
||||||
|
|
||||||
return success_response(
|
return success_response(
|
||||||
data={
|
data={
|
||||||
'last_updated': updated_last_updated,
|
'last_updated': updated_last_updated,
|
||||||
'commit': updated_commit
|
'commit': updated_commit,
|
||||||
|
'update_status': update_status
|
||||||
},
|
},
|
||||||
message=message
|
message=message
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -168,6 +168,58 @@ const PluginInstallManager = {
|
|||||||
}
|
}
|
||||||
|
|
||||||
return results;
|
return results;
|
||||||
|
},
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Classify one POST /plugins/update answer.
|
||||||
|
*
|
||||||
|
* The route reports what actually happened in `data.update_status`
|
||||||
|
* (`updated`, `up_to_date`, `local_only`). A plugin the updater had
|
||||||
|
* nothing to do for -- e.g. a ZIP-installed monorepo plugin already at the
|
||||||
|
* registry version -- is still a success response, so it must not be
|
||||||
|
* counted as updated. Older servers only say so in the message.
|
||||||
|
*
|
||||||
|
* @param {Object} entry - One element of updateAll()'s results
|
||||||
|
* @returns {string} 'failed' | 'updated' | 'up_to_date' | 'local_only'
|
||||||
|
*/
|
||||||
|
updateOutcome(entry) {
|
||||||
|
if (!entry || !entry.success) return 'failed';
|
||||||
|
const result = entry.result || {};
|
||||||
|
const status = result.data && result.data.update_status;
|
||||||
|
if (status === 'up_to_date' || status === 'local_only' || status === 'updated') {
|
||||||
|
return status;
|
||||||
|
}
|
||||||
|
const message = typeof result.message === 'string' ? result.message : '';
|
||||||
|
if (message.includes('already up to date')) return 'up_to_date';
|
||||||
|
if (message.includes('managed locally')) return 'local_only';
|
||||||
|
return 'updated';
|
||||||
|
},
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Summarise updateAll()'s results for the Check & Update All toast.
|
||||||
|
*
|
||||||
|
* @param {Array} results - updateAll()'s results
|
||||||
|
* @returns {{updated: number, upToDate: number, localOnly: number, failed: number, text: string, type: string}}
|
||||||
|
*/
|
||||||
|
summarizeUpdateResults(results) {
|
||||||
|
const counts = { updated: 0, up_to_date: 0, local_only: 0, failed: 0 };
|
||||||
|
for (const entry of (Array.isArray(results) ? results : [])) {
|
||||||
|
counts[this.updateOutcome(entry)]++;
|
||||||
|
}
|
||||||
|
const parts = [];
|
||||||
|
if (counts.updated > 0) parts.push(`${counts.updated} updated`);
|
||||||
|
if (counts.up_to_date > 0) parts.push(`${counts.up_to_date} already up to date`);
|
||||||
|
if (counts.local_only > 0) parts.push(`${counts.local_only} managed locally`);
|
||||||
|
if (counts.failed > 0) parts.push(`${counts.failed} failed`);
|
||||||
|
const type = counts.failed > 0 ? (counts.updated > 0 ? 'warning' : 'error') : 'success';
|
||||||
|
return {
|
||||||
|
updated: counts.updated,
|
||||||
|
upToDate: counts.up_to_date,
|
||||||
|
localOnly: counts.local_only,
|
||||||
|
failed: counts.failed,
|
||||||
|
text: parts.join(', '),
|
||||||
|
type
|
||||||
|
};
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
|
|||||||
@@ -1999,22 +1999,21 @@ function runUpdateAllPlugins() {
|
|||||||
showNotification('No plugins to update.', 'info');
|
showNotification('No plugins to update.', 'info');
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
let updated = 0, upToDate = 0, failed = 0;
|
// Counted by install_manager.js from each answer's update_status:
|
||||||
for (const r of results) {
|
// a no-op update is "already up to date", not "updated". A cached
|
||||||
if (!r.success) {
|
// install_manager.js from before that helper gets a plain count.
|
||||||
failed++;
|
const manager = window.PluginInstallManager;
|
||||||
} else if (r.result && r.result.message && r.result.message.includes('already up to date')) {
|
const summary = (manager && typeof manager.summarizeUpdateResults === 'function')
|
||||||
upToDate++;
|
? manager.summarizeUpdateResults(results)
|
||||||
} else {
|
: (() => {
|
||||||
updated++;
|
const failed = results.filter(r => !r.success).length;
|
||||||
}
|
const checked = results.length - failed;
|
||||||
}
|
return {
|
||||||
const parts = [];
|
text: `${checked} checked` + (failed ? `, ${failed} failed` : ''),
|
||||||
if (updated > 0) parts.push(`${updated} updated`);
|
type: failed ? (checked ? 'warning' : 'error') : 'success'
|
||||||
if (upToDate > 0) parts.push(`${upToDate} already up to date`);
|
};
|
||||||
if (failed > 0) parts.push(`${failed} failed`);
|
})();
|
||||||
const type = failed > 0 ? (updated > 0 ? 'warning' : 'error') : 'success';
|
showNotification(summary.text, summary.type);
|
||||||
showNotification(parts.join(', '), type);
|
|
||||||
})
|
})
|
||||||
.catch(error => {
|
.catch(error => {
|
||||||
console.error('Error updating all plugins:', error);
|
console.error('Error updating all plugins:', error);
|
||||||
|
|||||||
Reference in New Issue
Block a user