Feature/124 update application page #603
Conversation
|
Congrats on your first PR! Seems like a your PR involves a lot of formatting changes. Please make sure that this isn't the case-- it's basically impossible to tell what logic changes are being made here otherwise! Thanks :). |
| fieldLocation, | ||
| fieldname, | ||
| "invalid string" | ||
| "Invalid String" |
There was a problem hiding this comment.
Nit: Maybe change others as well to capitalize, or leave them all uncapitalized?
| VALIDATOR.stringValidator("body", "degree", false), | ||
| VALIDATOR.stringValidator("body", "gender", false), | ||
| VALIDATOR.booleanValidator("body", "needsBus", false), | ||
| VALIDATOR.stringValidator("body", "application.general.school", false), |
There was a problem hiding this comment.
Does application need to be validated as an object before this?
| !app.portfolioURL.personal || | ||
| typeof app.portfolioURL.personal === "string"; | ||
| !app.general.URL.personal || | ||
| typeof app.general.URL.personal === "string"; |
There was a problem hiding this comment.
should stringValidator be used for these fields now instead, like application.general.url.personal in updateHackerValidator?
tests/hacker.test.js
Outdated
| return done(error); | ||
| } | ||
| let app = TeamHacker0.application; | ||
| app.other.gender = "Other"; |
There was a problem hiding this comment.
Nit: Maybe send application: {other: {gender: "Other"}}} ?
There was a problem hiding this comment.
I agree. We shouldn't mutate the test data.
printharsh
left a comment
printharsh
left a comment
There was a problem hiding this comment.
Awesome! I think just one test is failing for number of female hackers in search
… adjusted hacker validation
loreina
left a comment
loreina
left a comment
There was a problem hiding this comment.
missing: needsBus should be renamed travel under the accommodation section and take in an integer from 0-100
dietaryRestrictions was moved to account creation/profile
package.json
Outdated
| "memory-cache": "^0.2.0", | ||
| "mongoose": "^5.7.5", | ||
| "multer": "^1.4.2", | ||
| "nvm": "0.0.4", |
There was a problem hiding this comment.
nvm should be installed on your system, not in the repo
package.json
Outdated
| "handlebars": "^4.4.3", | ||
| "handlebars": "^4.5.3", | ||
| "jsonwebtoken": "^8.5.1", | ||
| "lodash": "^4.17.15", |
package.json
Outdated
| "handlebars": "^4.5.3", | ||
| "jsonwebtoken": "^8.5.1", | ||
| "lodash": "^4.17.15", | ||
| "lodash.clonedeep": "^4.5.0", |
tests/hacker.test.js
Outdated
| const Hacker = require("../models/hacker.model"); | ||
| const fs = require("fs"); | ||
| const path = require("path"); | ||
| // const cloneDeep = require("lodash/clonedeep"); |
There was a problem hiding this comment.
Is this commented out for future use?
Description
Updated the application page by refactoring the hacker model for the front and backend by grouping together similar properties. Also changed certain fields (i.e. major -> fieldOfStudy) and added new question to essay section.
Fixes # (124)
Type of change
How Has This Been Tested?
Checklist: