Created
December 5, 2018 19:29
-
-
Save berkeleycole/823ed7c88bef3274e7678ada7a24d17d to your computer and use it in GitHub Desktop.
loose_functions_code_smell.js
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| var fetch = require("node-fetch"); | |
| module.exports = (app) => { | |
| // Collect some information from the Pull Request in order to leave a comment later | |
| let number; | |
| let id; | |
| // listen for a newly opened pull request and find its number | |
| app.on('pull_request.opened', async context => { | |
| app.log(":::PULL REQUEST OPENED:::") | |
| number = context.payload.number | |
| }) | |
| // listen for a newly re-opened pull request and find its number | |
| app.on('pull_request.reopened', async context => { | |
| app.log(":::PULL REQUEST RE-OPENED:::") | |
| number = context.payload.number | |
| }) | |
| // listen for an incoming status event (will be triggered by a PR action) | |
| app.on('status', async context => { | |
| // To see what's going on, un-comment the logs: | |
| // you can log "context.payload" any time | |
| // app.log("payload: ", context.payload.state); | |
| // If the status event shows a failure | |
| if(context.payload.state === "failure") { | |
| // app.log(":::BUILD FAILED:::") | |
| // collect the id from the PR commit | |
| id = context.payload.commit.sha | |
| // fetch the url of the artifact file generated by a failing Jest test on Circle CI | |
| fetch("https://circleci.com/api/v1.1/project/github/berkeleycole/portfolio-test/latest/artifacts?circle-token=1cc94c91b04e237e0d675351ffe8c62d70bf040b&filter=failed", { | |
| headers: { | |
| "Accept": "application/json" | |
| }, | |
| }) | |
| .then((resp) => resp.json()) | |
| .then(json => { | |
| // fetch the artifact text file (url is included in the response from Circle CI) | |
| fetch(`${json[0].url}`) | |
| .then(artifact => { | |
| let j = artifact.json() | |
| return j | |
| }) | |
| .then(j => { | |
| // each Jest test has its own results, check each test for a failure | |
| j.testResults.map((test, index) => { | |
| // app.log(":::CHECKING TEST:::") | |
| // if test fails: | |
| if(test.status === "failed"){ | |
| // app.log(":::FOUND FAILING TEST:::") | |
| // create body of comment to be added to PR | |
| let jestFailMessage = "Oh no! A test failed, this was the error message recieved: \n\n" + test.message | |
| // assemble comment: | |
| // comment_id is the sha code of the PR commit | |
| // number is the number of the PR (only recorded in the PR context payload) | |
| const comment = context.issue({ body: jestFailMessage, comment_id: id, number: number}) | |
| // create the comment | |
| return context.github.issues.createComment(comment) | |
| } | |
| }) | |
| }) | |
| }) | |
| } | |
| }) | |
| } | |
| // solutions | |
| // This code has a lot going on, but no organized place to put all that functionality, and is therefore fragile and less useful | |
| // closest code smell: Primitive Obsession | |
| // Don't use a gaggle of primitive data type variables as a poor man's substitute for a class. | |
| // If your data type is sufficiently complex, write a class to represent it. | |
| // This is the service with functionality I pulled out of the old program | |
| // Lightbulb moment for me - once I pulled the functions out, they were so much easier to see and edit - and I could easily extend my program! | |
| class ProjectService { | |
| constructor(probot) { | |
| this.db = PG | |
| if (probot) this.probot = probot | |
| // watchers are the projects -- each with their own instance of Commenter | |
| this.watchers = {} | |
| this.initDB() | |
| .then(() => this.loadData()) | |
| .catch(err => console.log("error initializing project service", err)) | |
| } | |
| list() { | |
| return this.db.any(`SELECT * FROM projects`) | |
| } | |
| create(username, project, token) { | |
| return this.db.one(`INSERT INTO projects (username, project, token) VALUES($1, $2, $3) RETURNING id`, [username, project, token]) | |
| .then((res) => { | |
| const id = res.id | |
| console.log(`creating commenter instance for ${id} : ${username}/${project}`); | |
| this.watchers[id] = new Commenter({ | |
| probot: this.probot, | |
| user: username, | |
| project: project, | |
| token: token, | |
| }) | |
| return res | |
| }) | |
| } | |
| destroy(id) { | |
| return this.db.one(`DELETE FROM projects WHERE id=$1`, [id]) | |
| .then((data) => { | |
| delete this.watchers[id] | |
| return data | |
| }) | |
| } | |
| destroyAll() { | |
| return this.db.one(`TRUNCATE projects`) | |
| .then((data) => { | |
| this.watchers = {} // reset in-memory value | |
| return data | |
| }) | |
| } | |
| initDB() { | |
| // load the uuid extension | |
| return this.db.any(`CREATE EXTENSION IF NOT EXISTS "uuid-ossp"`) | |
| .then(data => { | |
| // create tables after the extension is loaded | |
| return this.db.any(`CREATE TABLE IF NOT EXISTS projects ( | |
| id UUID DEFAULT uuid_generate_v1(), | |
| project VARCHAR(255) NOT NULL, | |
| username VARCHAR(255) NOT NULL, | |
| token VARCHAR(255) NOT NULL, | |
| UNIQUE (project, username, token), | |
| CONSTRAINT project_pkey_ PRIMARY KEY (id) | |
| )`) | |
| .catch(err => console.log(err)) | |
| }) | |
| .catch(err => console.log(err)) | |
| } | |
| loadData() { | |
| return this.list().then(projects => { | |
| projects.map(p => { | |
| console.log(`creating commenter instance for ${p.id} : ${p.username}/${p.project}`); | |
| this.watchers[p.id] = new Commenter({ | |
| probot: this.probot, | |
| project: p.project, | |
| user: p.username, | |
| token: p.token, | |
| }) | |
| }) | |
| }) | |
| } | |
| } | |
| module.exports = ProjectService |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment