Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 57 additions & 3 deletions __tests__/utils/ListPRs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,12 @@ describe('ListPRs Utility', () => {
});

it('returns PRs with valid matching file and tenant', async () => {
const pr = { number: 1, head: { sha: 'abc123' } };
const pr = {
number: 1,
title: 'collection Ingest Request for test',
user: { login: 'ingest-bot[bot]', type: 'Bot' },
head: { sha: 'abc123', ref: 'feat/test' },
};
mockList.mockResolvedValue({ data: [pr] });
mockListFiles.mockResolvedValue({
data: [
Expand All @@ -89,7 +94,12 @@ describe('ListPRs Utility', () => {
});

it('returns PRs with tenant undefined if JSON parse fails', async () => {
const pr = { number: 2, head: { sha: 'def456' } };
const pr = {
number: 2,
title: 'collection Ingest Request for bad',
user: { login: 'ingest-bot[bot]', type: 'Bot' },
head: { sha: 'def456', ref: 'feat/bad' },
};
mockList.mockResolvedValue({ data: [pr] });
mockListFiles.mockResolvedValue({
data: [{ filename: 'ingestion-data/staging/collections/bad.json' }],
Expand All @@ -106,7 +116,12 @@ describe('ListPRs Utility', () => {
});

it('filters out PRs without matching files', async () => {
const pr = { number: 3, head: { sha: 'ghi789' } };
const pr = {
number: 3,
title: 'collection Ingest Request for nope',
user: { login: 'ingest-bot[bot]', type: 'Bot' },
head: { sha: 'ghi789', ref: 'feat/nope' },
};
mockList.mockResolvedValue({ data: [pr] });
mockListFiles.mockResolvedValue({
data: [{ filename: 'not-a-match.txt' }],
Expand All @@ -116,6 +131,45 @@ describe('ListPRs Utility', () => {
expect(result).toEqual([]);
});

it('filters out manual PRs that do not match app naming conventions', async () => {
const manualPr = {
number: 4,
title: 'Update GEDI WMTS collection',
user: { login: 'human-user', type: 'User' },
head: { sha: 'jkl012', ref: 'collection/GEDI' },
};

mockList.mockResolvedValue({ data: [manualPr] });

const result = await ListPRs('collection');
expect(result).toEqual([]);
expect(mockListFiles).not.toHaveBeenCalled();
});

it('includes user-authored PRs when branch/title follow UI naming conventions', async () => {
const userPrWithConventions = {
number: 5,
title: 'collection Ingest Request for GEDI',
user: { login: 'human-user', type: 'User' },
head: { sha: 'mno345', ref: 'feat/GEDI' },
};

mockList.mockResolvedValue({ data: [userPrWithConventions] });
mockListFiles.mockResolvedValue({
data: [{ filename: 'ingestion-data/staging/collections/GEDI.json' }],
});
const fileContent = Buffer.from(
JSON.stringify({ 'local:tenant': 'tenant2' })
).toString('base64');
mockGetContent.mockResolvedValue({
data: { content: fileContent },
});

const result = await ListPRs('collection');
expect(result).toHaveLength(1);
expect(result[0].pr).toEqual(userPrWithConventions);
});

it('throws and logs error if octokit fails', async () => {
mockList.mockRejectedValue(new Error('API fail'));
const spy = vi.spyOn(console, 'error').mockImplementation(() => {});
Expand Down
7 changes: 6 additions & 1 deletion __tests__/utils/ListPRsFallback.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,12 @@ describe('ListPRs fallback tenant behavior', () => {
});

it('falls back to eic:tenant when VEDA_TENANT_FILTER_FIELD is unset and ignores other tenant-like keys', async () => {
const pr = { number: 1, head: { sha: 'abc123' } };
const pr = {
number: 1,
title: 'collection Ingest Request for test',
user: { login: 'ingest-bot[bot]', type: 'Bot' },
head: { sha: 'abc123', ref: 'feat/test' },
};
mockList.mockResolvedValue({ data: [pr] });
mockListFiles.mockResolvedValue({
data: [{ filename: 'ingestion-data/staging/collections/test.json' }],
Expand Down
54 changes: 54 additions & 0 deletions __tests__/utils/RetrieveJSON.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,60 @@ describe('RetrieveJSON', () => {
content: { id: 'test-collection' },
});
});

it('derives collection filename from collection/* ref', async () => {
const mockRef = 'collection/GEDI';
const mockFilePath = 'ingestion-data/staging/collections/GEDI.json';
const mockContentBase64 = Buffer.from(
JSON.stringify({ id: 'GEDI' })
).toString('base64');

mockGetContent.mockResolvedValue({
data: {
sha: 'mockShaCollectionRef',
content: mockContentBase64,
path: mockFilePath,
},
});

const result = await RetrieveJSON(mockRef, 'collection');

expect(mockGetContent).toHaveBeenCalledWith(
expect.objectContaining({ path: mockFilePath, ref: mockRef })
);
expect(result).toEqual({
fileSha: 'mockShaCollectionRef',
filePath: mockFilePath,
content: { id: 'GEDI' },
});
});

it('derives collection filename from refs/heads/collection/* ref', async () => {
const mockRef = 'refs/heads/collection/GEDI';
const mockFilePath = 'ingestion-data/staging/collections/GEDI.json';
const mockContentBase64 = Buffer.from(
JSON.stringify({ id: 'GEDI' })
).toString('base64');

mockGetContent.mockResolvedValue({
data: {
sha: 'mockShaRefsHeadsCollectionRef',
content: mockContentBase64,
path: mockFilePath,
},
});

const result = await RetrieveJSON(mockRef, 'collection');

expect(mockGetContent).toHaveBeenCalledWith(
expect.objectContaining({ path: mockFilePath, ref: mockRef })
);
expect(result).toEqual({
fileSha: 'mockShaRefsHeadsCollectionRef',
filePath: mockFilePath,
content: { id: 'GEDI' },
});
});
});

describe('Error Handling', () => {
Expand Down
49 changes: 48 additions & 1 deletion utils/githubUtils/ListPRs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,49 @@ const TARGET_PATHS = {

type IngestionType = 'collection' | 'dataset';

const isGitHubAppAuthored = (
login: string | undefined,
type: string | undefined
): boolean => {
if (!login || !type) {
return false;
}

return type === 'Bot' || login.endsWith('[bot]');
};

const matchesUiIngestNamingConvention = (
title: string | undefined,
headRef: string | undefined,
ingestionType: IngestionType
): boolean => {
if (!title || !headRef) {
return false;
}

const expectedTitlePrefix = `${ingestionType} Ingest Request for `;
return title.startsWith(expectedTitlePrefix) && headRef.startsWith('feat/');
};

const shouldIncludePullRequest = (
pr: {
title?: string;
user?: { login?: string; type?: string } | null;
head?: { ref?: string };
},
ingestionType: IngestionType
): boolean => {
const login = pr.user?.login;
const userType = pr.user?.type;
const headRef = pr.head?.ref;

if (isGitHubAppAuthored(login, userType)) {
return true;
}

return matchesUiIngestNamingConvention(pr.title, headRef, ingestionType);
};

const extractTenantFromContent = (
parsedContent: Record<string, unknown>,
tenantFieldKey: string
Expand Down Expand Up @@ -46,8 +89,12 @@ const ListPRs = async (
state: 'open',
});

const filteredPullRequests = pullRequests.filter((pr) =>
shouldIncludePullRequest(pr, ingestionType)
);

// 2. Create an array of promises to check the files for each pull request.
const checkFilePromises = pullRequests.map(async (pr) => {
const checkFilePromises = filteredPullRequests.map(async (pr) => {
const { data: files } = await octokit.rest.pulls.listFiles({
owner,
repo,
Expand Down
34 changes: 32 additions & 2 deletions utils/githubUtils/RetrieveJSON.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,36 @@ import GetGithubToken from '@/utils/githubUtils/GetGithubToken';

type IngestionType = 'collection' | 'dataset';

const normalizeRefToFileStem = (
ref: string,
ingestionType: IngestionType
): string => {
let stem = ref.replace(/^refs\/heads\//, '');

if (stem.startsWith('feat/')) {
stem = stem.slice('feat/'.length);
}

if (ingestionType === 'collection' && stem.startsWith('collection/')) {
stem = stem.slice('collection/'.length);
}

if (ingestionType === 'dataset' && stem.startsWith('dataset/')) {
stem = stem.slice('dataset/'.length);
}

// Branch names can include path separators; the ingest filename is the final slug.
if (stem.includes('/')) {
const parts = stem.split('/').filter(Boolean);
const lastPart = parts[parts.length - 1];
if (lastPart) {
stem = lastPart;
}
}

return stem;
};

const RetrieveJSON = async (ref: string, ingestionType: IngestionType) => {
const { OWNER: owner, REPO: repo } = await import('@/config/env').then(
(m) => m.cfg
Expand All @@ -21,8 +51,8 @@ const RetrieveJSON = async (ref: string, ingestionType: IngestionType) => {
throw new Error(`Invalid ingestionType provided: ${ingestionType}`);
}

// The filename is derived from the branch name ('ref').
const fileName = ref.replace('feat/', '');
// The filename is derived from the branch ref slug.
const fileName = normalizeRefToFileStem(ref, ingestionType);
const fullPath = `${targetPath}/${fileName}.json`;

try {
Expand Down
Loading