diff --git a/bin/repair-renamed-labels b/bin/repair-renamed-labels new file mode 100755 index 00000000..7eabf4c1 --- /dev/null +++ b/bin/repair-renamed-labels @@ -0,0 +1,112 @@ +#!/usr/bin/env node +/** + * Labels renamed on GitHub before Pulldasher handled the `label` webhook + * left their old names on the PRs and issues it stores. This finds every + * stored label its repo no longer has, reads each carrying PR's or issue's + * labels off GitHub for the name it has now, and with --apply renames the + * stored rows in place (a refresh would drop who added the label and when, + * which the Projects tab sorts by), moving a renamed project's slug-kept + * state too (lib/work.js renameProject). Run it with the instance's config: + * + * docker exec pulldasher bin/repair-renamed-labels [--apply] + * + * Without --apply it only lists. A label with no single new name (deleted, + * or its items now disagree) is listed and left alone. + */ +import config from '../lib/config-loader.js'; +import db from '../lib/db.js'; +import dbManager from '../lib/db-manager.js'; +import git from '../lib/git-manager.js'; +import { projectSettings } from '../lib/projects.js'; +import { loadSettings } from '../lib/settings.js'; +import { renameProject } from '../lib/work.js'; + +const apply = process.argv.includes('--apply'); +// items read off GitHub per stale label +const SAMPLE = 5; +const split = repo => ({ owner: repo.split('/')[0], repo: repo.split('/')[1] }); + +async function repoLabels(repo) { + const names = new Set(); + for (let page = 1; ; page++) { + const { data } = await git.github.issues.listLabelsForRepo({ + ...split(repo), + per_page: 100, + page, + }); + for (const l of data) names.add(l.name.toLowerCase()); + if (data.length < 100) return names; + } +} + +try { + await loadSettings(); + const settings = config.projects ? projectSettings() : null; + const stored = await db.query('SELECT DISTINCT `repo`, `title` FROM `pull_labels`'); + const byRepo = new Map(); + for (const row of stored) byRepo.set(row.repo, [...(byRepo.get(row.repo) ?? []), row.title]); + + const renames = []; + for (const [repo, titles] of byRepo) { + let live; + try { + live = await repoLabels(repo); + } catch (err) { + console.log('%s: skipped, its labels can’t be read (%s)', repo, err.status ?? err.message); + continue; + } + for (const title of titles.filter(t => !live.has(t.toLowerCase()))) { + const nums = ( + await db.query( + 'SELECT `number` FROM `pull_labels` WHERE `repo` = ? AND `title` = ? ORDER BY `number` DESC', + [repo, title] + ) + ).map(r => r.number); + // the new name: on GitHub on every item checked, and stored on none + // of them; a few items say it, and an old label on thousands of + // merged PRs would spend the board's GitHub quota + let now = null; + for (const number of nums.slice(0, SAMPLE)) { + const data = await git.github.issues + .listLabelsOnIssue({ ...split(repo), number }) + .then(res => res.data) + // deleted or moved away: it says nothing about the new name + .catch(err => (err.status === 404 ? null : Promise.reject(err))); + if (!data) continue; + const mine = (await dbManager.getLabels(repo, number)).map(l => l.data.title); + const fresh = data.map(l => l.name).filter(name => !mine.includes(name)); + now = now ? now.filter(name => fresh.includes(name)) : fresh; + } + const to = now?.length === 1 ? now[0] : null; + console.log( + '%s: "%s" on %s -> %s', + repo, + title, + nums + .slice(0, SAMPLE) + .map(n => '#' + n) + .join(' ') + (nums.length > SAMPLE ? ` and ${nums.length - SAMPLE} more` : ''), + to ? `"${to}"` : `no single new name (${now?.join(', ') || 'none'}), left alone` + ); + if (to) renames.push({ repo, from: title, to }); + } + } + + if (apply) { + for (const { repo, from, to } of renames) await dbManager.renameLabel(repo, from, to); + const slug = name => + settings && name.startsWith(settings.prefix) ? name.slice(settings.prefix.length) : null; + const moves = new Map( + renames.filter(r => slug(r.from) && slug(r.to)).map(r => [slug(r.from), slug(r.to)]) + ); + for (const [from, to] of moves) { + const moved = await renameProject(settings, from, to); + console.log('project %s -> %s: %s', from, to, moved ? 'moved' : 'old label still in use'); + } + console.log('Renamed %d stored labels.', renames.length); + } else if (renames.length) { + console.log('Dry run: --apply renames the %d above.', renames.length); + } +} finally { + db.end(); +} diff --git a/controllers/githubHooks.js b/controllers/githubHooks.js index 4e6f19de..61d83ec1 100644 --- a/controllers/githubHooks.js +++ b/controllers/githubHooks.js @@ -25,7 +25,13 @@ function withTimeout(promise, ms) { return Promise.race([promise, late]).finally(() => clearTimeout(timer)); } import { projectSettings } from '../lib/projects.js'; -import { forgetIssue, workIssueTouched, workPullTouched } from '../lib/work.js'; +import { + forgetIssue, + renameProject, + syncWork, + workIssueTouched, + workPullTouched, +} from '../lib/work.js'; const hooksDebug = debug('pulldasher:hooks'); @@ -255,6 +261,8 @@ const HooksController = { completed_at: body.check_run.completed_at, }); dbUpdated = dbManager.updateCommitStatus(status); + } else if (event === 'label') { + dbUpdated = Promise.resolve(handleLabelRename(body)); } if (dbUpdated) { @@ -313,6 +321,36 @@ function handleIssueEvent(body) { }); } +/** + * A repo's label renamed on GitHub (the `label` event, `edited` with + * `changes.name`). GitHub sends no labeled/unlabeled for the PRs and issues + * carrying it, so their stored labels take the new name here; a project + * label's rename moves the project too (renameProject). + */ +async function handleLabelRename(body) { + const from = body.changes?.name?.from; + if (body.action !== 'edited' || !from) return; + const repo = body.repository.full_name; + const to = body.label.name; + hooksDebug('Renamed label: %s -> %s in %s', from, to, repo); + const open = await dbManager.renameLabel(repo, from, to); + for (const number of open) queue.markPullAsDirty(repo, number); + + const settings = projectSettings(); + if (!settings) return; + const slugOf = name => + name.startsWith(settings.prefix) ? name.slice(settings.prefix.length) : null; + const [was, now] = [slugOf(from), slugOf(to)]; + if (was == null && now == null) return; + if (was != null && now != null) await renameProject(settings, was, now); + // the Projects tab reads labels and slugs: open boards fetch again once + // the work is read (app.js listens for the sync); not awaited, like a + // labeled issue's sync + syncWork(settings).catch(err => + console.error('Work sync after a label rename failed: %s', (err && err.message) || err) + ); +} + /** * Handles the response body if it represents a labeled / unlabled issue * (or pull) event and returns a promise that is fulfilled when the DB change diff --git a/lib/db-manager.js b/lib/db-manager.js index fa10d41d..6543f270 100644 --- a/lib/db-manager.js +++ b/lib/db-manager.js @@ -522,6 +522,37 @@ const dbManager = { return DBComment.delete(repo, 'review', comment_id); }, + /** + * A label renamed on GitHub: every PR and issue in `repo` carrying `from` + * carries `to` now. Resolves to the numbers of the open PRs among them, + * the ones the board shows. + */ + renameLabel: async function renameLabel(repo, from, to) { + dbDebug("Calling renameLabel for '%s' -> '%s' in repo %s", from, to, repo); + const open = await db.query( + 'SELECT l.number FROM pull_labels l JOIN pulls p ON p.repo = l.repo AND ' + + "p.number = l.number WHERE l.repo = ? AND l.title = ? AND p.state = 'open'", + [repo, from] + ); + // a row already under the new name (left by a missed webhook) stays, + // and the old one beside it goes. Titles compare without case, so on a + // case-only rename the join would match each row to itself. (Not + // UPDATE IGNORE: it would cut a name longer than the column to fit.) + if (from.toLowerCase() !== to.toLowerCase()) { + await db.query( + 'DELETE o FROM pull_labels o JOIN pull_labels n ON n.repo = o.repo AND ' + + 'n.number = o.number AND n.title = ? WHERE o.repo = ? AND o.title = ?', + [to, repo, from] + ); + } + await db.query('UPDATE pull_labels SET title = ? WHERE repo = ? AND title = ?', [ + to, + repo, + from, + ]); + return open.map(row => row.number); + }, + /** * Returns a promise which resolves when all the labels for pull `number` * have been deleted from the database. diff --git a/lib/work.js b/lib/work.js index 4a4a8840..50ae88c2 100644 --- a/lib/work.js +++ b/lib/work.js @@ -4,6 +4,7 @@ import git from './git-manager.js'; import { issueRepos, loadProjects } from './projects.js'; import { isBot } from './review-model.js'; import { listItems } from './roadmap.js'; +import { saveSetting, savedSetting } from './settings.js'; import { MISC_SLUG, SUGGEST_DAYS, @@ -419,6 +420,45 @@ export async function forgetIssue(ref, now = Math.floor(Date.now() / 1000)) { await db.query('DELETE FROM `issues` WHERE `repo` = ? AND `number` = ?', [ref.repo, ref.number]); } +/** + * A project label was renamed on GitHub (its PRs and issues already carry + * the new name). What's kept by slug moves with it: the issues added by hand + * or by a link, the roadmap item, and the ongoing mark. A project's label + * lives in many repos and each one's rename arrives on its own, so this + * waits until no repo has a PR or issue under the old label: the last + * repo's rename moves the project. Resolves whether it moved. + */ +export async function renameProject(settings, from, to) { + const old = settings.prefix + from; + // titles compare without case: BINARY keeps a case-only rename from + // counting the renamed rows as the old label + const [left] = await db.query( + 'SELECT COUNT(*) AS `n` FROM `pull_labels` WHERE `title` = ? AND BINARY `title` = ?', + [old, old] + ); + if (Number(left.n)) return false; + // an issue the new project has already keeps its own row + await db.query('UPDATE IGNORE `project_issues` SET `project` = ? WHERE `project` = ?', [ + to, + from, + ]); + // a project has one roadmap item: one tracking the new slug already stays + // the project's, and the old one is left for a person to merge + const [taken] = await db.query( + 'SELECT COUNT(*) AS `n` FROM `roadmap_items` WHERE `project` = ? AND `project` <> ? AND `removed_at` IS NULL', + [to, from] + ); + if (!Number(taken.n)) { + await db.query('UPDATE `roadmap_items` SET `project` = ? WHERE `project` = ?', [to, from]); + } + const ongoing = savedSetting('ongoing_projects'); + if (ongoing?.includes(from)) { + const moved = [...new Set(ongoing.map(slug => (slug === from ? to : slug)))].sort(); + await saveSetting('ongoing_projects', moved, null); + } + return true; +} + /** Read the issues added by hand or by a link off GitHub again, and keep * their titles and states current. Resolves their refs. */ async function refreshHandIssues() { diff --git a/test/hooks-label-rename.test.js b/test/hooks-label-rename.test.js new file mode 100644 index 00000000..62bf2ee0 --- /dev/null +++ b/test/hooks-label-rename.test.js @@ -0,0 +1,120 @@ +import { test, afterEach, mock } from 'node:test'; +import assert from 'node:assert/strict'; +import HooksController from '../controllers/githubHooks.js'; +import config from '../lib/config-loader.js'; +import db from '../lib/db.js'; +import queue from '../lib/pull-queue.js'; +import { _resetSettings, loadSettings } from '../lib/settings.js'; +import { renameProject } from '../lib/work.js'; + +afterEach(() => { + mock.restoreAll(); + _resetSettings(); +}); + +// every query, and what each kind answers +function fakeDb(answers = {}) { + const queries = []; + mock.method(db, 'query', async (sql, params) => { + queries.push({ sql, params }); + for (const [pattern, rows] of Object.entries(answers)) { + if (sql.includes(pattern)) return typeof rows === 'function' ? rows(params) : rows; + } + return sql.includes('COUNT(*)') ? [{ n: 0 }] : []; + }); + return queries; +} + +function send(event, body) { + const req = { + query: { secret: 'test-hook-secret' }, + headers: { 'content-type': 'application/json' }, + body, + get: () => event, + }; + return new Promise(resolve => { + const res = { status: () => res, send: resolve }; + HooksController.main(req, res); + }); +} + +const renamed = (from, to) => ({ + action: 'edited', + label: { name: to }, + changes: { name: { from } }, + repository: { full_name: 'o/r' }, +}); + +test('a renamed label moves to the new name on every row in the repo', async () => { + const saved = config.projects; + config.projects = undefined; + const queries = fakeDb({ 'JOIN pulls': [{ number: 7 }] }); + const dirty = []; + mock.method(queue, 'markPullAsDirty', (repo, number) => dirty.push(`${repo}#${number}`)); + + await send('label', renamed('P-strapi-shared-fields', 'P-strapi-cli')); + config.projects = saved; + + // a row already under the new name goes first, so the update can't collide + const writes = queries.filter(q => /^(DELETE|UPDATE)/.test(q.sql)); + assert.deepEqual( + writes.map(q => [q.sql.split(' ')[0], q.params]), + [ + ['DELETE', ['P-strapi-cli', 'o/r', 'P-strapi-shared-fields']], + ['UPDATE', ['P-strapi-cli', 'o/r', 'P-strapi-shared-fields']], + ] + ); + // the open PR the board shows is sent again + assert.deepEqual(dirty, ['o/r#7']); +}); + +test('a case-only rename keeps the renamed rows', async () => { + const saved = config.projects; + config.projects = undefined; + const queries = fakeDb(); + await send('label', renamed('P-Docs', 'P-docs')); + config.projects = saved; + assert.ok(queries.some(q => q.sql.startsWith('UPDATE pull_labels'))); + assert.ok(!queries.some(q => q.sql.startsWith('DELETE'))); +}); + +test('a label edit with no new name, a new label, or a deleted one changes nothing', async () => { + const queries = fakeDb(); + await send('label', { ...renamed('a', 'a'), changes: { color: { from: 'fff' } } }); + await send('label', { ...renamed('a', 'a'), action: 'created', changes: undefined }); + await send('label', { ...renamed('a', 'a'), action: 'deleted', changes: undefined }); + assert.equal(queries.length, 0); +}); + +const settings = { prefix: 'P-' }; + +test('a project moves once no repo has a PR or issue under its old label', async () => { + const queries = fakeDb({ + '`project_settings`': [{ name: 'ongoing_projects', value: '["audit","docs"]' }], + }); + await loadSettings(); + assert.equal(await renameProject(settings, 'audit', 'remediation'), true); + const sql = queries.map(q => [q.sql.split(' ').slice(0, 3).join(' '), q.params]); + assert.deepEqual( + sql.filter(([s]) => s.startsWith('UPDATE')), + [ + ['UPDATE IGNORE `project_issues`', ['remediation', 'audit']], + ['UPDATE `roadmap_items` SET', ['remediation', 'audit']], + ] + ); + const ongoing = queries.find(q => q.sql.startsWith('REPLACE INTO `project_settings`')); + assert.equal(ongoing.params[0].value, '["docs","remediation"]'); +}); + +test('a project another repo still labels the old way stays put', async () => { + const queries = fakeDb({ '`pull_labels`': [{ n: 2 }] }); + assert.equal(await renameProject(settings, 'audit', 'remediation'), false); + assert.equal(queries.length, 1); +}); + +test('a roadmap item already tracking the new slug keeps it', async () => { + const queries = fakeDb({ '`roadmap_items` WHERE': [{ n: 1 }] }); + await renameProject(settings, 'audit', 'remediation'); + assert.ok(!queries.some(q => q.sql.startsWith('UPDATE `roadmap_items`'))); + assert.ok(queries.some(q => q.sql.startsWith('UPDATE IGNORE `project_issues`'))); +});