From c3d4225fd0cd828e2706cac2c65431d2f0810739 Mon Sep 17 00:00:00 2001 From: fabiovincenzi Date: Sun, 8 Jun 2025 17:12:59 +0200 Subject: [PATCH 01/21] feat: add new chain for tag push --- src/proxy/chain.ts | 49 ++++++++++++++++++++++++++++++++++------------ 1 file changed, 37 insertions(+), 12 deletions(-) diff --git a/src/proxy/chain.ts b/src/proxy/chain.ts index 8bc5e3120..1e690277d 100644 --- a/src/proxy/chain.ts +++ b/src/proxy/chain.ts @@ -3,8 +3,7 @@ import { Action } from './actions'; import * as proc from './processors'; import { attemptAutoApproval, attemptAutoRejection } from './actions/autoActions'; -const pushActionChain: ((req: any, action: Action) => Promise)[] = [ - proc.push.parsePush, +const branchPushChain: ((req: any, action: Action) => Promise)[] = [ proc.push.checkRepoInAuthorisedList, proc.push.checkCommitMessages, proc.push.checkAuthorEmails, @@ -21,6 +20,17 @@ const pushActionChain: ((req: any, action: Action) => Promise)[] = [ proc.push.blockForAuth, ]; +const tagPushChain: ((req: any, action: Action) => Promise)[] = [ + proc.push.checkRepoInAuthorisedList, + proc.push.checkUserPushPermission, + proc.push.checkIfWaitingAuth, + proc.push.pullRemote, + proc.push.writePack, + proc.push.preReceive, + // TODO: implement tag message validation? + proc.push.blockForAuth, +]; + const pullActionChain: ((req: any, action: Action) => Promise)[] = [ proc.push.checkRepoInAuthorisedList, ]; @@ -30,16 +40,19 @@ let pluginsInserted = false; export const executeChain = async (req: any, res: any): Promise => { let action: Action = {} as Action; try { + // 1) Initialize basic action fields action = await proc.pre.parseAction(req); + // 2) Parse the push payload first to detect tags/branches + if (action.type === 'push') { + action = await proc.push.parsePush(req, action); + } + // 3) Select the correct chain now that action.tag is set const actionFns = await getChain(action); + // 4) Execute each step in the selected chain for (const fn of actionFns) { action = await fn(req, action); - if (!action.continue()) { - return action; - } - - if (action.allowPush) { + if (!action.continue() || action.allowPush) { return action; } } @@ -70,6 +83,7 @@ export const getChain = async ( ); pluginsInserted = true; } + if (!pluginsInserted) { console.log( `Inserting loaded plugins (${chainPluginLoader.pushPlugins.length} push, ${chainPluginLoader.pullPlugins.length} pull) into proxy chains`, @@ -77,7 +91,8 @@ export const getChain = async ( for (const pluginObj of chainPluginLoader.pushPlugins) { console.log(`Inserting push plugin ${pluginObj.constructor.name} into chain`); // insert custom functions after parsePush but before other actions - pushActionChain.splice(1, 0, pluginObj.exec); + branchPushChain.splice(1, 0, pluginObj.exec); + tagPushChain.splice(1, 0, pluginObj.exec); } for (const pluginObj of chainPluginLoader.pullPlugins) { console.log(`Inserting pull plugin ${pluginObj.constructor.name} into chain`); @@ -87,8 +102,15 @@ export const getChain = async ( // This is set to true so that we don't re-insert the plugins into the chain pluginsInserted = true; } - if (action.type === 'pull') return pullActionChain; - if (action.type === 'push') return pushActionChain; + if (action.type === 'pull') { + return pullActionChain; + } + if (action.type === 'push' && action.tag) { + return tagPushChain; + } + if (action.type === 'push') { + return branchPushChain; + } return []; }; @@ -102,8 +124,11 @@ export default { get pluginsInserted() { return pluginsInserted; }, - get pushActionChain() { - return pushActionChain; + get branchPushChain() { + return branchPushChain; + }, + get tagPushChain() { + return tagPushChain; }, get pullActionChain() { return pullActionChain; From 95e86ee8500786f53331c8643878992e177f1865 Mon Sep 17 00:00:00 2001 From: fabiovincenzi Date: Sun, 8 Jun 2025 17:20:00 +0200 Subject: [PATCH 02/21] feat: add TagData to Action --- src/proxy/actions/Action.ts | 23 +++++++++++++------ .../push-action/checkAuthorEmails.ts | 8 ++++--- 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/src/proxy/actions/Action.ts b/src/proxy/actions/Action.ts index 78dbc2ef0..92913df4e 100644 --- a/src/proxy/actions/Action.ts +++ b/src/proxy/actions/Action.ts @@ -1,10 +1,10 @@ -import { getProxyUrl } from "../../config"; -import { Step } from "./Step"; +import { getProxyUrl } from '../../config'; +import { Step } from './Step'; /** * Represents a commit. */ -export interface Commit { +export interface CommitData { message: string; committer: string; tree: string; @@ -15,6 +15,13 @@ export interface Commit { commitTimestamp?: string; } +export interface TagData { + object: string; + type: string; + tagName: string; + tagger: string; + message: string; +} /** * Class representing a Push. */ @@ -38,7 +45,7 @@ class Action { rejected: boolean = false; autoApproved: boolean = false; autoRejected: boolean = false; - commitData?: Commit[] = []; + commitData?: CommitData[] = []; commitFrom?: string; commitTo?: string; branch?: string; @@ -48,6 +55,8 @@ class Action { attestation?: string; lastStep?: Step; proxyGitPath?: string; + tag?: string; + tagData?: TagData[]; /** * Create an action. @@ -62,15 +71,15 @@ class Action { this.type = type; this.method = method; this.timestamp = timestamp; - this.project = repo.split("/")[0]; - this.repoName = repo.split("/")[1]; + this.project = repo.split('/')[0]; + this.repoName = repo.split('/')[1]; this.url = `${getProxyUrl()}/${repo}`; this.repo = repo; } /** * Add a step to the action. - * @param {Step} step + * @param {Step} step */ addStep(step: Step): void { this.steps.push(step); diff --git a/src/proxy/processors/push-action/checkAuthorEmails.ts b/src/proxy/processors/push-action/checkAuthorEmails.ts index a25a0e6c9..ff1891066 100644 --- a/src/proxy/processors/push-action/checkAuthorEmails.ts +++ b/src/proxy/processors/push-action/checkAuthorEmails.ts @@ -1,6 +1,6 @@ import { Action, Step } from '../../actions'; import { getCommitConfig } from '../../../config'; -import { Commit } from '../../actions/Action'; +import { CommitData } from '../../actions/Action'; const commitConfig = getCommitConfig(); @@ -27,14 +27,16 @@ const isEmailAllowed = (email: string): boolean => { } return true; -} +}; const exec = async (req: any, action: Action): Promise => { console.log({ req, action }); const step = new Step('checkAuthorEmails'); - const uniqueAuthorEmails = [...new Set(action.commitData?.map((commit: Commit) => commit.authorEmail))]; + const uniqueAuthorEmails = [ + ...new Set(action.commitData?.map((commit: CommitData) => commit.authorEmail)), + ]; console.log({ uniqueAuthorEmails }); const illegalEmails = uniqueAuthorEmails.filter((email) => !isEmailAllowed(email)); From eeb0f013273cab6ae0a1ed6eda2378fe39b2bd82 Mon Sep 17 00:00:00 2001 From: fabiovincenzi Date: Sun, 8 Jun 2025 17:20:57 +0200 Subject: [PATCH 03/21] feat: handle tags in parsepush --- src/proxy/processors/push-action/parsePush.ts | 250 +++++++++++------- 1 file changed, 148 insertions(+), 102 deletions(-) diff --git a/src/proxy/processors/push-action/parsePush.ts b/src/proxy/processors/push-action/parsePush.ts index 42adcb48e..678f7a1f5 100644 --- a/src/proxy/processors/push-action/parsePush.ts +++ b/src/proxy/processors/push-action/parsePush.ts @@ -2,8 +2,8 @@ import { Action, Step } from '../../actions'; import zlib from 'zlib'; import fs from 'fs'; import path from 'path'; -import lod from 'lodash'; import { CommitContent } from '../types'; +import { CommitData, TagData } from '../../actions/Action'; const BitMask = require('bit-mask') as any; const dir = path.resolve(__dirname, './.tmp'); @@ -20,8 +20,14 @@ async function exec(req: any, action: Action): Promise { throw new Error('No body found in request'); } const messageParts = req.body.toString('utf8').split(' '); + console.log('messageParts', messageParts); - action.branch = messageParts[2].trim().replace('\u0000', ''); + const refName = messageParts[2].replace('\u0000', '').trim(); + const isTag = refName.startsWith('refs/tags/'); + const isBranch = refName.startsWith('refs/heads/'); + + action.branch = isBranch ? refName : undefined; + action.tag = isTag ? refName : undefined; action.setCommit(messageParts[0].substr(4), messageParts[1]); const index = req.body.lastIndexOf('PACK'); @@ -29,15 +35,33 @@ async function exec(req: any, action: Action): Promise { const [meta, contentBuff] = getPackMeta(buf); const contents = getContents(contentBuff as any, meta.entries as number); - action.commitData = getCommitData(contents as any); + const ParsedObjects = { + commits: [] as CommitData[], + tags: [] as TagData[], + }; - if (action.commitFrom === '0000000000000000000000000000000000000000') { - action.commitFrom = action.commitData[action.commitData.length - 1].parent; + for (const obj of contents) { + if (obj.type === 1) ParsedObjects.commits.push(parseCommit(obj)); + else if (obj.type === 4) ParsedObjects.tags.push(parseTag(obj)); } - const user = action.commitData[action.commitData.length - 1].committer; - console.log(`Push Request received from user ${user}`); - action.user = user; + action.commitData = ParsedObjects.commits; + action.tagData = ParsedObjects.tags; + + if (action.commitData.length) { + if (action.commitFrom === '0000000000000000000000000000000000000000') { + action.commitFrom = action.commitData[action.commitData.length - 1].parent; + } + action.user = action.commitData.at(-1)!.committer; + } else if (action.tagData?.length) { + action.user = action.tagData.at(-1)!.tagger; + } else if (action.tag) { + // lightweight tag + console.log(req.body.toString('utf8')); + action.user = req.user.username; + } else { + action.user = 'unknown'; + } step.content = { meta: meta, @@ -50,92 +74,118 @@ async function exec(req: any, action: Action): Promise { action.addStep(step); } return action; -}; +} -const getCommitData = (contents: CommitContent[]) => { - console.log({ contents }); - return lod - .chain(contents) - .filter({ type: 1 }) - .map((x) => { - console.log({ x }); +function parseCommit(x: CommitContent): CommitData { + console.log({ x }); + const lines = x.content.split('\n'); + console.log({ lines }); - const formattedContent = x.content.split('\n'); - console.log({ formattedContent }); + const parts = lines.filter((part) => part.length > 0); + console.log({ parts }); - const parts = formattedContent.filter((part) => part.length > 0); - console.log({ parts }); + if (!parts || parts.length < 5) { + throw new Error('Invalid commit data'); + } - if (!parts || parts.length < 5) { - throw new Error('Invalid commit data'); - } + const tree = parts + .find((t) => t.split(' ')[0] === 'tree') + ?.replace('tree', '') + .trim(); + console.log({ tree }); + const parent = + lines + .find((l) => l.startsWith('parent ')) + ?.slice(7) + .trim() ?? '0000000000000000000000000000000000000000'; + console.log({ parent }); + const authorLine = lines + .find((l) => l.startsWith('author ')) + ?.slice(7) + .trim(); + console.log({ authorLine }); + const committerLine = lines + .find((l) => l.startsWith('committer ')) + ?.slice(10) + .trim(); + console.log({ committerLine }); + const msgIndex = lines.indexOf(''); + const message = lines.slice(msgIndex + 1, lines.length - 1).join(' '); + + console.log({ message }); + + const commitTimestamp = committerLine?.split(' ').reverse()[1]; + console.log({ commitTimestamp }); + const authorEmail = authorLine?.split(' ').reverse()[2].slice(1, -1); + console.log({ authorEmail }); + + console.log({ + tree, + parent, + authorLine: authorLine?.split('<')[0].trim(), + committerLine: committerLine?.split('<')[0].trim(), + commitTimestamp, + message, + authorEmail, + }); + if ( + !tree || + !parent || + !authorLine || + !committerLine || + !commitTimestamp || + !message || + !authorEmail + ) { + throw new Error('Invalid commit data'); + } - const tree = parts - .find((t) => t.split(' ')[0] === 'tree') - ?.replace('tree', '') - .trim(); - console.log({ tree }); - - const parentValue = parts.find((t) => t.split(' ')[0] === 'parent'); - console.log({ parentValue }); - - const parent = parentValue - ? parentValue.replace('parent', '').trim() - : '0000000000000000000000000000000000000000'; - console.log({ parent }); - - const author = parts - .find((t) => t.split(' ')[0] === 'author') - ?.replace('author', '') - .trim(); - console.log({ author }); - - const committer = parts - .find((t) => t.split(' ')[0] === 'committer') - ?.replace('committer', '') - .trim(); - console.log({ committer }); - - const indexOfMessages = formattedContent.indexOf(''); - console.log({ indexOfMessages }); - - const message = formattedContent - .slice(indexOfMessages + 1, formattedContent.length - 1) - .join(' '); - console.log({ message }); - - const commitTimestamp = committer?.split(' ').reverse()[1]; - console.log({ commitTimestamp }); - - const authorEmail = author?.split(' ').reverse()[2].slice(1, -1); - console.log({ authorEmail }); - - console.log({ - tree, - parent, - author: author?.split('<')[0].trim(), - committer: committer?.split('<')[0].trim(), - commitTimestamp, - message, - authorEmail, - }); - - if (!tree || !parent || !author || !committer || !commitTimestamp || !message || !authorEmail) { - throw new Error('Invalid commit data'); - } + return { + tree, + parent, + author: authorLine.split('<')[0].trim(), + committer: committerLine.split('<')[0].trim(), + commitTimestamp, + message, + authorEmail, + }; +} - return { - tree, - parent, - author: author.split('<')[0].trim(), - committer: committer.split('<')[0].trim(), - commitTimestamp, - message, - authorEmail: authorEmail, - }; - }) - .value(); -}; +function parseTag(x: CommitContent): TagData { + const lines = x.content.split('\n'); + const object = lines + .find((l) => l.startsWith('object ')) + ?.slice(7) + .trim(); + const typeLine = lines + .find((l) => l.startsWith('type ')) + ?.slice(5) + .trim(); // commit | tree | blob + const tagName = lines + .find((l) => l.startsWith('tag ')) + ?.slice(4) + .trim(); + const rawTagger = lines + .find((l) => l.startsWith('tagger ')) + ?.slice(7) + .trim(); + if (!rawTagger) throw new Error('Invalid tag object: no tagger line'); + + const taggerName = rawTagger.split('<')[0].trim(); + + const messageIndex = lines.indexOf(''); + const message = lines.slice(messageIndex + 1).join('\n'); + + if (!object || !typeLine || !tagName || !taggerName) throw new Error('Invalid tag object'); + + return { + object, + type: typeLine, + tagName, + tagger: taggerName, + message, + }; +} const getPackMeta = (buffer: Buffer) => { const sig = buffer.slice(0, 4).toString('utf-8'); @@ -151,8 +201,8 @@ const getPackMeta = (buffer: Buffer) => { return [meta, buffer.slice(12)]; }; -const getContents = (buffer: Buffer | CommitContent[], entries: number) => { - const contents = []; +const getContents = (buffer: Buffer, entries: number): CommitContent[] => { + const contents: CommitContent[] = []; for (let i = 0; i < entries; i++) { try { @@ -177,7 +227,7 @@ const getInt = (bits: boolean[]) => { return parseInt(strBits, 2); }; -const getContent = (item: number, buffer: Buffer) => { +const getContent = (item: number, buffer: Buffer): [CommitContent, Buffer] => { // FIRST byte contains the type and some of the size of the file // a MORE flag -8th byte tells us if there is a subsequent byte // which holds the file size @@ -227,14 +277,14 @@ const getContent = (item: number, buffer: Buffer) => { // NOTE Size is the unziped size, not the zipped size // so it's kind of useless for us in terms of reading the stream - const result = { - item: item, + const result: CommitContent = { + item, value: byte, - type: type, + type, size: intSize, - deflatedSize: deflatedSize, - objectRef: objectRef, - content: content, + deflatedSize: deflatedSize as number, + objectRef, + content: content as string, }; // Move on by the zipped content size. @@ -256,8 +306,4 @@ const unpack = (buf: Buffer) => { exec.displayName = 'parsePush.exec'; -export { - exec, - getPackMeta, - unpack -}; +export { exec, getPackMeta, unpack }; From 87bee022fe48fe9acb314cec9736acd93e5f355a Mon Sep 17 00:00:00 2001 From: fabiovincenzi Date: Sun, 8 Jun 2025 17:21:33 +0200 Subject: [PATCH 04/21] feat: handle tags in PushesTable view --- .../components/PushesTable.jsx | 153 +++++++++++------- 1 file changed, 98 insertions(+), 55 deletions(-) diff --git a/src/ui/views/OpenPushRequests/components/PushesTable.jsx b/src/ui/views/OpenPushRequests/components/PushesTable.jsx index 2a3a7f33a..0d3daf1f9 100644 --- a/src/ui/views/OpenPushRequests/components/PushesTable.jsx +++ b/src/ui/views/OpenPushRequests/components/PushesTable.jsx @@ -28,7 +28,8 @@ export default function PushesTable(props) { const [currentPage, setCurrentPage] = useState(1); const itemsPerPage = 5; const [searchTerm, setSearchTerm] = useState(''); - const openPush = (push) => navigate(`/dashboard/push/${push}`, { replace: true }); + + const openPush = (pushId) => navigate(`/dashboard/push/${pushId}`, { replace: true }); useEffect(() => { const query = {}; @@ -42,15 +43,22 @@ export default function PushesTable(props) { setFilteredData(data); }, [data]); + // Include “tag” in the searchable fields when tag exists useEffect(() => { const lowerCaseTerm = searchTerm.toLowerCase(); const filtered = searchTerm - ? data.filter( - (item) => - item.repo.toLowerCase().includes(lowerCaseTerm) || - item.commitTo.toLowerCase().includes(lowerCaseTerm) || - item.commitData[0].message.toLowerCase().includes(lowerCaseTerm), - ) + ? data.filter((item) => { + const repoName = item.repo.toLowerCase(); + const commitMsg = item.commitData?.[0]?.message?.toLowerCase() || ''; + const commitToSha = item.commitTo.toLowerCase(); + const tagName = item.tag?.replace('refs/tags/', '').toLowerCase() || ''; + return ( + repoName.includes(lowerCaseTerm) || + commitToSha.includes(lowerCaseTerm) || + commitMsg.includes(lowerCaseTerm) || + tagName.includes(lowerCaseTerm) + ); + }) : data; setFilteredData(filtered); setCurrentPage(1); @@ -73,87 +81,122 @@ export default function PushesTable(props) { return (
- {} + - +
Timestamp Repository - Branch - Commit SHA - Committer + Branch/Tag + Commit SHA/Tag + Committer/Tagger Author Author E-mail - Commit Message + Message No. of Commits {currentItems.reverse().map((row) => { + const isTagPush = Boolean(row.tag); const repoFullName = row.repo.replace('.git', ''); - const repoBranch = row.branch.replace('refs/heads/', ''); + const firstCommit = row.commitData?.[0] || null; + const tagName = isTagPush ? row.tag.replace('refs/tags/', '') : ''; + const timestampUnix = isTagPush + ? /* use row.user’s timestamp? fallback to commitTimestamp */ firstCommit?.commitTimestamp + : firstCommit?.commitTimestamp; + const displayTime = timestampUnix ? moment.unix(timestampUnix).toString() : '—'; + const refToShow = isTagPush ? tagName : row.branch.replace('refs/heads/', ''); + const shaOrTag = isTagPush ? tagName : row.commitTo.substring(0, 8); + const committerOrTagger = isTagPush ? row.user : firstCommit?.committer; + const authorOrNA = isTagPush ? '—' : firstCommit?.author || '—'; + const authorEmailOrNA = isTagPush ? '—' : firstCommit?.authorEmail || '—'; + const messageOrNote = isTagPush + ? /* for lightweight tags, commitData[0].message is often the tagger’s commit message */ + firstCommit?.message || '' + : firstCommit?.message || ''; + const commitCount = row.commitData?.length || 0; return ( + {displayTime} - {moment - .unix(row.commitData[0].commitTs || row.commitData[0].commitTimestamp) - .toString()} - - - + {repoFullName} + - - {repoBranch} - + {isTagPush ? ( + + {refToShow} + + ) : ( + + {refToShow} + + )} - - {row.commitTo.substring(0, 8)} - + {isTagPush ? ( + + {shaOrTag} + + ) : ( + + {shaOrTag} + + )} - - {row.commitData[0].committer} - + {committerOrTagger !== '—' ? ( + + {committerOrTagger} + + ) : ( + '—' + )} - - {row.commitData[0].author} - + {authorOrNA !== '—' ? ( + + {authorOrNA} + + ) : ( + '—' + )} - {row.commitData[0].authorEmail ? ( - - {row.commitData[0].authorEmail} - + {authorEmailOrNA !== '—' ? ( + {authorEmailOrNA} ) : ( - 'No data...' + '—' )} - {row.commitData[0].message} - {row.commitData.length} + {messageOrNote} + {commitCount} - - - - ) : null} - {data.attestation && data.authorised ? ( -
- - +
+ + + +
+ + {/* Attestation detail panel if attested and authorised */} + {data.attestation && data.authorised ? ( +
{ - if (!data.autoApproved) { - setAttestation(true); - } - }} - htmlColor='green' - /> - - - {data.autoApproved ? ( - <> -
-

- Auto-approved by system -

-
- - ) : ( - <> - - + + { + if (!data.autoApproved) setAttestation(true); + }} + htmlColor='green' /> - -
-

+ + + {data.autoApproved ? ( +

+

+ Auto-approved by system +

+
+ ) : ( + <> - {data.attestation.reviewer.gitAccount} - {' '} - approved this contribution -

-
- - )} + Reviewer + + + + )} - - - {moment(data.attestation.timestamp).fromNow()} - - + + + {moment(data.attestation.timestamp).fromNow()} + + - {data.autoApproved ? ( - <> - ) : ( - - )} -
- ) : null} + {!data.autoApproved && ( + + )} +
+ ) : null} + + )} @@ -244,109 +221,159 @@ export default function Dashboard() {

{moment(data.timestamp).toString()}

-

Remote Head

+

Repository

- - {data.commitFrom} + + {repoFullName}

-

Commit SHA

+ {isTagPush ? ( + <> +

Tag

+

{data.tag.replace('refs/tags/', '')}

+ + ) : ( + <> +

Branch

+

{repoBranch}

+ + )} + + +

From

- {data.commitTo} - -

- - -

Repository

-

- - {repoFullName} + {data.commitFrom}

-

Branch

+

To

- {repoBranch} + {data.commitTo}

- - -

{headerData.title}

- - -
- - Timestamp - Committer - Author - Author E-mail - Message - - - {data.commitData.map((c) => ( - - - {moment.unix(c.commitTs || c.commitTimestamp).toString()} - - - - {c.committer} - - - - - {c.author} - - - - {c.authorEmail ? ( - {c.authorEmail} - ) : ( - 'No data...' - )} - - {c.message} - - ))} - -
- - - - - - - - - - - + + {/* Branch push: show commits and diff */} + {!isTagPush && ( + <> + + + +

{headerData.title}

+ + + + + Timestamp + Committer + Author + Email + Message + + + {data.commitData.map((c) => ( + + + {moment.unix(c.commitTs || c.commitTimestamp).toString()} + + + + {c.committer} + + + + + {c.author} + + + + {c.authorEmail ? ( + {c.authorEmail} + ) : ( + '-' + )} + + {c.message} + + ))} + +
+ + + + + + + + + + + + )} + + {/* Tag push: show tagData */} + {isTagPush && ( + + + +

Tag Details

+ + + + + Tag Name + Type + Tagger + Message + + + {data.tagData.map((t) => ( + + {t.tagName} + {t.type} + + + {t.tagger} + + + {t.message} + + ))} + +
+ + + + )}
); From 53bf1d0730396bfd8ed61f193cc7121a6e796fb1 Mon Sep 17 00:00:00 2001 From: fabiovincenzi Date: Wed, 11 Jun 2025 15:05:44 +0200 Subject: [PATCH 06/21] feat: add chain tests for tags --- test/chain.test.js | 40 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/test/chain.test.js b/test/chain.test.js index 1fc749248..d1ad0a907 100644 --- a/test/chain.test.js +++ b/test/chain.test.js @@ -1,3 +1,4 @@ +const { Action } = require('../src/proxy/actions/Action'); const chai = require('chai'); const sinon = require('sinon'); const { PluginLoader } = require('../src/plugin'); @@ -95,14 +96,14 @@ describe('proxy chain', function () { it('getChain should set pluginLoaded if loader is undefined', async function () { chain.chainPluginLoader = undefined; const actual = await chain.getChain({ type: 'push' }); - expect(actual).to.deep.equal(chain.pushActionChain); + expect(actual).to.deep.equal(chain.branchPushChain); expect(chain.chainPluginLoader).to.be.undefined; expect(chain.pluginsInserted).to.be.true; }); it('getChain should load plugins from an initialized PluginLoader', async function () { chain.chainPluginLoader = mockLoader; - const initialChain = [...chain.pushActionChain]; + const initialChain = [...chain.branchPushChain]; const actual = await chain.getChain({ type: 'push' }); expect(actual.length).to.be.greaterThan(initialChain.length); expect(chain.pluginsInserted).to.be.true; @@ -452,4 +453,39 @@ describe('proxy chain', function () { db.reject.restore(); consoleErrorStub.restore(); }); + + it('returns pullActionChain for pull actions', async () => { + const action = new Action('1', 'pull', 'GET', Date.now(), 'owner/repo.git'); + const pullChain = await chain.getChain(action); + expect(pullChain).to.deep.equal(chain.pullActionChain); + }); + + it('returns tagPushChain when action.type is push and action.tag is set', async () => { + const action = new Action('2', 'push', 'POST', Date.now(), 'owner/repo.git'); + action.tag = 'refs/tags/v1.0'; + const tagChain = await chain.getChain(action); + expect(tagChain).to.deep.equal(chain.tagPushChain); + }); + + it('returns branchPushChain when action.type is push and no tag', async () => { + const action = new Action('3', 'push', 'POST', Date.now(), 'owner/repo.git'); + action.tag = undefined; + const branchChain = await chain.getChain(action); + expect(branchChain).to.deep.equal(chain.branchPushChain); + }); + it('getChain should set pluginsInserted and return tagPushChain if loader is undefined for tag pushes', async function () { + chain.chainPluginLoader = undefined; + const actual = await chain.getChain({ type: 'push', tag: 'refs/tags/v1.0' }); + expect(actual).to.deep.equal(chain.tagPushChain); + expect(chain.chainPluginLoader).to.be.undefined; + expect(chain.pluginsInserted).to.be.true; + }); + + it('getChain should load tag plugins from an initialized PluginLoader', async function () { + chain.chainPluginLoader = mockLoader; + const initialChain = [...chain.tagPushChain]; + const actual = await chain.getChain({ type: 'push', tag: 'refs/tags/v2.0' }); + expect(actual.length).to.be.greaterThan(initialChain.length); + expect(chain.pluginsInserted).to.be.true; + }); }); From b6fe22a1f5b3321c7d94c2bf5833012a1cc50ace Mon Sep 17 00:00:00 2001 From: fabiovincenzi Date: Wed, 11 Jun 2025 16:12:25 +0200 Subject: [PATCH 07/21] chore: remove lightweight tag support --- src/proxy/processors/push-action/parsePush.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/proxy/processors/push-action/parsePush.ts b/src/proxy/processors/push-action/parsePush.ts index f509e3278..58c7f6c09 100644 --- a/src/proxy/processors/push-action/parsePush.ts +++ b/src/proxy/processors/push-action/parsePush.ts @@ -54,10 +54,6 @@ async function exec(req: any, action: Action): Promise { action.user = action.commitData.at(-1)!.committer; } else if (action.tagData?.length) { action.user = action.tagData.at(-1)!.tagger; - } else if (action.tag) { - // lightweight tag - console.log(req.body.toString('utf8')); - action.user = req.user.username; } else { action.user = 'unknown'; } From ca77a6f7b26e125a7b1d390371b4b0aca3cde03d Mon Sep 17 00:00:00 2001 From: fabiovincenzi Date: Tue, 22 Jul 2025 10:34:19 +0200 Subject: [PATCH 08/21] refactor: improve tag push implementation with utilities and types --- .../ErrorBoundary/ErrorBoundary.tsx | 70 +++++++++++++++++++ 1 file changed, 70 insertions(+) create mode 100644 src/ui/components/ErrorBoundary/ErrorBoundary.tsx diff --git a/src/ui/components/ErrorBoundary/ErrorBoundary.tsx b/src/ui/components/ErrorBoundary/ErrorBoundary.tsx new file mode 100644 index 000000000..548d055df --- /dev/null +++ b/src/ui/components/ErrorBoundary/ErrorBoundary.tsx @@ -0,0 +1,70 @@ +import React, { Component, ErrorInfo, ReactNode } from 'react'; + +interface Props { + children: ReactNode; + fallback?: ReactNode; +} + +interface State { + hasError: boolean; + error?: Error; +} + +class ErrorBoundary extends Component { + public state: State = { + hasError: false, + }; + + public static getDerivedStateFromError(error: Error): State { + return { hasError: true, error }; + } + + public componentDidCatch(error: Error, errorInfo: ErrorInfo) { + console.error('ErrorBoundary caught an error:', error, errorInfo); + } + + public render() { + if (this.state.hasError) { + return ( + this.props.fallback || ( +
+

Something went wrong

+

We encountered an error while displaying this content.

+ {process.env.NODE_ENV === 'development' && this.state.error && ( +
+ Error details (development only) + 
+                  {this.state.error.stack}
+
+
+ )} + +
+ ) + ); + } + + return this.props.children; + } +} + +export default ErrorBoundary; \ No newline at end of file From 0cb4288f37afd0cd5d6a343b0a20d4d859813249 Mon Sep 17 00:00:00 2001 From: fabiovincenzi Date: Tue, 22 Jul 2025 10:39:05 +0200 Subject: [PATCH 09/21] refactor: improve tag push implementation with utilities and types --- src/types/models.ts | 12 ++ src/ui/utils/pushUtils.ts | 174 ++++++++++++++++++ .../components/PushesTable.tsx | 134 ++++++-------- src/ui/views/PushDetails/PushDetails.tsx | 34 ++-- 4 files changed, 266 insertions(+), 88 deletions(-) create mode 100644 src/ui/utils/pushUtils.ts diff --git a/src/types/models.ts b/src/types/models.ts index a114683e8..8436004fe 100644 --- a/src/types/models.ts +++ b/src/types/models.ts @@ -22,6 +22,14 @@ export interface CommitData { commitTimestamp?: number; } +export interface TagData { + tagName: string; + type: 'lightweight' | 'annotated'; + tagger: string; + message: string; + timestamp?: number; +} + export interface PushData { id: string; repo: string; @@ -38,6 +46,10 @@ export interface PushData { attestation?: AttestationData; autoApproved?: boolean; timestamp: string | Date; + // Tag-specific fields + tag?: string; + tagData?: TagData[]; + user?: string; // Used for tag pushes as the tagger } export interface Route { diff --git a/src/ui/utils/pushUtils.ts b/src/ui/utils/pushUtils.ts new file mode 100644 index 000000000..f03b15ba3 --- /dev/null +++ b/src/ui/utils/pushUtils.ts @@ -0,0 +1,174 @@ +import moment from 'moment'; +import { CommitData, PushData, TagData } from '../../types/models'; +import { trimPrefixRefsHeads, trimTrailingDotGit } from '../../db/helper'; + +/** + * Determines if a push is a tag push + * @param {PushData} pushData - The push data to check + * @return {boolean} True if this is a tag push, false otherwise + */ +export const isTagPush = (pushData: PushData): boolean => { + return Boolean(pushData?.tag && pushData?.tagData && pushData.tagData.length > 0); +}; + +/** + * Gets the display timestamp for a push (handles both commits and tags) + * @param {boolean} isTag - Whether this is a tag push + * @param {CommitData | null} commitData - The commit data + * @param {TagData} [tagData] - The tag data (optional) + * @return {string} Formatted timestamp string or 'N/A' + */ +export const getDisplayTimestamp = ( + isTag: boolean, + commitData: CommitData | null, + tagData?: TagData +): string => { + if (isTag && tagData?.timestamp) { + return moment.unix(tagData.timestamp).toString(); + } + + if (commitData) { + const timestamp = commitData.commitTimestamp || commitData.commitTs; + return timestamp ? moment.unix(timestamp).toString() : 'N/A'; + } + + return 'N/A'; +}; + +/** + * Safely extracts tag name from git reference + * @param {string} [tagRef] - The git tag reference (e.g., 'refs/tags/v1.0.0') + * @return {string} The tag name without the 'refs/tags/' prefix + */ +export const getTagName = (tagRef?: string): string => { + if (!tagRef || typeof tagRef !== 'string') return ''; + try { + return tagRef.replace('refs/tags/', ''); + } catch (error) { + console.warn('Error parsing tag reference:', tagRef, error); + return ''; + } +}; + +/** + * Gets the appropriate reference to show (tag name or branch name) + * @param {PushData} pushData - The push data + * @return {string} The reference name to display + */ +export const getRefToShow = (pushData: PushData): string => { + if (isTagPush(pushData)) { + return getTagName(pushData.tag); + } + return trimPrefixRefsHeads(pushData.branch); +}; + +/** + * Gets the SHA or tag identifier for display + * @param {PushData} pushData - The push data + * @return {string} The SHA (shortened) or tag name + */ +export const getShaOrTag = (pushData: PushData): string => { + if (isTagPush(pushData)) { + return getTagName(pushData.tag); + } + + if (!pushData.commitTo || typeof pushData.commitTo !== 'string') { + console.warn('Invalid commitTo value:', pushData.commitTo); + return 'N/A'; + } + + return pushData.commitTo.substring(0, 8); +}; + +/** + * Gets the committer or tagger based on push type + * @param {PushData} pushData - The push data + * @return {string} The committer username for commits or tagger for tags + */ +export const getCommitterOrTagger = (pushData: PushData): string => { + if (isTagPush(pushData) && pushData.user) { + return pushData.user; + } + + if (!pushData.commitData || !Array.isArray(pushData.commitData) || pushData.commitData.length === 0) { + console.warn('Invalid or empty commitData:', pushData.commitData); + return 'N/A'; + } + + return pushData.commitData[0]?.committer || 'N/A'; +}; + +/** + * Gets the author (N/A for tag pushes) + * @param {PushData} pushData - The push data + * @return {string} The author username or 'N/A' for tag pushes + */ +export const getAuthor = (pushData: PushData): string => { + if (isTagPush(pushData)) { + return 'N/A'; + } + return pushData.commitData[0]?.author || 'N/A'; +}; + +/** + * Gets the author email (N/A for tag pushes) + * @param {PushData} pushData - The push data + * @return {string} The author email or 'N/A' for tag pushes + */ +export const getAuthorEmail = (pushData: PushData): string => { + if (isTagPush(pushData)) { + return 'N/A'; + } + return pushData.commitData[0]?.authorEmail || 'N/A'; +}; + +/** + * Gets the message (tag message or commit message) + * @param {PushData} pushData - The push data + * @return {string} The appropriate message for the push type + */ +export const getMessage = (pushData: PushData): string => { + if (isTagPush(pushData)) { + // For tags, try tag message first, then fallback to commit message + return pushData.tagData?.[0]?.message || pushData.commitData[0]?.message || ''; + } + return pushData.commitData[0]?.message || 'N/A'; +}; + +/** + * Gets the commit count + * @param {PushData} pushData - The push data + * @return {number} The number of commits in the push + */ +export const getCommitCount = (pushData: PushData): number => { + return pushData.commitData?.length || 0; +}; + +/** + * Gets the cleaned repository name + * @param {string} repo - The repository name (may include .git suffix) + * @return {string} The cleaned repository name without .git suffix + */ +export const getRepoFullName = (repo: string): string => { + return trimTrailingDotGit(repo); +}; + +/** + * Generates GitHub URLs for different reference types + */ +export const getGitHubUrl = { + repo: (repoName: string) => `https://github.com/${repoName}`, + commit: (repoName: string, sha: string) => `https://github.com/${repoName}/commit/${sha}`, + branch: (repoName: string, branch: string) => `https://github.com/${repoName}/tree/${branch}`, + tag: (repoName: string, tagName: string) => `https://github.com/${repoName}/releases/tag/${tagName}`, + user: (username: string) => `https://github.com/${username}`, +}; + +/** + * Checks if a value is not "N/A" and not empty + * @param {string | undefined} value - The value to check + * @return {boolean} True if the value is valid (not N/A and not empty) + */ +export const isValidValue = (value: string | undefined): value is string => { + return Boolean(value && value !== 'N/A'); +}; \ No newline at end of file diff --git a/src/ui/views/OpenPushRequests/components/PushesTable.tsx b/src/ui/views/OpenPushRequests/components/PushesTable.tsx index ab5e7d9dc..19c509e23 100644 --- a/src/ui/views/OpenPushRequests/components/PushesTable.tsx +++ b/src/ui/views/OpenPushRequests/components/PushesTable.tsx @@ -1,6 +1,5 @@ import React, { useState, useEffect } from 'react'; import { makeStyles } from '@material-ui/core/styles'; -import moment from 'moment'; import { useNavigate } from 'react-router-dom'; import Button from '@material-ui/core/Button'; import Table from '@material-ui/core/Table'; @@ -15,8 +14,23 @@ import { getPushes } from '../../../services/git-push'; import { KeyboardArrowRight } from '@material-ui/icons'; import Search from '../../../components/Search/Search'; import Pagination from '../../../components/Pagination/Pagination'; +import ErrorBoundary from '../../../components/ErrorBoundary/ErrorBoundary'; import { PushData } from '../../../../types/models'; -import { trimPrefixRefsHeads, trimTrailingDotGit } from '../../../../db/helper'; +import { + isTagPush, + getDisplayTimestamp, + getTagName, + getRefToShow, + getShaOrTag, + getCommitterOrTagger, + getAuthor, + getAuthorEmail, + getMessage, + getCommitCount, + getRepoFullName, + getGitHubUrl, + isValidValue, +} from '../../../utils/pushUtils'; interface PushesTableProps { [key: string]: any; @@ -58,14 +72,15 @@ const PushesTable: React.FC = (props) => { const lowerCaseTerm = searchTerm.toLowerCase(); const filtered = searchTerm ? data.filter((item) => { - const repoName = item.repo.toLowerCase(); - const commitMsg = item.commitData?.[0]?.message?.toLowerCase() || ''; + const repoName = getRepoFullName(item.repo).toLowerCase(); + const message = getMessage(item).toLowerCase(); const commitToSha = item.commitTo.toLowerCase(); - const tagName = item.tag?.replace('refs/tags/', '').toLowerCase() || ''; + const tagName = getTagName(item.tag).toLowerCase(); + return ( repoName.includes(lowerCaseTerm) || commitToSha.includes(lowerCaseTerm) || - commitMsg.includes(lowerCaseTerm) || + message.includes(lowerCaseTerm) || tagName.includes(lowerCaseTerm) ); }) @@ -88,9 +103,10 @@ const PushesTable: React.FC = (props) => { if (isError) return
{errorMessage}
; return ( -
- - + +
+ + @@ -108,77 +124,46 @@ const PushesTable: React.FC = (props) => { {[...currentItems].reverse().map((row) => { - const isTagPush = Boolean(row.tag); - const repoFullName = trimTrailingDotGit(row.repo); - const firstCommit = row.commitData?.[0] || null; - const tagName = isTagPush ? row.tag.replace('refs/tags/', '') : ''; - const timestampUnix = isTagPush - ? firstCommit?.commitTimestamp - : firstCommit?.commitTimestamp || firstCommit?.commitTs; - const displayTime = timestampUnix ? moment.unix(timestampUnix).toString() : 'N/A'; - const refToShow = isTagPush ? tagName : trimPrefixRefsHeads(row.branch); - const shaOrTag = isTagPush ? tagName : row.commitTo.substring(0, 8); - const committerOrTagger = isTagPush ? row.user : firstCommit?.committer; - const authorOrNA = isTagPush ? 'N/A' : firstCommit?.author || 'N/A'; - const authorEmailOrNA = isTagPush ? 'N/A' : firstCommit?.authorEmail || 'N/A'; - const messageOrNote = isTagPush - ? firstCommit?.message || '' - : firstCommit?.message || 'N/A'; - const commitCount = row.commitData?.length || 0; + const isTag = isTagPush(row); + const repoFullName = getRepoFullName(row.repo); + const displayTime = getDisplayTimestamp(isTag, row.commitData[0], row.tagData?.[0]); + const refToShow = getRefToShow(row); + const shaOrTag = getShaOrTag(row); + const committerOrTagger = getCommitterOrTagger(row); + const author = getAuthor(row); + const authorEmail = getAuthorEmail(row); + const message = getMessage(row); + const commitCount = getCommitCount(row); return ( {displayTime} - + {repoFullName} - {isTagPush ? ( - - {refToShow} - - ) : ( - - {refToShow} - - )} + + {refToShow} + - {isTagPush ? ( - - {shaOrTag} - - ) : ( - - {shaOrTag} - - )} + + {shaOrTag} + - {committerOrTagger && committerOrTagger !== 'N/A' ? ( - + {isValidValue(committerOrTagger) ? ( + {committerOrTagger} ) : ( @@ -186,22 +171,22 @@ const PushesTable: React.FC = (props) => { )} - {authorOrNA !== 'N/A' ? ( - - {authorOrNA} + {isValidValue(author) ? ( + + {author} ) : ( 'N/A' )} - {authorEmailOrNA !== 'N/A' ? ( - {authorEmailOrNA} + {isValidValue(authorEmail) ? ( + {authorEmail} ) : ( 'N/A' )} - {messageOrNote} + {message} {commitCount}
Tag Name - Type Tagger + Email Message {data.tagData.map((t) => ( {t.tagName} - {t.type} { {t.tagger} + + {t.taggerEmail ? ( + {t.taggerEmail} + ) : ( + '-' + )} + {t.message} ))} diff --git a/test/pushUtils.test.js b/test/pushUtils.test.js index 742bb45ce..06493461a 100644 --- a/test/pushUtils.test.js +++ b/test/pushUtils.test.js @@ -183,8 +183,8 @@ describe('pushUtils', () => { }); describe('getAuthor', () => { - it('returns N/A for tag push', () => { - expect(getAuthor(mockTagPush)).to.equal('N/A'); + it('returns tagger for tag push', () => { + expect(getAuthor(mockTagPush)).to.equal('release-bot'); }); it('returns author for commit push', () => { diff --git a/test/testParsePush.test.js b/test/testParsePush.test.js index e4d096962..d87a9609d 100644 --- a/test/testParsePush.test.js +++ b/test/testParsePush.test.js @@ -8,7 +8,7 @@ const { getPackMeta, parsePacketLines, parseTag, - unpack + unpack, } = require('../src/proxy/processors/push-action/parsePush'); import { EMPTY_COMMIT_HASH, FLUSH_PACKET, PACK_SIGNATURE } from '../src/proxy/processors/constants'; @@ -19,7 +19,7 @@ import { EMPTY_COMMIT_HASH, FLUSH_PACKET, PACK_SIGNATURE } from '../src/proxy/pr * @param {string} commitContent - Content of the commit object. * @param {number} type - Type of the object (1 for commit). * @return {Buffer} - The generated PACK buffer. - */ + */ function createSamplePackBuffer( numEntries = 1, commitContent = 'tree 123\nparent 456\nauthor A 123 +0000\ncommitter C 456 +0000\n\nmessage', @@ -56,7 +56,7 @@ function createSamplePackBuffer( */ function createPacketLineBuffer(lines) { let buffer = Buffer.alloc(0); - lines.forEach(line => { + lines.forEach((line) => { const lengthInHex = (line.length + 4).toString(16).padStart(4, '0'); buffer = Buffer.concat([buffer, Buffer.from(lengthInHex, 'ascii'), Buffer.from(line, 'ascii')]); }); @@ -70,7 +70,7 @@ function createPacketLineBuffer(lines) { * @param {string} tagContent - Content of the tag object. * @param {number} type - Type of the object (4 for tag). * @return {Buffer} - The generated PACK buffer. - */ + */ function createSampleTagPackBuffer( tagContent = 'object 1234567890abcdef1234567890abcdef12345678\ntype commit\ntag v1.0.0\ntagger Test Tagger 1234567890 +0000\n\nTag message', type = 4, @@ -180,8 +180,8 @@ describe('parsePackFile', () => { const step = action.steps[0]; expect(step.stepName).to.equal('parsePackFile'); expect(step.error).to.be.true; - expect(step.errorMessage).to.include('pushing to a single branch'); - expect(step.logs[0]).to.include('Invalid number of branch updates'); + expect(step.errorMessage).to.include('push one ref at a time'); + expect(step.logs[0]).to.include('Invalid number of ref updates'); }); it('should add error step if multiple ref updates found', async () => { @@ -196,8 +196,8 @@ describe('parsePackFile', () => { const step = action.steps[0]; expect(step.stepName).to.equal('parsePackFile'); expect(step.error).to.be.true; - expect(step.errorMessage).to.include('pushing to a single branch'); - expect(step.logs[0]).to.include('Invalid number of branch updates'); + expect(step.errorMessage).to.include('push one ref at a time'); + expect(step.logs[0]).to.include('Invalid number of ref updates'); expect(step.logs[1]).to.include('Expected 1, but got 2'); }); @@ -227,12 +227,13 @@ describe('parsePackFile', () => { const ref = 'refs/heads/main'; const packetLine = `${oldCommit} ${newCommit} ${ref}\0capabilities\n`; - const commitContent = "tree 1234567890abcdef1234567890abcdef12345678\n" + - "parent abcdef1234567890abcdef1234567890abcdef12\n" + - "author Test Author 1234567890 +0000\n" + - "committer Test Committer 1234567890 +0000\n\n" + - "feat: Add new feature\n\n" + - "This is the commit body."; + const commitContent = + 'tree 1234567890abcdef1234567890abcdef12345678\n' + + 'parent abcdef1234567890abcdef1234567890abcdef12\n' + + 'author Test Author 1234567890 +0000\n' + + 'committer Test Committer 1234567890 +0000\n\n' + + 'feat: Add new feature\n\n' + + 'This is the commit body.'; const commitContentBuffer = Buffer.from(commitContent, 'utf8'); zlibInflateStub.returns(commitContentBuffer); @@ -245,7 +246,7 @@ describe('parsePackFile', () => { expect(result).to.equal(action); // Check step and action properties - const step = action.steps.find(s => s.stepName === 'parsePackFile'); + const step = action.steps.find((s) => s.stepName === 'parsePackFile'); expect(step).to.exist; expect(step.error).to.be.false; expect(step.errorMessage).to.be.null; @@ -257,7 +258,7 @@ describe('parsePackFile', () => { expect(action.user).to.equal('Test Committer'); // Check parsed commit data - const commitMessages = action.commitData.map(commit => commit.message); + const commitMessages = action.commitData.map((commit) => commit.message); expect(action.commitData).to.be.an('array').with.lengthOf(1); expect(commitMessages[0]).to.equal('feat: Add new feature\n\nThis is the commit body.'); @@ -284,10 +285,11 @@ describe('parsePackFile', () => { const packetLine = `${oldCommit} ${newCommit} ${ref}\0capabilities\n`; // Commit content without a parent line - const commitContent = "tree 1234567890abcdef1234567890abcdef12345678\n" + - "author Test Author 1234567890 +0000\n" + - "committer Test Committer 1234567890 +0100\n\n" + - "feat: Initial commit"; + const commitContent = + 'tree 1234567890abcdef1234567890abcdef12345678\n' + + 'author Test Author 1234567890 +0000\n' + + 'committer Test Committer 1234567890 +0100\n\n' + + 'feat: Initial commit'; const parentFromCommit = '0'.repeat(40); // Expected parent hash const commitContentBuffer = Buffer.from(commitContent, 'utf8'); @@ -299,7 +301,7 @@ describe('parsePackFile', () => { const result = await exec(req, action); expect(result).to.equal(action); - const step = action.steps.find(s => s.stepName === 'parsePackFile'); + const step = action.steps.find((s) => s.stepName === 'parsePackFile'); expect(step).to.exist; expect(step.error).to.be.false; @@ -323,11 +325,12 @@ describe('parsePackFile', () => { const parent1 = 'b1'.repeat(20); const parent2 = 'b2'.repeat(20); - const commitContent = "tree 1234567890abcdef1234567890abcdef12345678\n" + + const commitContent = + 'tree 1234567890abcdef1234567890abcdef12345678\n' + `parent ${parent1}\n` + `parent ${parent2}\n` + - "author Test Author 1234567890 +0000\n" + - "committer Test Committer 1234567890 +0100\n\n" + + 'author Test Author 1234567890 +0000\n' + + 'committer Test Committer 1234567890 +0100\n\n' + "Merge branch 'feature'"; const commitContentBuffer = Buffer.from(commitContent, 'utf8'); @@ -340,7 +343,7 @@ describe('parsePackFile', () => { expect(result).to.equal(action); // Check step and action properties - const step = action.steps.find(s => s.stepName === 'parsePackFile'); + const step = action.steps.find((s) => s.stepName === 'parsePackFile'); expect(step).to.exist; expect(step.error).to.be.false; @@ -360,10 +363,11 @@ describe('parsePackFile', () => { const packetLine = `${oldCommit} ${newCommit} ${ref}\0capabilities\n`; // Malformed commit content - missing tree line - const commitContent = "parent abcdef1234567890abcdef1234567890abcdef12\n" + - "author Test Author 1678886400 +0000\n" + - "committer Test Committer 1678886460 +0100\n\n" + - "feat: Missing tree"; + const commitContent = + 'parent abcdef1234567890abcdef1234567890abcdef12\n' + + 'author Test Author 1678886400 +0000\n' + + 'committer Test Committer 1678886460 +0100\n\n' + + 'feat: Missing tree'; const commitContentBuffer = Buffer.from(commitContent, 'utf8'); zlibInflateStub.returns(commitContentBuffer); @@ -373,7 +377,7 @@ describe('parsePackFile', () => { const result = await exec(req, action); expect(result).to.equal(action); - const step = action.steps.find(s => s.stepName === 'parsePackFile'); + const step = action.steps.find((s) => s.stepName === 'parsePackFile'); expect(step).to.exist; expect(step.error).to.be.true; expect(step.errorMessage).to.include('Invalid commit data: Missing tree'); @@ -384,7 +388,7 @@ describe('parsePackFile', () => { const newCommit = 'b'.repeat(40); const ref = 'refs/heads/main'; const packetLines = [`${oldCommit} ${newCommit} ${ref}\0capa\n`]; - + const packetLineBuffer = createPacketLineBuffer(packetLines); const garbageData = Buffer.from('NOT PACK DATA'); req.body = Buffer.concat([packetLineBuffer, garbageData]); @@ -411,11 +415,12 @@ describe('parsePackFile', () => { 'some other data containing PACK keyword', // Include "PACK" within a packet line's content ]; - const commitContent = "tree 1234567890abcdef1234567890abcdef12345678\n" + + const commitContent = + 'tree 1234567890abcdef1234567890abcdef12345678\n' + `parent ${oldCommit}\n` + - "author Test Author 1234567890 +0000\n" + - "committer Test Committer 1234567890 +0000\n\n" + - "Test commit message with PACK inside"; + 'author Test Author 1234567890 +0000\n' + + 'committer Test Committer 1234567890 +0000\n\n' + + 'Test commit message with PACK inside'; const samplePackBuffer = createSamplePackBuffer(1, commitContent, 1); zlibInflateStub.returns(Buffer.from(commitContent, 'utf8')); @@ -450,11 +455,12 @@ describe('parsePackFile', () => { const ref = 'refs/heads/master'; const packetLines = [`${oldCommit} ${newCommit} ${ref}\0`]; - const commitContent = "tree 1234567890abcdef1234567890abcdef12345678\n" + + const commitContent = + 'tree 1234567890abcdef1234567890abcdef12345678\n' + `parent ${oldCommit}\n` + - "author Test Author 1234567890 +0000\n" + - "committer Test Committer 1234567890 +0000\n\n" + - "Commit A"; + 'author Test Author 1234567890 +0000\n' + + 'committer Test Committer 1234567890 +0000\n\n' + + 'Commit A'; const samplePackBuffer = createSamplePackBuffer(1, commitContent, 1); zlibInflateStub.returns(Buffer.from(commitContent, 'utf8')); @@ -503,7 +509,7 @@ describe('parsePackFile', () => { expect(result).to.equal(action); - const step = action.steps.find(s => s.stepName === 'parsePackFile'); + const step = action.steps.find((s) => s.stepName === 'parsePackFile'); expect(step).to.exist; expect(step.error).to.be.false; expect(action.branch).to.equal(ref); @@ -520,7 +526,8 @@ describe('parsePackFile', () => { const ref = 'refs/tags/v1.0.0'; const packetLine = `${oldCommit} ${newCommit} ${ref}\0capabilities\n`; - const tagContent = 'object 1234567890abcdef1234567890abcdef12345678\ntype commit\ntag v1.0.0\ntagger Test Tagger 1234567890 +0000\n\nThis is a test tag message'; + const tagContent = + 'object 1234567890abcdef1234567890abcdef12345678\ntype commit\ntag v1.0.0\ntagger Test Tagger 1234567890 +0000\n\nThis is a test tag message'; const tagContentBuffer = Buffer.from(tagContent, 'utf8'); zlibInflateStub.returns(tagContentBuffer); @@ -532,7 +539,7 @@ describe('parsePackFile', () => { expect(result).to.equal(action); // Check step and action properties - const step = action.steps.find(s => s.stepName === 'parsePackFile'); + const step = action.steps.find((s) => s.stepName === 'parsePackFile'); expect(step).to.exist; expect(step.error).to.be.false; expect(step.errorMessage).to.be.null; @@ -552,7 +559,7 @@ describe('parsePackFile', () => { expect(parsedTag.tagName).to.equal('v1.0.0'); expect(parsedTag.tagger).to.equal('Test Tagger'); expect(parsedTag.message).to.equal('This is a test tag message'); - + expect(action.user).to.equal('Test Tagger'); }); @@ -563,7 +570,8 @@ describe('parsePackFile', () => { const packetLine = `${oldCommit} ${newCommit} ${ref}\0capabilities\n`; // Tag content without tagger line - const malformedTagContent = 'object 1234567890abcdef1234567890abcdef12345678\ntype commit\ntag v1.0.0\n\nTag without tagger'; + const malformedTagContent = + 'object 1234567890abcdef1234567890abcdef12345678\ntype commit\ntag v1.0.0\n\nTag without tagger'; const tagContentBuffer = Buffer.from(malformedTagContent, 'utf8'); zlibInflateStub.returns(tagContentBuffer); @@ -580,7 +588,7 @@ describe('parsePackFile', () => { expect(action.actionType).to.equal('tag'); // Should have error due to parsing failure - const step = action.steps.find(s => s.stepName === 'parsePackFile'); + const step = action.steps.find((s) => s.stepName === 'parsePackFile'); expect(step).to.exist; expect(step.error).to.be.true; expect(step.errorMessage).to.include('Invalid tag object: no tagger line'); @@ -593,7 +601,8 @@ describe('parsePackFile', () => { const packetLine = `${oldCommit} ${newCommit} ${ref}\0capabilities\n`; // Tag content missing object field - const incompleteTagContent = 'type commit\ntag v2.0.0\ntagger Test Tagger 1234567890 +0000\n\nIncomplete tag'; + const incompleteTagContent = + 'type commit\ntag v2.0.0\ntagger Test Tagger 1234567890 +0000\n\nIncomplete tag'; const tagContentBuffer = Buffer.from(incompleteTagContent, 'utf8'); zlibInflateStub.returns(tagContentBuffer); @@ -606,9 +615,9 @@ describe('parsePackFile', () => { expect(action.tag).to.equal(ref); expect(action.actionType).to.equal('tag'); - + // Should have error due to parsing failure - const step = action.steps.find(s => s.stepName === 'parsePackFile'); + const step = action.steps.find((s) => s.stepName === 'parsePackFile'); expect(step).to.exist; expect(step.error).to.be.true; expect(step.errorMessage).to.include('Invalid tag object'); @@ -620,7 +629,8 @@ describe('parsePackFile', () => { const ref = 'refs/tags/v3.0.0-beta1'; const packetLine = `${oldCommit} ${newCommit} ${ref}\0capabilities\n`; - const complexMessage = 'Release v3.0.0-beta1\n\nThis is a major release with:\n- Feature A\n- Feature B\n\nBreaking changes:\n- API change in module X'; + const complexMessage = + 'Release v3.0.0-beta1\n\nThis is a major release with:\n- Feature A\n- Feature B\n\nBreaking changes:\n- API change in module X'; const tagContent = `object 1234567890abcdef1234567890abcdef12345678\ntype commit\ntag v3.0.0-beta1\ntagger Release Bot 1678886400 +0000\n\n${complexMessage}`; const tagContentBuffer = Buffer.from(tagContent, 'utf8'); @@ -632,12 +642,12 @@ describe('parsePackFile', () => { const result = await exec(req, action); expect(result).to.equal(action); - const step = action.steps.find(s => s.stepName === 'parsePackFile'); + const step = action.steps.find((s) => s.stepName === 'parsePackFile'); expect(step.error).to.be.false; expect(action.tag).to.equal(ref); expect(action.tagData).to.be.an('array').with.lengthOf(1); - + const parsedTag = action.tagData[0]; expect(parsedTag.tagName).to.equal('v3.0.0-beta1'); expect(parsedTag.tagger).to.equal('Release Bot'); @@ -662,13 +672,13 @@ describe('parsePackFile', () => { it('should handle buffer exactly 12 bytes long', () => { const buffer = createSamplePackBuffer(1).slice(0, 12); // Only header - const [meta, contentBuff] = getPackMeta(buffer); + const [meta, contentBuff] = getPackMeta(buffer); - expect(meta).to.deep.equal({ - sig: PACK_SIGNATURE, - version: 2, - entries: 1, - }); + expect(meta).to.deep.equal({ + sig: PACK_SIGNATURE, + version: 2, + entries: 1, + }); expect(contentBuff.length).to.equal(0); // No content left }); }); @@ -714,7 +724,10 @@ describe('parsePackFile', () => { describe('getCommitData', () => { it('should return empty array if no type 1 contents', () => { - const contents = [{ type: 2, content: 'blob' }, { type: 3, content: 'tree' }]; + const contents = [ + { type: 2, content: 'blob' }, + { type: 3, content: 'tree' }, + ]; expect(getCommitData(contents)).to.deep.equal([]); }); @@ -810,37 +823,41 @@ describe('parsePackFile', () => { }); it('should correctly parse a commit with a GPG signature header', () => { - const gpgSignedCommit = "tree b4d3c0ffee1234567890abcdef1234567890aabbcc\n" + - "parent 01dbeef9876543210fedcba9876543210fedcba\n" + - "author Test Author 1744814600 +0100\n" + - "committer Test Committer 1744814610 +0200\n" + - "gpgsig -----BEGIN PGP SIGNATURE-----\n \n" + - " wsFcBAABCAAQBQJn/8ISCRC1aQ7uu5UhlAAAntAQACeyQd6IykNXiN6m9DfVp8DJ\n" + - " UsY64ws+Td0inrEee+cHXVI9uJn15RJYQkICwlM4TZsVGav7nYaVqO+gfAg2ORAH\n" + - " ghUnwSFFs7ucN/p0a47ItkJmt04+jQIFlZIC+wy1u2H3aKJwqaF+kGP5SA33ahgV\n" + - " ZWviKodXFki8/G+sKB63q1qrDw6aELtftEgeAPQUcuLzj+vu/m3dWrDbatfUXMkC\n" + - " JC6PbFajqrJ5pEtFwBqqRE+oIsOM9gkNAti1yDD5eoS+bNXACe0hT0+UoIzn5a34\n" + - " xcElXTSdAK/MRjGiLN91G2nWvlbpM5wAEqr5Bl5ealCc6BbWfPxbP46slaE5DfkD\n" + - " u0+RkVX06MSSPqzOmEV14ZWKap5C19FpF9o/rY8vtLlCxjWMhtUvvdR4OQfQpEDY\n" + - " eTqzCHRnM3+7r3ABAWt9v7cG99bIMEs3sGcMy11HMeaoBpye6vCIP4ghNnoB1hUJ\n" + - " D7MD77jzk4Kbf4IzS5omExyMu3AiNZecZX4+1w/527yPhv3s/HB1Gfz0oCUned+6\n" + - " b9Kkle+krsQ/EK/4gPcb/Kb1cTcm3HhjaOSYwA+JpApJQ0mrduH34AT5MZJuIPFe\n" + - " QheLzQI1d2jmFs11GRC5hc0HBk1WmGm6U8+FBuxCX0ECZPdYeQJjUeWjnNeUoE6a\n" + - " 5lytZU4Onk57nUhIMSrx\n" + - " =IxZr\n" + - " -----END PGP SIGNATURE-----\n\n" + - "This is the commit message.\n" + - "It can span multiple lines.\n\n" + - "And include blank lines internally."; + const gpgSignedCommit = + 'tree b4d3c0ffee1234567890abcdef1234567890aabbcc\n' + + 'parent 01dbeef9876543210fedcba9876543210fedcba\n' + + 'author Test Author 1744814600 +0100\n' + + 'committer Test Committer 1744814610 +0200\n' + + 'gpgsig -----BEGIN PGP SIGNATURE-----\n \n' + + ' wsFcBAABCAAQBQJn/8ISCRC1aQ7uu5UhlAAAntAQACeyQd6IykNXiN6m9DfVp8DJ\n' + + ' UsY64ws+Td0inrEee+cHXVI9uJn15RJYQkICwlM4TZsVGav7nYaVqO+gfAg2ORAH\n' + + ' ghUnwSFFs7ucN/p0a47ItkJmt04+jQIFlZIC+wy1u2H3aKJwqaF+kGP5SA33ahgV\n' + + ' ZWviKodXFki8/G+sKB63q1qrDw6aELtftEgeAPQUcuLzj+vu/m3dWrDbatfUXMkC\n' + + ' JC6PbFajqrJ5pEtFwBqqRE+oIsOM9gkNAti1yDD5eoS+bNXACe0hT0+UoIzn5a34\n' + + ' xcElXTSdAK/MRjGiLN91G2nWvlbpM5wAEqr5Bl5ealCc6BbWfPxbP46slaE5DfkD\n' + + ' u0+RkVX06MSSPqzOmEV14ZWKap5C19FpF9o/rY8vtLlCxjWMhtUvvdR4OQfQpEDY\n' + + ' eTqzCHRnM3+7r3ABAWt9v7cG99bIMEs3sGcMy11HMeaoBpye6vCIP4ghNnoB1hUJ\n' + + ' D7MD77jzk4Kbf4IzS5omExyMu3AiNZecZX4+1w/527yPhv3s/HB1Gfz0oCUned+6\n' + + ' b9Kkle+krsQ/EK/4gPcb/Kb1cTcm3HhjaOSYwA+JpApJQ0mrduH34AT5MZJuIPFe\n' + + ' QheLzQI1d2jmFs11GRC5hc0HBk1WmGm6U8+FBuxCX0ECZPdYeQJjUeWjnNeUoE6a\n' + + ' 5lytZU4Onk57nUhIMSrx\n' + + ' =IxZr\n' + + ' -----END PGP SIGNATURE-----\n\n' + + 'This is the commit message.\n' + + 'It can span multiple lines.\n\n' + + 'And include blank lines internally.'; const contents = [ { type: 1, content: gpgSignedCommit }, - { type: 1, content: `tree 111\nparent 000\nauthor A1 1744814600 +0200\ncommitter C1 1744814610 +0200\n\nMsg1` } + { + type: 1, + content: `tree 111\nparent 000\nauthor A1 1744814600 +0200\ncommitter C1 1744814610 +0200\n\nMsg1`, + }, ]; const result = getCommitData(contents); expect(result).to.be.an('array').with.lengthOf(2); - + // Check the GPG signed commit data const gpgResult = result[0]; expect(gpgResult.tree).to.equal('b4d3c0ffee1234567890abcdef1234567890aabbcc'); @@ -849,8 +866,10 @@ describe('parsePackFile', () => { expect(gpgResult.committer).to.equal('Test Committer'); expect(gpgResult.authorEmail).to.equal('test.author@example.com'); expect(gpgResult.commitTimestamp).to.equal('1744814610'); - expect(gpgResult.message).to.equal(`This is the commit message.\nIt can span multiple lines.\n\nAnd include blank lines internally.`); - + expect(gpgResult.message).to.equal( + `This is the commit message.\nIt can span multiple lines.\n\nAnd include blank lines internally.`, + ); + // Sanity check: the second commit should be the simple commit const simpleResult = result[1]; expect(simpleResult.message).to.equal('Msg1'); @@ -864,27 +883,31 @@ describe('parsePackFile', () => { describe('parseTag', () => { it('should parse a valid tag object correctly', () => { - const tagContent = 'object 1234567890abcdef1234567890abcdef12345678\ntype commit\ntag v1.0.0\ntagger John Doe 1678886400 +0000\n\nFirst stable release'; + const tagContent = + 'object 1234567890abcdef1234567890abcdef12345678\ntype commit\ntag v1.0.0\ntagger John Doe 1678886400 +0000\n\nFirst stable release'; const tagObject = { content: tagContent }; - + const result = parseTag(tagObject); - + expect(result).to.deep.equal({ object: '1234567890abcdef1234567890abcdef12345678', type: 'commit', tagName: 'v1.0.0', tagger: 'John Doe', + taggerEmail: 'john@example.com', + timestamp: '1678886400', message: 'First stable release', }); }); it('should parse tag with multi-line message correctly', () => { - const complexMessage = 'Release v2.0.0\n\nMajor release with:\n- Feature A\n- Feature B\n\nBreaking changes included.'; + const complexMessage = + 'Release v2.0.0\n\nMajor release with:\n- Feature A\n- Feature B\n\nBreaking changes included.'; const tagContent = `object abcdef1234567890abcdef1234567890abcdef12\ntype commit\ntag v2.0.0\ntagger Release Bot 1678972800 +0000\n\n${complexMessage}`; const tagObject = { content: tagContent }; - + const result = parseTag(tagObject); - + expect(result.object).to.equal('abcdef1234567890abcdef1234567890abcdef12'); expect(result.type).to.equal('commit'); expect(result.tagName).to.equal('v2.0.0'); @@ -893,61 +916,68 @@ describe('parsePackFile', () => { }); it('should handle tag with empty message', () => { - const tagContent = 'object 1234567890abcdef1234567890abcdef12345678\ntype commit\ntag v1.0.0\ntagger Jane Doe 1678886400 +0000\n\n'; + const tagContent = + 'object 1234567890abcdef1234567890abcdef12345678\ntype commit\ntag v1.0.0\ntagger Jane Doe 1678886400 +0000\n\n'; const tagObject = { content: tagContent }; - + const result = parseTag(tagObject); - + expect(result.message).to.equal(''); expect(result.tagName).to.equal('v1.0.0'); expect(result.tagger).to.equal('Jane Doe'); }); it('should throw error when tagger line is missing', () => { - const tagContent = 'object 1234567890abcdef1234567890abcdef12345678\ntype commit\ntag v1.0.0\n\nTag without tagger'; + const tagContent = + 'object 1234567890abcdef1234567890abcdef12345678\ntype commit\ntag v1.0.0\n\nTag without tagger'; const tagObject = { content: tagContent }; - + expect(() => parseTag(tagObject)).to.throw('Invalid tag object: no tagger line'); }); it('should throw error when object line is missing', () => { - const tagContent = 'type commit\ntag v1.0.0\ntagger John Doe 1678886400 +0000\n\nTag without object'; + const tagContent = + 'type commit\ntag v1.0.0\ntagger John Doe 1678886400 +0000\n\nTag without object'; const tagObject = { content: tagContent }; - + expect(() => parseTag(tagObject)).to.throw('Invalid tag object'); }); it('should throw error when type line is missing', () => { - const tagContent = 'object 1234567890abcdef1234567890abcdef12345678\ntag v1.0.0\ntagger John Doe 1678886400 +0000\n\nTag without type'; + const tagContent = + 'object 1234567890abcdef1234567890abcdef12345678\ntag v1.0.0\ntagger John Doe 1678886400 +0000\n\nTag without type'; const tagObject = { content: tagContent }; - + expect(() => parseTag(tagObject)).to.throw('Invalid tag object'); }); it('should throw error when tag name is missing', () => { - const tagContent = 'object 1234567890abcdef1234567890abcdef12345678\ntype commit\ntagger John Doe 1678886400 +0000\n\nTag without name'; + const tagContent = + 'object 1234567890abcdef1234567890abcdef12345678\ntype commit\ntagger John Doe 1678886400 +0000\n\nTag without name'; const tagObject = { content: tagContent }; - + expect(() => parseTag(tagObject)).to.throw('Invalid tag object'); }); it('should handle tagger with complex email format', () => { - const tagContent = 'object 1234567890abcdef1234567890abcdef12345678\ntype commit\ntag v1.0.0\ntagger John Doe (Developer) 1678886400 +0100\n\nTag with complex tagger'; + const tagContent = + 'object 1234567890abcdef1234567890abcdef12345678\ntype commit\ntag v1.0.0\ntagger John Doe (Developer) 1678886400 +0100\n\nTag with complex tagger'; const tagObject = { content: tagContent }; - + const result = parseTag(tagObject); - + expect(result.tagger).to.equal('John Doe (Developer)'); expect(result.tagName).to.equal('v1.0.0'); expect(result.message).to.equal('Tag with complex tagger'); }); it('should handle tag pointing to different object types', () => { - const tagContent = 'object 1234567890abcdef1234567890abcdef12345678\ntype tree\ntag tree-tag\ntagger Tree Tagger 1678886400 +0000\n\nTag pointing to tree object'; + const tagContent = + 'object 1234567890abcdef1234567890abcdef12345678\ntype tree\ntag tree-tag\ntagger Tree Tagger 1678886400 +0000\n\nTag pointing to tree object'; const tagObject = { content: tagContent }; - + const result = parseTag(tagObject); - + expect(result.type).to.equal('tree'); expect(result.tagName).to.equal('tree-tag'); expect(result.tagger).to.equal('Tree Tagger'); @@ -956,11 +986,7 @@ describe('parsePackFile', () => { describe('parsePacketLines', () => { it('should parse multiple valid packet lines correctly and return the correct offset', () => { - const lines = [ - 'line1 content', - 'line2 more content\nwith newline', - 'line3', - ]; + const lines = ['line1 content', 'line2 more content\nwith newline', 'line3']; const buffer = createPacketLineBuffer(lines); // Helper adds "0000" at the end const expectedOffset = buffer.length; // Should indicate the end of the buffer after flush packet const [parsedLines, offset] = parsePacketLines(buffer); @@ -1004,7 +1030,7 @@ describe('parsePackFile', () => { buffer = Buffer.concat([buffer, extraData]); const expectedOffset = buffer.length - extraData.length; - const [parsedLines, offset] = parsePacketLines(buffer); + const [parsedLines, offset] = parsePacketLines(buffer); expect(parsedLines).to.deep.equal(lines); expect(offset).to.equal(expectedOffset); @@ -1013,7 +1039,9 @@ describe('parsePackFile', () => { it('should throw an error if a packet line length exceeds buffer bounds', () => { // 000A -> length 10, but actual line length is only 3 bytes const invalidLengthBuffer = Buffer.from('000Aabc'); - expect(() => parsePacketLines(invalidLengthBuffer)).to.throw(/Invalid packet line length 000A/); + expect(() => parsePacketLines(invalidLengthBuffer)).to.throw( + /Invalid packet line length 000A/, + ); }); it('should throw an error for non-hex length prefix (all non-hex)', () => { @@ -1027,7 +1055,7 @@ describe('parsePackFile', () => { expect(() => parsePacketLines(invalidHexBuffer)).to.throw(/Invalid packet line length 000z/); }); - it('should handle buffer ending exactly after a valid line length without content', () => { + it('should handle buffer ending exactly after a valid line length without content', () => { // 0008 -> length 8, but buffer ends after header (no content) const incompleteBuffer = Buffer.from('0008'); expect(() => parsePacketLines(incompleteBuffer)).to.throw(/Invalid packet line length 0008/); From b969eba8f502f06d8c2fc46b473ed0be1b02507d Mon Sep 17 00:00:00 2001 From: fabiovincenzi Date: Wed, 20 Aug 2025 16:45:28 +0200 Subject: [PATCH 17/21] fix: show diff section only for commits, not tags --- src/ui/views/PushDetails/PushDetails.tsx | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/ui/views/PushDetails/PushDetails.tsx b/src/ui/views/PushDetails/PushDetails.tsx index 501814101..1fc220d51 100644 --- a/src/ui/views/PushDetails/PushDetails.tsx +++ b/src/ui/views/PushDetails/PushDetails.tsx @@ -398,14 +398,16 @@ const Dashboard: React.FC = () => { )} - {/* Diff section - show for both commits and tags */} - - - - - - - + {/* Diff section - show only for commits */} + {!isTag && data.diff?.content && ( + + + + + + + + )} ); From c1d9636c272975f9f632ed9c6227784e9d6a17e5 Mon Sep 17 00:00:00 2001 From: fabiovincenzi Date: Fri, 22 Aug 2025 11:05:35 +0200 Subject: [PATCH 18/21] fix: convert getUserProfileLink to return JSX instead of HTML strings --- src/ui/utils.tsx | 17 ++++++++++------- .../OpenPushRequests/components/PushesTable.tsx | 4 ++-- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/src/ui/utils.tsx b/src/ui/utils.tsx index ab6413c41..e4889a798 100644 --- a/src/ui/utils.tsx +++ b/src/ui/utils.tsx @@ -1,3 +1,4 @@ +import React from 'react'; import axios from 'axios'; import { SCMRepositoryMetadata, @@ -55,24 +56,26 @@ export const getUserProfileUrl = (username: string, provider: string, hostname: }; /** - * Attempts to construct a link to the user's profile at an SCM provider. + * Attempts to construct a JSX link to the user's profile at an SCM provider. * @param {string} username The username. * @param {string} provider The name of the SCM provider. * @param {string} hostname The hostname of the SCM provider. - * @return {string} A string containing an HTML A tag pointing to the user's profile, if possible, degrading to just the username or 'N/A' when not (e.g. because the SCM provider is unknown). + * @return {JSX.Element} A JSX element containing a link to the user's profile, if possible, degrading to just the username or 'N/A' when not (e.g. because the SCM provider is unknown). */ export const getUserProfileLink = (username: string, provider: string, hostname: string) => { if (username) { - let profileData = ''; const profileUrl = getUserProfileUrl(username, provider, hostname); if (profileUrl) { - profileData = `${username}`; + return ( + + {username} + + ); } else { - profileData = `${username}`; + return {username}; } - return profileData; } else { - return 'N/A'; + return N/A; } }; diff --git a/src/ui/views/OpenPushRequests/components/PushesTable.tsx b/src/ui/views/OpenPushRequests/components/PushesTable.tsx index 89d82a4b7..82f13d4e2 100644 --- a/src/ui/views/OpenPushRequests/components/PushesTable.tsx +++ b/src/ui/views/OpenPushRequests/components/PushesTable.tsx @@ -170,14 +170,14 @@ const PushesTable: React.FC = (props) => { {isValidValue(committerOrTagger) ? ( - + getUserProfileLink(committerOrTagger, gitProvider, hostname) ) : ( 'N/A' )} {isValidValue(author) ? ( - + getUserProfileLink(author, gitProvider, hostname) ) : ( 'N/A' )} From d0f847d0aeedf679fc65cdfe6778c74319c6f60b Mon Sep 17 00:00:00 2001 From: fabiovincenzi Date: Fri, 22 Aug 2025 12:30:53 +0200 Subject: [PATCH 19/21] chore: restore proxy.config.json --- proxy.config.json | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/proxy.config.json b/proxy.config.json index e210074eb..bdaedff4f 100644 --- a/proxy.config.json +++ b/proxy.config.json @@ -14,16 +14,6 @@ "project": "finos", "name": "git-proxy", "url": "https://github.com/finos/git-proxy.git" - }, - { - "project": "fabiovincenzi", - "name": "test", - "url": "https://github.com/fabiovincenzi/test.git" - }, - { - "project": "fabio.vincenzi.reviewit-group", - "name": "test", - "url": "https://gitlab.com/fabio.vincenzi.reviewit-group/test.git" } ], "sink": [ @@ -80,9 +70,6 @@ "api": { "github": { "baseUrl": "https://api.github.com" - }, - "gitlab": { - "baseUrl": "https://gitlab.com/api/v4" } }, "commitConfig": { From d54303279bfe8cb9cecd86ef57cbc0d05e2bad37 Mon Sep 17 00:00:00 2001 From: fabiovincenzi Date: Wed, 27 Aug 2025 14:49:30 +0200 Subject: [PATCH 20/21] fix: remove duplicate parsePush execution in branch chain --- src/proxy/chain.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/proxy/chain.ts b/src/proxy/chain.ts index 04eae3ebf..ef999a457 100644 --- a/src/proxy/chain.ts +++ b/src/proxy/chain.ts @@ -4,7 +4,6 @@ import * as proc from './processors'; import { attemptAutoApproval, attemptAutoRejection } from './actions/autoActions'; const branchPushChain: ((req: any, action: Action) => Promise)[] = [ - proc.push.parsePush, proc.push.checkEmptyBranch, proc.push.checkRepoInAuthorisedList, proc.push.checkCommitMessages, From 279b2fb54b4ad13cf51f3836402c79f068319a63 Mon Sep 17 00:00:00 2001 From: fabiovincenzi Date: Wed, 27 Aug 2025 14:53:15 +0200 Subject: [PATCH 21/21] chore: format code with npm run format --- cypress/e2e/tagPush.cy.js | 46 ++++++++++--------- src/proxy/actions/Action.ts | 4 +- .../ErrorBoundary/ErrorBoundary.tsx | 22 +++++---- src/ui/utils/pushUtils.ts | 18 ++++++-- .../components/PushesTable.tsx | 23 +++++----- src/ui/views/PushDetails/PushDetails.tsx | 16 ++----- 6 files changed, 68 insertions(+), 61 deletions(-) diff --git a/cypress/e2e/tagPush.cy.js b/cypress/e2e/tagPush.cy.js index 62b60d61a..40a350b35 100644 --- a/cypress/e2e/tagPush.cy.js +++ b/cypress/e2e/tagPush.cy.js @@ -7,34 +7,38 @@ describe('Tag Push Functionality', () => { describe('Tag Push Display in PushesTable', () => { it('can navigate to repo dashboard and view push table', () => { cy.visit('/dashboard/repo'); - + // Check that we can see the basic table structure cy.get('table').should('exist'); cy.get('tbody tr').should('have.length.at.least', 1); - + // Look for any push entries in the table - cy.get('tbody tr').first().within(() => { - // Check that basic cells exist - adjust expectation to actual data (2 cells) - cy.get('td').should('have.length.at.least', 2); - }); + cy.get('tbody tr') + .first() + .within(() => { + // Check that basic cells exist - adjust expectation to actual data (2 cells) + cy.get('td').should('have.length.at.least', 2); + }); }); it('has search functionality', () => { cy.visit('/dashboard/repo'); - + // Look for search input - it might have different selector cy.get('input[type="text"]').first().should('exist'); }); it('can interact with push table entries', () => { cy.visit('/dashboard/repo'); - + // Try to find clickable links within table rows instead of clicking the row - cy.get('tbody tr').first().within(() => { - // Look for any clickable elements (links, buttons) - cy.get('a, button, [role="button"]').should('have.length.at.least', 0); - }); - + cy.get('tbody tr') + .first() + .within(() => { + // Look for any clickable elements (links, buttons) + cy.get('a, button, [role="button"]').should('have.length.at.least', 0); + }); + // Just verify we can navigate to a push details page directly cy.visit('/dashboard/push/123', { failOnStatusCode: false }); cy.get('body').should('exist'); // Should not crash @@ -45,10 +49,10 @@ describe('Tag Push Functionality', () => { it('can access push details page structure', () => { // Try to access a push details page directly cy.visit('/dashboard/push/test-push-id', { failOnStatusCode: false }); - + // Check basic page structure exists (regardless of whether push exists) cy.get('body').should('exist'); // Basic content check - + // If we end up redirected, that's also acceptable behavior cy.url().should('include', '/dashboard'); }); @@ -59,7 +63,7 @@ describe('Tag Push Functionality', () => { // Test navigation to repo dashboard cy.visit('/dashboard/repo'); cy.get('table').should('exist'); - + // Test navigation to user management if it exists cy.visit('/dashboard/user'); cy.get('body').should('exist'); @@ -70,7 +74,7 @@ describe('Tag Push Functionality', () => { it('handles navigation to non-existent push gracefully', () => { // Try to visit a non-existent push detail page cy.visit('/dashboard/push/non-existent-push-id', { failOnStatusCode: false }); - + // Should either redirect or show error page, but not crash cy.get('body').should('exist'); }); @@ -78,13 +82,13 @@ describe('Tag Push Functionality', () => { it('maintains functionality after page refresh', () => { cy.visit('/dashboard/repo'); cy.get('table').should('exist'); - + // Refresh the page cy.reload(); - + // Wait for page to reload and check basic functionality cy.get('body').should('exist'); - + // Give more time for table to load after refresh, or check if redirected cy.url().then((url) => { if (url.includes('/dashboard/repo')) { @@ -96,4 +100,4 @@ describe('Tag Push Functionality', () => { }); }); }); -}); \ No newline at end of file +}); diff --git a/src/proxy/actions/Action.ts b/src/proxy/actions/Action.ts index 5a0e8ac4a..c48f8568b 100644 --- a/src/proxy/actions/Action.ts +++ b/src/proxy/actions/Action.ts @@ -6,7 +6,7 @@ export enum RequestType { // eslint-disable-next-line no-unused-vars PUSH = 'push', // eslint-disable-next-line no-unused-vars - PULL = 'pull' + PULL = 'pull', } export enum ActionType { @@ -15,7 +15,7 @@ export enum ActionType { // eslint-disable-next-line no-unused-vars TAG = 'tag', // eslint-disable-next-line no-unused-vars - BRANCH = 'branch' + BRANCH = 'branch', } /** diff --git a/src/ui/components/ErrorBoundary/ErrorBoundary.tsx b/src/ui/components/ErrorBoundary/ErrorBoundary.tsx index 548d055df..25e3b628b 100644 --- a/src/ui/components/ErrorBoundary/ErrorBoundary.tsx +++ b/src/ui/components/ErrorBoundary/ErrorBoundary.tsx @@ -33,18 +33,20 @@ class ErrorBoundary extends Component { {process.env.NODE_ENV === 'development' && this.state.error && (
Error details (development only) - 
+                
                   {this.state.error.stack}
)} -