Skip to content

Commit 5e7e681

Browse files
authored
Merge pull request #243 from OpenPathfinder/ulises/v1-add-gh-org
2 parents 3ab3fd1 + 183d5bc commit 5e7e681

File tree

3 files changed

+509
-3
lines changed

3 files changed

+509
-3
lines changed

__tests__/httpServer/apiV1.test.js

Lines changed: 166 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ let serverStop
3434
let app
3535
let knex
3636
let getAllProjects
37+
let addProject
38+
let getAllGithubOrganizationsByProjectsId
3739

3840
beforeAll(async () => {
3941
// Initialize server asynchronously
@@ -43,7 +45,9 @@ beforeAll(async () => {
4345
app = request(server)
4446
knex = knexInit(dbSettings);
4547
({
46-
getAllProjects
48+
getAllProjects,
49+
addProject,
50+
getAllGithubOrganizationsByProjectsId
4751
} = initializeStore(knex))
4852
})
4953

@@ -222,4 +226,165 @@ describe('HTTP Server API V1', () => {
222226

223227
test.todo('should return 500 when workflow execution times out')
224228
})
229+
230+
describe('POST /api/v1/project/:projectId/gh-org', () => {
231+
let projectId
232+
233+
beforeEach(async () => {
234+
// Create a test project for each test
235+
const project = await addProject({ name: 'test-project' })
236+
projectId = project.id
237+
})
238+
239+
test('should return 201 and add a new GitHub organization', async () => {
240+
const githubOrgUrl = 'https://github.com/expressjs'
241+
242+
const response = await app
243+
.post(`/api/v1/project/${projectId}/gh-org`)
244+
.send({ githubOrgUrl })
245+
246+
expect(response.status).toBe(201)
247+
expect(response.body).toHaveProperty('id')
248+
expect(response.body).toHaveProperty('login', 'expressjs')
249+
expect(response.body).toHaveProperty('html_url', githubOrgUrl.toLowerCase())
250+
expect(response.body).toHaveProperty('project_id', projectId)
251+
252+
// Verify the Location header is set correctly
253+
expect(response.headers).toHaveProperty('location', `/api/v1/project/${projectId}/gh-org/${response.body.id}`)
254+
255+
// Verify organization was added to the database
256+
const orgs = await getAllGithubOrganizationsByProjectsId([projectId])
257+
expect(orgs.length).toBe(1)
258+
expect(orgs[0].html_url).toBe(githubOrgUrl.toLowerCase())
259+
})
260+
261+
test('should correctly extract organization login from URL with trailing slash', async () => {
262+
const githubOrgUrl = 'https://github.com/expressjs/'
263+
264+
const response = await app
265+
.post(`/api/v1/project/${projectId}/gh-org`)
266+
.send({ githubOrgUrl })
267+
268+
expect(response.status).toBe(201)
269+
expect(response.body).toHaveProperty('login', 'expressjs')
270+
expect(response.body).toHaveProperty('html_url', githubOrgUrl.toLowerCase().replace(/\/$/, ''))
271+
})
272+
273+
test('should correctly extract organization login from URL with query parameters', async () => {
274+
const githubOrgUrl = 'https://github.com/expressjs?tab=repositories'
275+
276+
const response = await app
277+
.post(`/api/v1/project/${projectId}/gh-org`)
278+
.send({ githubOrgUrl })
279+
280+
expect(response.status).toBe(201)
281+
expect(response.body).toHaveProperty('login', 'expressjs')
282+
// The stored URL should be normalized without query parameters
283+
expect(response.body.html_url).not.toContain('?')
284+
})
285+
286+
test('should return 400 for invalid project ID', async () => {
287+
const response = await app
288+
.post('/api/v1/project/invalid/gh-org')
289+
.send({ githubOrgUrl: 'https://github.com/expressjs' })
290+
291+
expect(response.status).toBe(400)
292+
expect(response.body).toHaveProperty('errors')
293+
expect(response.body.errors[0]).toHaveProperty('message', 'must be integer')
294+
})
295+
296+
test('should return 400 for zero project ID', async () => {
297+
const response = await app
298+
.post('/api/v1/project/0/gh-org')
299+
.send({ githubOrgUrl: 'https://github.com/expressjs' })
300+
301+
expect(response.status).toBe(400)
302+
expect(response.body).toHaveProperty('errors')
303+
expect(response.body.errors[0]).toHaveProperty('message', 'Invalid project ID. Must be a positive integer.')
304+
})
305+
306+
test('should return 400 for negative project ID', async () => {
307+
const response = await app
308+
.post('/api/v1/project/-1/gh-org')
309+
.send({ githubOrgUrl: 'https://github.com/expressjs' })
310+
311+
expect(response.status).toBe(400)
312+
expect(response.body).toHaveProperty('errors')
313+
expect(response.body.errors[0]).toHaveProperty('message', 'Invalid project ID. Must be a positive integer.')
314+
})
315+
316+
test('should return 400 for invalid GitHub organization URL', async () => {
317+
const response = await app
318+
.post(`/api/v1/project/${projectId}/gh-org`)
319+
.send({ githubOrgUrl: 'https://invalid-url.com/org' })
320+
321+
expect(response.status).toBe(400)
322+
expect(response.body).toHaveProperty('errors')
323+
expect(response.body.errors[0]).toHaveProperty('message', 'must match pattern "^https://github.com/[^/]+"')
324+
})
325+
326+
test('should return 404 for project not found', async () => {
327+
const nonExistentProjectId = 9999999
328+
329+
const response = await app
330+
.post(`/api/v1/project/${nonExistentProjectId}/gh-org`)
331+
.send({ githubOrgUrl: 'https://github.com/expressjs' })
332+
333+
expect(response.status).toBe(404)
334+
expect(response.body).toHaveProperty('errors')
335+
expect(response.body.errors[0]).toHaveProperty('message', 'Project not found')
336+
})
337+
338+
test('should return 409 for duplicate GitHub organization', async () => {
339+
const githubOrgUrl = 'https://github.com/expressjs'
340+
341+
// First add the organization
342+
await app
343+
.post(`/api/v1/project/${projectId}/gh-org`)
344+
.send({ githubOrgUrl })
345+
346+
// Try to add it again
347+
const response = await app
348+
.post(`/api/v1/project/${projectId}/gh-org`)
349+
.send({ githubOrgUrl })
350+
351+
expect(response.status).toBe(409)
352+
expect(response.body).toHaveProperty('errors')
353+
expect(response.body.errors[0]).toHaveProperty('message', 'GitHub organization already exists for this project')
354+
})
355+
356+
test('should return 409 for duplicate GitHub organization with different case', async () => {
357+
// First add the organization with lowercase
358+
await app
359+
.post(`/api/v1/project/${projectId}/gh-org`)
360+
.send({ githubOrgUrl: 'https://github.com/expressjs' })
361+
362+
// Try to add it again with uppercase
363+
const response = await app
364+
.post(`/api/v1/project/${projectId}/gh-org`)
365+
.send({ githubOrgUrl: 'https://github.com/ExpressJS' })
366+
367+
expect(response.status).toBe(409)
368+
expect(response.body).toHaveProperty('errors')
369+
expect(response.body.errors[0]).toHaveProperty('message', 'GitHub organization already exists for this project')
370+
})
371+
372+
test('should return 409 for duplicate GitHub organization with trailing slash', async () => {
373+
// First add the organization without trailing slash
374+
await app
375+
.post(`/api/v1/project/${projectId}/gh-org`)
376+
.send({ githubOrgUrl: 'https://github.com/expressjs' })
377+
378+
// Try to add it again with trailing slash
379+
const response = await app
380+
.post(`/api/v1/project/${projectId}/gh-org`)
381+
.send({ githubOrgUrl: 'https://github.com/expressjs/' })
382+
383+
expect(response.status).toBe(409)
384+
expect(response.body).toHaveProperty('errors')
385+
expect(response.body.errors[0]).toHaveProperty('message', 'GitHub organization already exists for this project')
386+
})
387+
388+
test.todo('should return 500 for internal server error')
389+
})
225390
})

src/httpServer/routers/apiV1.js

Lines changed: 60 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
const pkg = require('../../../package.json')
2-
const { logger } = require('../../utils')
2+
const { logger, validateGithubUrl } = require('../../utils')
33
const { initializeStore } = require('../../store')
44
const _ = require('lodash')
55
const { isSlug } = require('validator')
@@ -27,14 +27,72 @@ const runWorkflow = ({ workflowName, knex, data } = {}) => new Promise((resolve,
2727
})
2828

2929
function createApiRouter (knex, express) {
30-
const { addProject, getProjectByName } = initializeStore(knex)
30+
const { addProject, getProjectByName, addGithubOrganization, getProjectById, getAllGithubOrganizationsByProjectsId } = initializeStore(knex)
3131

3232
const router = express.Router()
3333

3434
router.get('/__health', (req, res) => {
3535
res.json({ status: 'ok', timestamp: new Date().toISOString(), version: pkg.version, name: pkg.name })
3636
})
3737

38+
router.post('/project/:projectId/gh-org', async (req, res) => {
39+
// Parse projectId with explicit radix 10 to avoid issues with leading zeros
40+
const projectId = parseInt(req.params.projectId, 10)
41+
const { githubOrgUrl } = req.body
42+
43+
// Stricter validation: check for NaN, non-integers, and non-positive values
44+
if (isNaN(projectId) || !Number.isInteger(projectId) || projectId <= 0) {
45+
return res.status(400).json({ errors: [{ message: 'Invalid project ID. Must be a positive integer.' }] })
46+
}
47+
if (!githubOrgUrl || !validateGithubUrl(githubOrgUrl)) {
48+
return res.status(400).json({ errors: [{ message: 'Invalid GitHub organization name' }] })
49+
}
50+
51+
const project = await getProjectById(projectId)
52+
if (!project) {
53+
return res.status(404).json({ errors: [{ message: 'Project not found' }] })
54+
}
55+
56+
// Normalize the input URL by parsing it with URL object
57+
// This allows us to handle different protocols, query parameters, and trailing slashes
58+
const urlObj = new URL(githubOrgUrl)
59+
// Create a normalized URL: lowercase, no trailing slash, no query parameters
60+
const normalizedGithubOrgUrl = `${urlObj.protocol}//${urlObj.host}${urlObj.pathname.replace(/\/$/, '')}`.toLowerCase()
61+
62+
const existingGhOrg = await getAllGithubOrganizationsByProjectsId([projectId])
63+
if (existingGhOrg.some(org => {
64+
try {
65+
// Apply the same normalization to stored URLs for consistent comparison
66+
const storedUrl = new URL(org.html_url)
67+
const normalizedStoredUrl = `${storedUrl.protocol}//${storedUrl.host}${storedUrl.pathname.replace(/\/$/, '')}`.toLowerCase()
68+
return normalizedStoredUrl === normalizedGithubOrgUrl
69+
} catch (e) {
70+
// Fallback to simple comparison if URL parsing fails
71+
return org.html_url.toLowerCase().replace(/\/$/, '') === normalizedGithubOrgUrl
72+
}
73+
})) {
74+
return res.status(409).json({ errors: [{ message: 'GitHub organization already exists for this project' }] })
75+
}
76+
77+
try {
78+
// Get the pathname from the already created URL object, remove leading/trailing slashes, and take the first segment
79+
const login = urlObj.pathname.replace(/^\/|\/$/, '').split('/')[0]
80+
81+
const org = await addGithubOrganization({
82+
html_url: normalizedGithubOrgUrl,
83+
login,
84+
project_id: project.id
85+
})
86+
87+
return res.status(201)
88+
.header('Location', `/api/v1/project/${projectId}/gh-org/${org.id}`)
89+
.json(org)
90+
} catch (err) {
91+
logger.error(err)
92+
return res.status(500).json({ errors: [{ message: 'Internal server error' }] })
93+
}
94+
})
95+
3896
router.post('/project', async (req, res) => {
3997
try {
4098
// Validate request body

0 commit comments

Comments
 (0)