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

Calculation Endpoints: Adds Validation and Tests #1521

Merged
merged 9 commits into from
Nov 22, 2023
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
4 changes: 3 additions & 1 deletion .github/workflows/tdm-server-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ on:
branches:
- develop
jobs:
build:
server-test:
runs-on: ubuntu-latest

steps:
Expand Down Expand Up @@ -39,6 +39,8 @@ jobs:
echo "APPLICATIONINSIGHTS_CONNECTION_STRING=InstrumentationKey=00000000-0000-0000-0000-000000000000;IngestionEndpoint=https://westus-0.in.applicationinsights.azure.com/;LiveEndpoint=https://westus.livediagnostics.monitor.azure.com/" >> .env
echo "[email protected]" >> .env
echo "SECURITY_ADMIN_PASSWORD=Dogfood1!" >> .env
echo "[email protected]" >> .env
echo "ADMIN_PASSWORD=Dogfood1!" >> .env
echo "SQL_SERVER_NAME=localhost" >> .env
echo "SQL_SERVER_PORT=1434" >> .env
echo "SQL_DATABASE_NAME=tdmtestdb" >> .env
Expand Down
12 changes: 0 additions & 12 deletions client/src/services/calculation.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,3 @@ export function get() {
export function getById(id) {
return axios.get(`${baseUrl}/${id}`);
}

export function post(calculation) {
return axios.post(baseUrl, calculation);
}

export function put(calculation) {
return axios.put(baseUrl + "/" + calculation.id, calculation);
}

export function del(id) {
return axios.delete(baseUrl + "/" + id);
}
44 changes: 23 additions & 21 deletions server/.example.env → server/.env.example
Copy link
Member Author

Choose a reason for hiding this comment

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

I changed the file name from
.example.env -> .env.example as it was before (better naming)

Original file line number Diff line number Diff line change
Expand Up @@ -67,31 +67,33 @@ APPLICATIONINSIGHTS_CONNECTION_STRING=
###############################################################
## Testing (Comment out ALL the above variables) ##
###############################################################
# TEST_ENV=
# TEST_ENV=true

# PORT=
# NODE_OPTIONS=
# PORT=5002
# NODE_OPTIONS=--trace-deprecation

# JWT_SECRET_KEY=
# JWT_SECRET_KEY=testingSecretKey

# CLIENT_URL=
# SERVER_URL=
# CLIENT_URL=http://localhost:3001
# SERVER_URL=http://localhost:5002

# SENDGRID_API_KEY=
# EMAIL_SENDER=
# EMAIL_PUBLIC_COMMENT_LA_CITY=
# EMAIL_PUBLIC_COMMENT_WEB_TEAM=
# SENDGRID_API_KEY=SG.testAPIkey
Copy link
Member Author

Choose a reason for hiding this comment

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

Since the testing environment is local and isolated, I added the dummy variables with fake API keys into the example .env file since we already have them in the CI file .github/workflows/tdm-server-tests.yml

Copy link
Member

Choose a reason for hiding this comment

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

Does this mean tdm-server-tests can run with .env.example?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope! I just changed it because VSCode has better highlighting with the new name. Also included the variables so new members could run the tests easily without the formal variables. I tried to get this to work unsuccessfully with ./push.sh
An alternate strategy I thought of this morning is a function in the push script that can uncomment the test variables for the server tests and then re-comment upon completion. I'll have to check if this is viable, but editing the .env file with code feels unkosher. Thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

I wouldn't edit the .env file. Instead, switch between files. That way the server tests can have one. Local dev can have another, prod, etc.

https://stackoverflow.com/questions/55406055/toggle-between-multiple-env-files-like-env-development-with-node-js

Copy link
Member Author

Choose a reason for hiding this comment

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

I was looking into that but had some concerns.

Since the container has a single database that is restored before every test suite (i.e. before every test file), the logic that handles this will call tedious-pool.js the connection logic from the server which will consume the original .env file. this means that this file would require a conditional to see which .env to consume right? Is this crufty?

Copy link
Member

Choose a reason for hiding this comment

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

It means everywhere we use dotenv should probably go through a tdmenv wrapper.

Copy link
Member Author

Choose a reason for hiding this comment

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

so new issue for refactoring codebase with this ^

# EMAIL_SENDER=[email protected]
# EMAIL_PUBLIC_COMMENT_LA_CITY=[email protected]
# EMAIL_PUBLIC_COMMENT_WEB_TEAM=[email protected]

# APPLICATIONINSIGHTS_CONNECTION_STRING=
# APPLICATIONINSIGHTS_CONNECTION_STRING=InstrumentationKey=00000000-0000-0000-0000-000000000000;IngestionEndpoint=https://westus-0.in.applicationinsights.azure.com/;LiveEndpoint=https://westus.livediagnostics.monitor.azure.com/

## Server Test Accounts
# SECURITY_ADMIN_EMAIL=
# SECURITY_ADMIN_PASSWORD=
# # Server Test Accounts
# [email protected]
# SECURITY_ADMIN_PASSWORD=Dogfood1!
# [email protected]
# ADMIN_PASSWORD=Dogfood1!

## testingcontainer Environment Variables
# SQL_SERVER_NAME=
# SQL_SERVER_PORT=
# SQL_DATABASE_NAME=
# SQL_USER_NAME=
# SQL_PASSWORD=
# SQL_ENCRYPT =
# # testingcontainer Environment Variables
# SQL_SERVER_NAME=localhost
# SQL_SERVER_PORT=1434
# SQL_DATABASE_NAME=tdmtestdb
# SQL_USER_NAME=sa
# SQL_PASSWORD=TestPassw0rd
# SQL_ENCRYPT = false
72 changes: 72 additions & 0 deletions server/__tests__/calculation.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
const request = require("supertest");
const {
setupServer,
teardownServer
} = require("../_jest-setup_/utils/server-setup");

let server;

beforeAll(async () => {
server = await setupServer();
});

afterAll(async () => {
await teardownServer();
});

describe("tests that require a calculation id", () => {
//////////////////////////////
// general //
//////////////////////////////
let calcId; // id of the calculation

beforeEach(async () => {
// get a calculation id for the tests
const res = await request(server).get("/api/calculations");
calcId = res.body[0].id;
});

afterEach(async () => {
// cleanup state
calcId = undefined;
});

// GET "/" all calculations
it("should get all calculations", async () => {
const res = await request(server).get("/api/calculations");
expect(Array.isArray(res.body)).toBeTruthy();
res.body.forEach(calc => {
expect(calc).toHaveProperty("id");
expect(calc).toHaveProperty("name");
expect(calc).toHaveProperty("description");
expect(calc).toHaveProperty("deprecated");
expect(calc).toHaveProperty("dateCreated");
expect(calc).toHaveProperty("dateModified");
});
});

// GET "/:calcId" calculation by id
it("should get calculation by id", async () => {
const res = await request(server).get(`/api/calculations/${calcId}`);
expect(res.statusCode).toEqual(200);
expect(res.body.id).toEqual(calcId);
});

// GET "/:calcId" calculation by inexistent id
it("should NOT get calculation by id", async () => {
const res = await request(server).get(`/api/calculations/9999999`);
expect(res.statusCode).toEqual(404);
});

// GET "/:calcId/rules" all rules for a calculation
it("should get all rules for a calculation", async () => {
const res = await request(server).get(`/api/calculations/${calcId}/rules`);
expect(res.statusCode).toEqual(200);
expect(Array.isArray(res.body)).toBeTruthy();
res.body.forEach(calc => {
expect(calc).toHaveProperty("id");
expect(calc).toHaveProperty("calculationId");
expect(calc.calculationId).toEqual(calcId);
});
});
});
32 changes: 1 addition & 31 deletions server/app/controllers/calculation.controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,37 +22,7 @@ const getById = async (req, res) => {
}
};

const post = async (req, res) => {
try {
const outputParms = await calculationService.post(req.body);
res.status(201).json(outputParms);
} catch (err) {
res.status(500).send(err);
}
};

const put = async (req, res) => {
try {
await calculationService.put(req.body);
res.sendStatus(200);
} catch (err) {
res.status(500).send(err);
}
};

const del = async (req, res) => {
try {
await calculationService.del(req.params.id);
res.sendStatus(200);
} catch (err) {
res.status(500).send(err);
}
};

module.exports = {
getAll,
getById,
post,
put,
del
getById
};
16 changes: 0 additions & 16 deletions server/app/routes/calculation.routes.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,26 +2,10 @@ const router = require("express").Router();

const calculationController = require("../controllers/calculation.controller");
const ruleController = require("../controllers/rule.controller");
const jwtSession = require("../../middleware/jwt-session");

module.exports = router;

router.get("/:id", calculationController.getById);
// Get all the rules for a calculation
router.get("/:id/rules", ruleController.getByCalculationId);
router.get("/", calculationController.getAll);
router.post(
"/",
jwtSession.validateRoles(["isAdmin"]),
calculationController.post
);
router.put(
"/:id",
jwtSession.validateRoles(["isAdmin"]),
calculationController.put
);
router.delete(
"/:id",
jwtSession.validateRoles(["isAdmin"]),
calculationController.del
);
46 changes: 1 addition & 45 deletions server/app/services/calculation.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,51 +28,7 @@ const getById = async id => {
}
};

const post = async item => {
try {
await poolConnect;
const request = pool.request();
request.input("name", mssql.NVarChar, item.name); // 50
request.input("description", mssql.NVarChar, item.description); // MAX
request.input("deprecated", mssql.Bit, item.deprecated);
request.output("id", mssql.Int, null);
const response = await request.execute("Calculation_Insert");

return response.outputParameters;
} catch (err) {
return Promise.reject(err);
}
};

const put = async item => {
try {
await poolConnect;
const request = pool.request();
request.input("name", mssql.NVarChar, item.name); // 50
request.input("description", mssql.NVarChar, item.description); // MAX
request.input("deprecated", mssql.Bit, item.deprecated);
request.input("id", mssql.Int, item.id);
await request.execute("Calculation_Update");
} catch (err) {
return Promise.reject(err);
}
};

const del = async id => {
try {
await poolConnect;
const request = pool.request();
request.input("id", mssql.Int, id);
await request.execute("Calculation_Delete");
} catch (err) {
return Promise.reject(err);
}
};

module.exports = {
getAll,
getById,
post,
put,
del
getById
};