Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add pullrequests/:id route to fetch PRs of a given id of user using GitHub API #90

Merged
merged 12 commits into from
Dec 3, 2020
Merged
5 changes: 5 additions & 0 deletions config/custom-environment-variables.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ module.exports = {
__format: 'boolean'
},

githubApi: {
baseUrl: 'GITHUB_API_BASE_URL',
org: 'GITHUB_ORGANISATION'
},

githubOauth: {
clientId: 'GITHUB_CLIENT_ID',
clientSecret: 'GITHUB_CLIENT_SECRET'
Expand Down
5 changes: 5 additions & 0 deletions config/default.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@ module.exports = {
enableFileLogs: true,
enableConsoleLogs: false,

githubApi: {
baseUrl: 'https://api.github.com',
org: 'Real-Dev-Squad'
},

githubOauth: {
clientId: '<clientId>',
clientSecret: '<clientSecret>'
Expand Down
58 changes: 58 additions & 0 deletions controllers/pullRequestsController.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
const logger = require('../utils/logger')
const githubService = require('../services/githubService')

/**
* Loops over an array of objects, takes a value corresponding to key provided and saves it in an array
*
* @param arrayOfObjects {Array} - Array of objects to loop over
* @param key {String} - Value corresponding to this key is saved
*/

const getNames = (arrayOfObjects, key) => {
const names = []
arrayOfObjects.forEach((object) => {
names.push(object[key])
})
return names
}

/**
* Collects all pull requests and sends only required data for each pull request
*
* @param req {Object} - Express request object
* @param res {Object} - Express response object
*/

const getPRdetails = async (req, res) => {
try {
const data = await githubService.fetchPRsByUser(req.params.id)

if (data.total_count) {
const allPRs = []
data.items.forEach(({ title, html_url: htmlUrl, state, created_at: createdAt, updated_at: updatedAt, draft, labels, assignees }) => {
const allAssignees = getNames(assignees, 'login')
const allLabels = getNames(labels, 'name')
allPRs.push({
title: title,
url: htmlUrl,
state: state,
created_at: createdAt,
updated_at: updatedAt,
ready_for_review: state === 'closed' ? false : !draft,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

snake_case -> camelCase -> snake_case conversion detected.
Any particular reason?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. Fixed it.

labels: allLabels,
assignees: allAssignees
})
})
return res.json(allPRs)
}
return res.json([])
} catch (err) {
logger.error(`Error while processing pull requests: ${err}`)
return res.boom.badImplementation('Something went wrong please contact admin')
}
}

module.exports = {
getPRdetails,
getNames
}
1 change: 1 addition & 0 deletions routes/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,6 @@ app.use('/healthcheck', require('./healthCheck.js'))
app.use('/auth', require('./auth.js'))
app.use('/users', require('./users.js'))
app.use('/members', require('./members.js'))
app.use('/pullrequests', require('./pullrequests.js'))

module.exports = app
29 changes: 29 additions & 0 deletions routes/pullrequests.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
const express = require('express')
const router = express.Router()
const pullRequestController = require('../controllers/pullRequestsController')

/**
* @swagger
* /pullrequests/:id:
* get:
* summary: Gets pull requests of a user in Real Dev Squad Github organisation
* tags:
* - Pull Requests
* responses:
* 200:
* description: Details of pull requests by a particular user in Real Dev Squad
* content:
* application/json:
* schema:
* $ref: '#/components/schemas/pullRequests'
* 500:
* description: badImplementation
* content:
* application/json:
* schema:
* $ref: '#/components/schemas/errors/badImplementation'
*/

router.get('/:id', pullRequestController.getPRdetails)

module.exports = router
27 changes: 27 additions & 0 deletions services/githubService.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
const logger = require('../utils/logger')
const config = require('config')
const { fetch } = require('../utils/fetch')
const { fetchUser } = require('../models/users')

/**
* Fetches the pull requests in Real-Dev-Squad by user using GitHub API
*
* @param req {Object} - Express request object
* @param res {Object} - Express response object
*/

const fetchPRsByUser = async (id) => {
try {
const { user } = await fetchUser(id)
const url = `${config.get('githubApi.baseUrl')}/search/issues?q=org:${config.get('githubApi.org')}+author:${user.github_id}+type:pr`
const { data } = await fetch(url)
return data
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT: What if we directly return fetch call, and destructure the data at call site?
@ankurnarkhede @swarajpure

Anyway TailCallOptimization will be there, but at call site we are using await, which is not required because we are returning data directly, not a promise.
What's ideal in this case ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have returned fetch call and destructured the data at function call

And about your second question, we need to use await at function call, otherwise data.total_count is undefined because data is not returned yet.

} catch (err) {
logger.error(`Error while fetching pull requests: ${err}`)
throw err
}
}

module.exports = {
fetchPRsByUser
}
6 changes: 4 additions & 2 deletions lib/fetch.js → utils/fetch.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
const axios = require('axios')
const logger = require('../utils/logger')
const logger = require('./logger')

/**
* Used for network calls
Expand Down Expand Up @@ -29,4 +29,6 @@ const fetch = async (url, method = 'get', params = null, data = null, headers =
}
}

module.exports = fetch
module.exports = {
fetch
}
29 changes: 29 additions & 0 deletions utils/swaggerDefinition.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,35 @@ const swaggerOptions = {
}
}
},
pullRequests: {
type: 'array',
properties: {
title: {
type: 'string'
},
url: {
type: 'string'
},
state: {
type: 'string'
},
created_at: {
type: 'string'
},
updated_at: {
type: 'string'
},
ready_for_review: {
type: 'boolean'
},
labels: {
type: 'array'
},
assignees: {
type: 'array'
}
}
},
users: {
type: 'object',
properties: {
Expand Down