Repository: naturalcrit/homebrewery (js)

Issue:
Config object is created several times through the codebase
We have the following piece of code copied at least three times:

https://github.com/naturalcrit/homebrewery/blob/279352377bbd533f1062af29f8328dc9a618c1ad/server.js#L74-L78

https://github.com/naturalcrit/homebrewery/blob/279352377bbd533f1062af29f8328dc9a618c1ad/server/googleActions.js#L6-L10

https://github.com/naturalcrit/homebrewery/blob/279352377bbd533f1062af29f8328dc9a618c1ad/server/token.js#L4-L8

This doesn't looks good and might lead to errors if we decide to update the way config is loaded.

What I would like to do is to load the config once in the entry point file `server.js` and then pass it as an argument to every module/function where it is used.

I'm submitting an issue do discuss such refactoring first, because it might complicate the code a bit: one extra argument will be passed around some code paths. Please let me know what do you think about the suggested approach

Pull request:
[NFC] Outline config creation into a separate module
This is done in order to have config creation rules
unified in one place to avoid modifying them multiple times
if they change.

We already had 3 duplicated pieces of code initializing the
config and there will be more config uses in future tests.

This resolves #1960

Patch:
diff --git a/server.js b/server.js
index c9ce63c8..cb58f9bd 100644
--- a/server.js
+++ b/server.js
@@ -1,11 +1,6 @@
 const DB = require('./server/db.js');
 const server = require('./server/app.js');
-
-const config = require('nconf')
-	.argv()
-	.env({ lowerCase: true })
-	.file('environment', { file: `config/${process.env.NODE_ENV}.json` })
-	.file('defaults', { file: 'config/default.json' });
+const config = require('./server/config.js');
 
 DB.connect(config).then(()=>{
 	// Ensure that we have successfully connected to the database
diff --git a/server/app.js b/server/app.js
index db2eaac5..2437e3b3 100644
--- a/server/app.js
+++ b/server/app.js
@@ -1,9 +1,13 @@
 /*eslint max-lines: ["warn", {"max": 300, "skipBlankLines": true, "skipComments": true}]*/
+// Set working directory to project root
+process.chdir(`${__dirname}/..`);
+
 const _ = require('lodash');
 const jwt = require('jwt-simple');
 const express = require('express');
 const yaml = require('js-yaml');
 const app = express();
+const config = require('./config.js');
 
 const homebrewApi = require('./homebrew.api.js');
 const GoogleActions = require('./googleActions.js');
@@ -62,22 +66,13 @@ const splitTextStyleAndMetadata = (brew)=>{
 	}
 };
 
-app.use('/', serveCompressedStaticAssets(`${__dirname}/../build`));
-
-process.chdir(__dirname);
+app.use('/', serveCompressedStaticAssets(`build`));
 
 //app.use(express.static(`${__dirname}/build`));
 app.use(require('body-parser').json({ limit: '25mb' }));
 app.use(require('cookie-parser')());
 app.use(require('./forcessl.mw.js'));
 
-// FIXME: the config should be passed as an argument for the app
-const config = require('nconf')
-	.argv()
-	.env({ lowerCase: true })
-	.file('environment', { file: `config/${process.env.NODE_ENV}.json` })
-	.file('defaults', { file: 'config/default.json' });
-
 //Account Middleware
 app.use((req, res, next)=>{
 	if(req.cookies && req.cookies.nc_session){
@@ -99,16 +94,16 @@ app.use(homebrewApi);
 app.use(require('./admin.api.js'));
 
 const HomebrewModel  = require('./homebrew.model.js').model;
-const welcomeText    = require('fs').readFileSync('./../client/homebrew/pages/homePage/welcome_msg.md', 'utf8');
-const welcomeTextV3  = require('fs').readFileSync('./../client/homebrew/pages/homePage/welcome_msg_v3.md', 'utf8');
-const changelogText  = require('fs').readFileSync('./../changelog.md', 'utf8');
-const faqText        = require('fs').readFileSync('./../faq.md', 'utf8');
+const welcomeText    = require('fs').readFileSync('client/homebrew/pages/homePage/welcome_msg.md', 'utf8');
+const welcomeTextV3  = require('fs').readFileSync('client/homebrew/pages/homePage/welcome_msg_v3.md', 'utf8');
+const changelogText  = require('fs').readFileSync('changelog.md', 'utf8');
+const faqText        = require('fs').readFileSync('faq.md', 'utf8');
 
 String.prototype.replaceAll = function(s, r){return this.split(s).join(r);};
 
 //Robots.txt
 app.get('/robots.txt', (req, res)=>{
-	return res.sendFile(`${__dirname}/robots.txt`);
+	return res.sendFile(`robots.txt`, { root: process.cwd() });
 });
 
 //Home page
diff --git a/server/config.js b/server/config.js
new file mode 100644
index 00000000..fc50e68b
--- /dev/null
+++ b/server/config.js
@@ -0,0 +1,5 @@
+module.exports = require('nconf')
+    .argv()
+    .env({ lowerCase: true })
+    .file('environment', { file: `config/${process.env.NODE_ENV}.json` })
+    .file('defaults', { file: 'config/default.json' });
diff --git a/server/googleActions.js b/server/googleActions.js
index d780dc4a..9d17abdc 100644
--- a/server/googleActions.js
+++ b/server/googleActions.js
@@ -3,11 +3,7 @@ const _ = require('lodash');
 const { google } = require('googleapis');
 const { nanoid } = require('nanoid');
 const token = require('./token.js');
-const config = require('nconf')
-	.argv()
-	.env({ lowerCase: true })	// Load environment variables
-	.file('environment', { file: `config/${process.env.NODE_ENV}.json` })
-	.file('defaults', { file: 'config/default.json' });
+const config = require('./config.js');
 
 //let oAuth2Client;
 
diff --git a/server/token.js b/server/token.js
index 40d76a48..70d6e01c 100644
--- a/server/token.js
+++ b/server/token.js
@@ -1,11 +1,7 @@
 const jwt = require('jwt-simple');
 
 // Load configuration values
-const config = require('nconf')
-	.argv()
-	.env({ lowerCase: true })	// Load environment variables
-	.file('environment', { file: `config/${process.env.NODE_ENV}.json` })
-	.file('defaults', { file: 'config/default.json' });
+const config = require('./config.js');
 
 // Generate an Access Token for the given User ID
 const generateAccessToken = (account)=>{


Test patch:
diff --git a/tests/routes/static-pages.test.js b/tests/routes/static-pages.test.js
index 0d86bc30..e69706f5 100644
--- a/tests/routes/static-pages.test.js
+++ b/tests/routes/static-pages.test.js
@@ -21,9 +21,7 @@ describe('Tests for static pages', ()=>{
 		return app.get('/faq').expect(200);
 	});
 
-	// FIXME: robots.txt file can't be properly loaded under testing environment,
-	// most likely due to __dirname being different from what is expected
-	it.skip('robots.txt works', ()=>{
+	it('robots.txt works', ()=>{
 		return app.get('/robots.txt').expect(200);
 	});
 });