Skip to content

Conversation

@Tony9984
Copy link
Contributor

@Tony9984 Tony9984 commented Oct 16, 2020

Tickets:

  • HCK-

List of changes:

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

  • Add "previousHackathons" field to model, updated application validation, tests, route documentation, etc. to reflect changes

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • New release
  • This change requires a documentation update

How has this been tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • Test A
  • Test B

Test Configuration:

Firmware version:
Hardware:
Toolchain:
SDK:

Questions for code reviewers?

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • Listed change(s) in the Changelog
  • 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 made corresponding changes to the documentation
  • Any dependent changes have been merged and published in downstream modules

@Tony9984 Tony9984 requested a review from logan-r October 16, 2020 23:00
@Tony9984 Tony9984 linked an issue Oct 16, 2020 that may be closed by this pull request
Copy link
Member

@logan-r logan-r left a comment

Choose a reason for hiding this comment

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

Looks good! Haven't run locally tho, can you confirm that:

a) Tests pass
b) You can POST localhost:3000/api/hacker with

{
    "accountId": "[AN EXISTING ACCOUNT ID]",
    "application":{
        "general":{
            "school": "McGill University",
            "degree": "Undergraduate",
            "fieldOfStudy": ["Computer Science"],
            "graduationYear": 2021,
            "jobInterest":"Internship",
            "URL":{
                "resume":"resumes/1543458163426-5bff4d736f86be0a41badb91",
                "github":"https://github.com/abcd",
                "dropler":"https://dribbble.com/abcd",
                "personal":"https://www.hi.com/",
                "linkedIn":"https://linkedin.com/in/abcd",
                "other":"https://github.com/hackmcgill/hackerAPI/issues/168"
            }
        },
        "shortAnswer": {
            "skills":["Javascript","Typescript"],
            "question1": "I love McHacks",
            "question2":"Pls accept me",
            "comments":"hi!",
            "previousHackathons": "0"
        },
        "other": {
            "gender": "male",
            "ethnicity": ["Asian or Pacific Islander"],
            "privacyPolicy": true,
            "codeOfConduct": true
        },
        "accommodation": {
            "travel": 0,
            "shirtSize": "S"
        }
    }
}

and it shows up in mongo

c) POSTing localhost:3000/api/hacker with

{
    "accountId": "[AN EXISTING ACCOUNT ID]",
    "application":{
        "general":{
            "school": "McGill University",
            "degree": "Undergraduate",
            "fieldOfStudy": ["Computer Science"],
            "graduationYear": 2021,
            "jobInterest":"Internship",
            "URL":{
                "resume":"resumes/1543458163426-5bff4d736f86be0a41badb91",
                "github":"https://github.com/abcd",
                "dropler":"https://dribbble.com/abcd",
                "personal":"https://www.hi.com/",
                "linkedIn":"https://linkedin.com/in/abcd",
                "other":"https://github.com/hackmcgill/hackerAPI/issues/168"
            }
        },
        "shortAnswer": {
            "skills":["Javascript","Typescript"],
            "question1": "I love McHacks",
            "question2":"Pls accept me",
            "comments":"hi!"
        },
        "other": {
            "gender": "male",
            "ethnicity": ["Asian or Pacific Islander"],
            "privacyPolicy": true,
            "codeOfConduct": true
        },
        "accommodation": {
            "travel": 0,
            "shirtSize": "S"
        }
    }
}

fails as it's missing previousHackathons

return false;
}
if (!!model.searchableField(value)) {
if (model.searchableField(value)) {
Copy link
Member

Choose a reason for hiding this comment

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

Was this a boolean cast? !! is odd in an if, should be casting already, just want to make sure we don't break anything here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not sure, i think the lint took it off by itself, should i restore it to !!?

"name": "hackerAPI",
"version": "3.0.0",
"private": true,
"scripts": {
Copy link
Member

Choose a reason for hiding this comment

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

Can you exclude this file from the commit?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah sure! mybad haha

@Tony9984
Copy link
Contributor Author

Tony9984 commented Oct 18, 2020

Looks good! Haven't run locally tho, can you confirm that:

a) Tests pass
b) You can POST localhost:3000/api/hacker with

{
    "accountId": "[AN EXISTING ACCOUNT ID]",
    "application":{
        "general":{
            "school": "McGill University",
            "degree": "Undergraduate",
            "fieldOfStudy": ["Computer Science"],
            "graduationYear": 2021,
            "jobInterest":"Internship",
            "URL":{
                "resume":"resumes/1543458163426-5bff4d736f86be0a41badb91",
                "github":"https://github.com/abcd",
                "dropler":"https://dribbble.com/abcd",
                "personal":"https://www.hi.com/",
                "linkedIn":"https://linkedin.com/in/abcd",
                "other":"https://github.com/hackmcgill/hackerAPI/issues/168"
            }
        },
        "shortAnswer": {
            "skills":["Javascript","Typescript"],
            "question1": "I love McHacks",
            "question2":"Pls accept me",
            "comments":"hi!",
            "previousHackathons": "0"
        },
        "other": {
            "gender": "male",
            "ethnicity": ["Asian or Pacific Islander"],
            "privacyPolicy": true,
            "codeOfConduct": true
        },
        "accommodation": {
            "travel": 0,
            "shirtSize": "S"
        }
    }
}

and it shows up in mongo

c) POSTing localhost:3000/api/hacker with

{
    "accountId": "[AN EXISTING ACCOUNT ID]",
    "application":{
        "general":{
            "school": "McGill University",
            "degree": "Undergraduate",
            "fieldOfStudy": ["Computer Science"],
            "graduationYear": 2021,
            "jobInterest":"Internship",
            "URL":{
                "resume":"resumes/1543458163426-5bff4d736f86be0a41badb91",
                "github":"https://github.com/abcd",
                "dropler":"https://dribbble.com/abcd",
                "personal":"https://www.hi.com/",
                "linkedIn":"https://linkedin.com/in/abcd",
                "other":"https://github.com/hackmcgill/hackerAPI/issues/168"
            }
        },
        "shortAnswer": {
            "skills":["Javascript","Typescript"],
            "question1": "I love McHacks",
            "question2":"Pls accept me",
            "comments":"hi!"
        },
        "other": {
            "gender": "male",
            "ethnicity": ["Asian or Pacific Islander"],
            "privacyPolicy": true,
            "codeOfConduct": true
        },
        "accommodation": {
            "travel": 0,
            "shirtSize": "S"
        }
    }
}

fails as it's missing previousHackathons

got it! here's what i have:

a) all tests pass! except for the 2 that i sent you on slack

b) returns a 500, debugging it right now
UPDATE: I believe it is caused by the 2 failing tests, which relates to the creation of a hacker when applications have closed/not opened. Let me know what you guys think
image

c) works as intended! 422 error, with previousHackathons as missing field

@pierreTklein
Copy link
Member

Are you able to debug the 500 error?

@logan-r
Copy link
Member

logan-r commented Oct 19, 2020

Ran tests on latest dev code & tests pass, meaning contrary to my thinking earlier on slack, this is a bug introduced by this PR

@Tony9984 I'm finishing up a midterm today, can help debug tomorrow if this is still an issue

image

@Tony9984
Copy link
Contributor Author

Are you able to debug the 500 error?

@Tony9984 Tony9984 closed this Oct 19, 2020
@Tony9984 Tony9984 reopened this Oct 19, 2020
@Tony9984
Copy link
Contributor Author

hey guys! thanks for the updates! tried debugging it, couldn't find the issue, I'll try again tonight after my midterm. @logan-r good luck for yours!! hope everything goes well.

@Tony9984 Tony9984 requested a review from logan-r October 20, 2020 22:54
Copy link
Member

@logan-r logan-r left a comment

Choose a reason for hiding this comment

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

Looks great!!

@logan-r logan-r changed the title Feature/Add previous hackathons field to model feat: add previous hackathons field to application Oct 20, 2020
@Tony9984 Tony9984 merged commit 61266c2 into dev Oct 20, 2020
@Tony9984 Tony9984 deleted the feature/669-Previous_hackathons branch October 20, 2020 23:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add number of previous hackathons field to application

4 participants