convert to typescript #95

j-chad · 2022-03-23T18:45:58Z

Description

Converts the backend codebase to typescript

Fixes/resolves #90

Type of change

Improvement (non-breaking change which improves existing functionality)

Checklist:

Leave blank if not applicable

I have completed these steps when making this pull request:

I have checked that the PR is from a forked repository
I have assigned my name to the issue
I have moved the issue to the In Progress column
I have labelled the PR appropriately
I have assigned people responsible to the PR

Before opening the PR for review:

My code follows the style guidelines of this project
I have performed a self-review of my code
I have documented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes
I have moved the linked issue to the Review in Progress column

mped822 · 2022-03-23T21:06:54Z

config/db.server.config.ts

+ * @param databaseName the name of the database being connected to
+ * @param isTestDatabase conditional for if the database in use is for testing
+ */
+export function connect(databaseName: string, isTestDatabase: boolean = false): Promise<typeof database> {


It looks like these function parameters are never used

yeah I overwrote some of the code so that it runs locally. Will change this before opening

mped822 · 2022-03-23T21:07:58Z

config/db.server.config.ts

+    if (isTestDatabase) {
+        return database.connection.readyState;
+    } else {
+        return database.connection.readyState;
+    }


This if statement is redundant if we do the same thing

yeah I'm not sure what the thought was there 😆

fishmonger45

looks fine though the tests fail all fail on me. but i probably haven't set something up properly. i'll report back.
well done on keeping the conversion unopinionated!

fishmonger45 · 2022-03-24T02:08:20Z

package.json

+    "@types/convict": "^6.1.1",
+    "@types/express": "^4.17.13",
+    "@types/mocha": "^9.1.0",
+    "@types/mongoose": "^5.11.97",


mongoose has it's own types afaik, so don't need this?

fishmonger45 · 2022-03-24T02:15:42Z

config/express.server.config.ts

+    dbServerRoutes(app);
+    forumServerRoutes(app);
+    userServerRoutes(app);


is dbServerRoutes etc usually put into a reexports file so that you can do the imports on a single line? eg: as a file routes/routes.module.ts

fishmonger45 · 2022-03-24T02:17:37Z

lib/middleware.lib.ts

    User.searchUserByAuthToken(authToken, function (result) {
-        if (result.status !== 200) {
-            return res.sendStatus(result.status).message(result.message);
+        console.log(result)


extra log here. noting just in case you missed; probably on purpose because authtoken stuff

~~yeah this was in there before~~. I will remove it

fishmonger45 · 2022-03-24T02:18:59Z

lib/validate.lib.ts

+export function isValidDocumentID(id) {
    const regExpDocumentID = new RegExp('^[a-fA-F0-9]{24}$');
    return id && typeof id === 'string' && id.length === 24 && regExpDocumentID.test(id);
 }


goodness... we can probably kill some of this validate stuff w/ types when we refactor

fishmonger45 · 2022-03-24T02:20:47Z

models/forum.server.model.ts

+      })
+      .catch((err) => {
+          // Forum post is already in the database with unique attributes, return duplicate conflict error
+          if (err.code === 11000) {


this code seems like magic to me; what is this error code?

it's a mongodb error code. I think we should fix this in another ticket later

fishmonger45 · 2022-03-24T02:22:35Z

models/forum.server.model.ts

+ * @param params object containing forum post attributes
+ * @param done function callback, returns status code, and message if error, or JSON if successful
+ */
+export function insertPost (params, done) {


we should run prettier over the project

I'll add this as part of #93

fishmonger45 · 2022-03-24T02:24:04Z

models/user.server.model.ts

-                });
+              .then((res) => {
+                  if (res.hashedPassword.match(hashPassword(plaintextPassword))) {
+                      return done({status: 200, res})


should replace these magics with the enums if avail.

fishmonger45 · 2022-03-24T02:25:24Z

server.ts


 // Database name - use environment variable DATABASE_NAME or enter here
-const databaseName = process.env.DATABASE_NAME;
+const databaseName = process.env.DATABASE_NAME as string;


doesn't seem to be used & don't envvars always cast as string? (is the explicit cast required?)

j-chad · 2022-03-24T02:55:31Z

the tests are very very broken it's not just you

mped822 · 2022-03-24T03:52:28Z

models/forum.server.model.ts

+            updates['comments'] = updates.comments ? result.comments.concat(updates.comments) : result.comments
+
+            // If the update does not just involve up votes, down votes, or comments, and actual edits
+            if (Object.keys(updates).length > 3) {


Can we make this magic number a constant

Let's not focus on all the unrelated issues right now, there are more than enough of them to fix later. If we try fix them now we cant parallelise efficiently

models/forum.server.model.ts

mped822 · 2022-03-24T04:02:26Z

test/db.connection.test.ts

+ */
+describe("Database connection test", function() {
+    it("should connect to and disconnect from the database with correct statuses", async function() {
+            const testDatabaseName = process.env.DATABASE_TEST_NAME;


This variable looks like it is never used

yeah as per our meeting we will be ignoring tests until they can be reviewed and fixed

they're very broken and dependant on a prod database. I have started some initial work on fixing them but we want to get this in first

yikes. thats good and makes sense though. thanks for the heads up

mped822

lgtm

j-chad added 6 commits March 23, 2022 00:25

add initial typings

8389e45

remove tsconfig comments

9d7c765

fix test errors

c09f5a5

fix model errors

152b99a

migrate schemas to typescript

53821d0

migrate models

1e312cc

j-chad added the enhancement New feature or request label Mar 23, 2022

j-chad self-assigned this Mar 23, 2022

j-chad added 3 commits March 24, 2022 09:27

migrate more files

51ed842

migrate controllers

3c04b70

change typings to mocha

f2e793a

mped822 reviewed Mar 23, 2022

View reviewed changes

j-chad added 3 commits March 24, 2022 10:37

remove unused db params

d69ba99

change database connection interface

88a6df8

add in-memory database

24336d9

fishmonger45 reviewed Mar 24, 2022

View reviewed changes

fix some auth tests

7b1e0a2

j-chad marked this pull request as ready for review March 24, 2022 03:20

j-chad requested a review from a team as a code owner March 24, 2022 03:20

j-chad requested review from kaylward and removed request for a team March 24, 2022 03:20

j-chad added 3 commits March 24, 2022 03:20

Merge branch 'main' into 90-typescript

c6cf25b

update port config

5a474e1

remove erroneous log

3738e13

mped822 reviewed Mar 24, 2022

View reviewed changes

models/forum.server.model.ts Show resolved Hide resolved

mped822 reviewed Mar 24, 2022

View reviewed changes

mped822 approved these changes Mar 24, 2022

View reviewed changes

j-chad merged commit 7cf1a6c into SE701-T5:main Mar 24, 2022

j-chad deleted the 90-typescript branch March 24, 2022 04:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

convert to typescript #95

convert to typescript #95

j-chad commented Mar 23, 2022

mped822 Mar 23, 2022

j-chad Mar 23, 2022

mped822 Mar 23, 2022

j-chad Mar 23, 2022

fishmonger45 left a comment •

edited

Loading

fishmonger45 Mar 24, 2022 •

edited

Loading

fishmonger45 Mar 24, 2022

fishmonger45 Mar 24, 2022

j-chad Mar 24, 2022 •

edited

Loading

fishmonger45 Mar 24, 2022

j-chad Mar 24, 2022

fishmonger45 Mar 24, 2022 •

edited

Loading

j-chad Mar 24, 2022

fishmonger45 Mar 24, 2022

j-chad Mar 24, 2022

fishmonger45 Mar 24, 2022

fishmonger45 Mar 24, 2022 •

edited

Loading

j-chad commented Mar 24, 2022

mped822 Mar 24, 2022

j-chad Mar 24, 2022 •

edited

Loading

mped822 Mar 24, 2022

j-chad Mar 24, 2022

mped822 Mar 24, 2022

j-chad Mar 24, 2022

mped822 Mar 24, 2022

mped822 left a comment

convert to typescript #95

convert to typescript #95

Conversation

j-chad commented Mar 23, 2022

Description

Type of change

Checklist:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fishmonger45 left a comment • edited Loading

Choose a reason for hiding this comment

fishmonger45 Mar 24, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

j-chad Mar 24, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fishmonger45 Mar 24, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fishmonger45 Mar 24, 2022 • edited Loading

Choose a reason for hiding this comment

j-chad commented Mar 24, 2022

Choose a reason for hiding this comment

j-chad Mar 24, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mped822 left a comment

Choose a reason for hiding this comment

fishmonger45 left a comment •

edited

Loading

fishmonger45 Mar 24, 2022 •

edited

Loading

j-chad Mar 24, 2022 •

edited

Loading

fishmonger45 Mar 24, 2022 •

edited

Loading

fishmonger45 Mar 24, 2022 •

edited

Loading

j-chad Mar 24, 2022 •

edited

Loading