Skip to content

refactor ( website-builder ): Refactor website-builder to typescript #5

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.

Sign up for GitHub

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

Open
Hamza-Asif99 wants to merge 1 commit into
base: the-one
Choose a base branch
from
Open

refactor ( website-builder ): Refactor website-builder to typescript #5

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
66 changes: 37 additions & 29 deletions index.js → index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,33 +1,33 @@
import express from "express";
import express = require("express");
import {exec} from "child_process";
import crypto from "crypto";
import dotenv from "dotenv";
import crypto = require("crypto");
import dotenv = require("dotenv");
Comment on lines +1 to +4
Copy link
Copy Markdown
Member

@Idrinth Idrinth May 23, 2024

Choose a reason for hiding this comment

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

proper imports please, no requires.


dotenv.config();

const app = express();
const port = process.env.PORT || 3000;

let isDocumentationWebsiteUpdated = false;
let isMindmapUpdated = false;
let contributorsBuildRequired = false;
let isDocumentationWebsiteUpdated: boolean = false;
let isMindmapUpdated: boolean = false;
let contributorsBuildRequired: boolean = false;

let documentationWebsiteBuildTime = 0;
let mindmapBuildTime = 0;
let contributorsBuildTime = 0;
let documentationWebsiteBuildTime: number = 0;
let mindmapBuildTime: number = 0;
let contributorsBuildTime: number = 0;

app.use(express.json());

app.post("/webhook", async (req, res) => {
app.post("/webhook", async (req: express.Request, res: express.Response) => {
console.log("req receieved");
const signature = req.headers["x-hub-signature"];
const payload = JSON.stringify(req.body);

const hmac = crypto.createHmac("sha1", process.env.GITHUB_SECRET);
const hmac = crypto.createHmac("sha1", process.env.GITHUB_SECRET as string);
const calculatedSignature = `sha1=${hmac.update(payload).digest("hex")}`;

if (crypto.timingSafeEqual(Buffer.from(signature), Buffer.from(calculatedSignature))) {
const { result, respMessage } = await getBranchStatus();
if (crypto.timingSafeEqual(Buffer.from(signature as string), Buffer.from(calculatedSignature))) {
const { result, respMessage } = await getBranchStatus(null);
Comment on lines +21 to +30
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot May 21, 2024

Choose a reason for hiding this comment

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

CodeRabbit identifies a potential security risk. The signature from req.headers["x-hub-signature"] should be validated for existence before being used. This prevents potential runtime errors when the header is missing.

-  if (crypto.timingSafeEqual(Buffer.from(signature as string), Buffer.from(calculatedSignature))) {
+  if (signature && crypto.timingSafeEqual(Buffer.from(signature as string), Buffer.from(calculatedSignature))) {
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
app.post("/webhook", async (req: express.Request, res: express.Response) => {
console.log("req receieved");
const signature = req.headers["x-hub-signature"];
const payload = JSON.stringify(req.body);
const hmac = crypto.createHmac("sha1", process.env.GITHUB_SECRET);
const hmac = crypto.createHmac("sha1", process.env.GITHUB_SECRET as string);
const calculatedSignature = `sha1=${hmac.update(payload).digest("hex")}`;
if (crypto.timingSafeEqual(Buffer.from(signature), Buffer.from(calculatedSignature))) {
const { result, respMessage } = await getBranchStatus();
if (crypto.timingSafeEqual(Buffer.from(signature as string), Buffer.from(calculatedSignature))) {
const { result, respMessage } = await getBranchStatus(null);
app.post("/webhook", async (req: express.Request, res: express.Response) => {
console.log("req receieved");
const signature = req.headers["x-hub-signature"];
const payload = JSON.stringify(req.body);
const hmac = crypto.createHmac("sha1", process.env.GITHUB_SECRET as string);
const calculatedSignature = `sha1=${hmac.update(payload).digest("hex")}`;
if (signature && crypto.timingSafeEqual(Buffer.from(signature as string), Buffer.from(calculatedSignature))) {
const { result, respMessage } = await getBranchStatus(null);

console.log("Result: ", result);
res.status(result).send(respMessage);
} else {
Expand All @@ -39,66 +39,74 @@ app.listen(process.env.PORT, () => {
console.log(`Server listening on port ${port}`);
});

const executeCmd = async (cmd) => {
const executeCmd = async (cmd: string) => {
let stdout;
let stderr;
try {
const { stdout, stderr } = await exec(cmd);
const cmdResp = await exec(cmd);

Check warning

Code scanning / CodeQL

Indirect uncontrolled command line

This command depends on an unsanitized [environment variable](1). This command depends on an unsanitized [environment variable](2). This command depends on an unsanitized [environment variable](3). This command depends on an unsanitized [environment variable](4). This command depends on an unsanitized [environment variable](5). This command depends on an unsanitized [environment variable](6).
stderr = cmdResp.stderr;
stdout = cmdResp.stdout
Comment on lines +42 to +48
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot May 21, 2024

Choose a reason for hiding this comment

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

CodeRabbit detects an issue with error handling. The error thrown in the catch block should include more contextual information to aid in debugging. Additionally, consider logging the command that failed.

-    throw new Error(stderr + "\n" + stdout);
+    throw new Error(`Command failed: ${cmd}\nError: ${stderr}\nOutput: ${stdout}`);
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
const executeCmd = async (cmd: string) => {
let stdout;
let stderr;
try {
const { stdout, stderr } = await exec(cmd);
const cmdResp = await exec(cmd);
stderr = cmdResp.stderr;
stdout = cmdResp.stdout
const executeCmd = async (cmd: string) => {
let stdout;
let stderr;
try {
const cmdResp = await exec(cmd);
stderr = cmdResp.stderr;
stdout = cmdResp.stdout;
} catch (error) {
throw new Error(`Command failed: ${cmd}\nError: ${stderr}\nOutput: ${stdout}`);
}
};

Comment on lines +46 to +48
Copy link
Copy Markdown
Member

@Idrinth Idrinth May 23, 2024

Choose a reason for hiding this comment

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

please revert the structural change, the extra lines for splitting are unnecessary :)

return stderr + "\n" + stdout;
} catch (error) {
console.error(`exec error: ${error}`);
throw new Error(stderr + "\n" + stdout);
}
};

const getBranchStatus = async (req) => {
const getBranchStatus = async (req: any) => {
Copy link
Copy Markdown
Member

@Idrinth Idrinth May 23, 2024

Choose a reason for hiding this comment

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

no use of any please. Create an interface for the github token please

console.log("Webhook received successfully");

const branchName = req.body?.ref?.split("/").pop();
if (!branchName) {
return 400, "Branch name not found in the request.";
return {result: 400, respMessage: "Branch name not found in the request."};
}
return branchName === process.env.BRANCH_NAME ? await buildProject() : 202, "Build not required.";
return branchName === process.env.BRANCH_NAME ? await buildProject() : {result: 202, respMessage: "Build not required."};
Comment on lines +56 to +63
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot May 21, 2024

Choose a reason for hiding this comment

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

CodeRabbit suggests improving the type safety of the req parameter in getBranchStatus. Currently, it is typed as any, which defeats the purpose of using TypeScript for type safety.

- const getBranchStatus = async (req: any) => {
+ const getBranchStatus = async (req: express.Request) => {
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
const getBranchStatus = async (req: any) => {
console.log("Webhook received successfully");
const branchName = req.body?.ref?.split("/").pop();
if (!branchName) {
return 400, "Branch name not found in the request.";
return {result: 400, respMessage: "Branch name not found in the request."};
}
return branchName === process.env.BRANCH_NAME ? await buildProject() : 202, "Build not required.";
return branchName === process.env.BRANCH_NAME ? await buildProject() : {result: 202, respMessage: "Build not required."};
const getBranchStatus = async (req: express.Request) => {
console.log("Webhook received successfully");
const branchName = req.body?.ref?.split("/").pop();
if (!branchName) {
return {result: 400, respMessage: "Branch name not found in the request."};
}
return branchName === process.env.BRANCH_NAME ? await buildProject() : {result: 202, respMessage: "Build not required."};

};

const isUpdateRequired = () => {
const currentTime = Date.now();
isMindmapUpdated = (currentTime - mindmapBuildTime) / 1000 / 60 > process.env.MINDMAP_UPDATE_TIME_INTERVAL ? true : false;
isDocumentationWebsiteUpdated = (currentTime - documentationWebsiteBuildTime) / 1000 / 60 > process.env.DOCUMENTATION_WEBSITE_UPDATE_TIME_INTERVAL ? true : false;
const currentTime: number = Date.now();
let mindmapUpdateTimeInterval = parseInt(process.env.MINDMAP_UPDATE_TIME_INTERVAL as string)
let docWebsiteUpdateTimeInterval = parseInt(process.env.DOCUMENTATION_WEBSITE_UPDATE_TIME_INTERVAL as string)

isMindmapUpdated = (currentTime - mindmapBuildTime) / 1000 / 60 > mindmapUpdateTimeInterval ? true : false;
Copy link
Copy Markdown
Member

@Idrinth Idrinth May 23, 2024

Choose a reason for hiding this comment

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

remove the ? true : false, it doesn't serve a benefit

isDocumentationWebsiteUpdated = (currentTime - documentationWebsiteBuildTime) / 1000 / 60 > docWebsiteUpdateTimeInterval ? true : false;
Comment on lines +67 to +72
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot May 21, 2024

Choose a reason for hiding this comment

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

Tip

Codebase Verification

CodeRabbit observes the use of environment variables without documentation or validation. Ensure these variables are documented and validated to prevent runtime errors due to missing or incorrect configurations.

  • MINDMAP_UPDATE_TIME_INTERVAL
  • DOCUMENTATION_WEBSITE_UPDATE_TIME_INTERVAL
  • CONTRIBUTORS_UPDATE_TIME_INTERVAL
Analysis chain

CodeRabbit observes the use of environment variables for configuration. Ensure these variables are documented and validated to prevent runtime errors due to missing or incorrect configurations.

Scripts Executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that all required environment variables are documented.

# Test: Search for documentation of environment variables. Expect: Documentation entries for each variable used.
rg --type markdown 'MINDMAP_UPDATE_TIME_INTERVAL|DOCUMENTATION_WEBSITE_UPDATE_TIME_INTERVAL|CONTRIBUTORS_UPDATE_TIME_INTERVAL'

Length of output: 128

Script:

#!/bin/bash
# Description: Verify the usage and validation of environment variables in the codebase.

# Search for the usage of the environment variables in the codebase.
rg 'process\.env\.MINDMAP_UPDATE_TIME_INTERVAL|process\.env\.DOCUMENTATION_WEBSITE_UPDATE_TIME_INTERVAL|process\.env\.CONTRIBUTORS_UPDATE_TIME_INTERVAL'

# Search for validation logic related to these environment variables.
rg 'if\s*\(.*process\.env\.MINDMAP_UPDATE_TIME_INTERVAL.*\)|if\s*\(.*process\.env\.DOCUMENTATION_WEBSITE_UPDATE_TIME_INTERVAL.*\)|if\s*\(.*process\.env\.CONTRIBUTORS_UPDATE_TIME_INTERVAL.*\)'

Length of output: 681

Copy link
Copy Markdown
Member

@Idrinth Idrinth May 23, 2024

Choose a reason for hiding this comment

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

remove the ? true : false, it doesn't serve a benefit

return isMindmapUpdated || isDocumentationWebsiteUpdated;
};

const buildProject = async () => {
const currentTime = Date.now();
if (!isUpdateRequired()) {
if (contributorsBuildRequired || (currentTime - contributorsBuildTime) / 1000 / 60 > process.env.CONTRIBUTORS_UPDATE_TIME_INTERVAL) {
let contUpdateTimeInterval = parseInt(process.env.CONTRIBUTORS_UPDATE_TIME_INTERVAL as string)

Check notice

Code scanning / CodeQL

Semicolon insertion

Avoid automated semicolon insertion (94% of all statements in [the enclosing function](1) have an explicit semicolon).
if (contributorsBuildRequired || (currentTime - contributorsBuildTime) / 1000 / 60 > contUpdateTimeInterval) {
console.log("No update required, updating the contributors only");
await initiateBuild("npm run contributor-build", process.env.DOCUMENTATION_WEBSITE_PATH, process.env.DOCUMENTATION_WEBSITE_DEST_PATH);
await initiateBuild("npm run contributor-build", process.env.DOCUMENTATION_WEBSITE_PATH as string, process.env.DOCUMENTATION_WEBSITE_DEST_PATH as string);
contributorsBuildTime = currentTime;
contributorsBuildRequired = false;
return 200;
return {result: 200, respMessage: ""};
} else {
contributorsBuildRequired = true;
return 202, "Contributors build will be done after the next build.";
return {result: 202, respMessage: "Contributors build will be done after the next build."};
}
}
if (isMindmapUpdated) {
console.log("Building Mindmap");
await initiateBuild("npm run build", process.env.MINDMAP_PATH, process.env.MINDMAP_DEST_PATH);
await initiateBuild("npm run build", process.env.MINDMAP_PATH as string, process.env.MINDMAP_DEST_PATH as string);
mindmapBuildTime = currentTime;
isMindmapUpdated = false;
}

if (isDocumentationWebsiteUpdated) {
console.log("Building Documentation Website");
await initiateBuild("npm run build", process.env.DOCUMENTATION_WEBSITE_PATH, process.env.DOCUMENTATION_WEBSITE_DEST_PATH);
await initiateBuild("npm run build", process.env.DOCUMENTATION_WEBSITE_PATH as string, process.env.DOCUMENTATION_WEBSITE_DEST_PATH as string);
documentationWebsiteBuildTime = currentTime;
contributorsBuildTime = currentTime;
isDocumentationWebsiteUpdated = false;
}

return 200, "Build has been created.";
return {result: 200, respMessage: "Build has been created."};
};

const initiateBuild = async (command, projectPath, destPath) => {
const initiateBuild = async (command: string, projectPath: string, destPath: string) => {
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot May 21, 2024

Choose a reason for hiding this comment

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

CodeRabbit suggests refactoring the initiateBuild function to reduce redundancy and potential errors in command execution paths.

-  await executeCmd(`cd ${projectPath}/ && git pull`);
-  await executeCmd(`cd ${projectPath}/ && npm ci`);
-  await executeCmd(`cd ${projectPath}/ && ${command}`);
-  await executeCmd(`cp -r ${projectPath}/dist/ ${destPath}/`);
+  const commands = ['git pull', 'npm ci', `${command}`, `cp -r dist/ ${destPath}/`];
+  for (const cmd of commands) {
+    await executeCmd(`cd ${projectPath}/ && ${cmd}`);
+  }
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
const initiateBuild = async (command: string, projectPath: string, destPath: string) => {
const initiateBuild = async (command: string, projectPath: string, destPath: string) => {
const commands = ['git pull', 'npm ci', `${command}`, `cp -r dist/ ${destPath}/`];
for (const cmd of commands) {
await executeCmd(`cd ${projectPath}/ && ${cmd}`);
}

await executeCmd(`cd ${projectPath}/ && git pull`);
await executeCmd(`cd ${projectPath}/ && npm ci`);
await executeCmd(`cd ${projectPath}/ && ${command}`);
Expand Down
100 changes: 95 additions & 5 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 5 additions & 3 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
"version": "1.0.0",
"description": "This repository is our website deploy and update tool to minimize github api queries.",
"main": "index.js",
"type" : "module",
Copy link
Copy Markdown
Member

@Idrinth Idrinth May 23, 2024

Choose a reason for hiding this comment

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

please restore

"scripts": {
"start": "node index.js",
"lint": "eslint --ext=.ts --debug .",
Expand Down Expand Up @@ -36,9 +35,12 @@
"express": "^4.19.2"
},
"devDependencies": {
"@idrinth-api-bench/eslint-config": "https://github.com/idrinth-api-bench/eslint-config#setup-base-config",
"@commitlint/cli": "^19.3.0",
"simple-git-hooks": "^2.11.1"
"@idrinth-api-bench/eslint-config": "https://github.com/idrinth-api-bench/eslint-config#setup-base-config",
"@types/express": "^4.17.21",
"@types/node": "^20.12.12",
"simple-git-hooks": "^2.11.1",
"typescript": "^5.4.5"
},
"engineStrict": true,
"engines": {
Expand Down
Loading