-
Notifications
You must be signed in to change notification settings - Fork 1
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
[CHE-17] Create Backend Alumni Routes #45
[CHE-17] Create Backend Alumni Routes #45
Conversation
…lAlumniData controller and basic fetch test on Directory page
import { useAppSelector } from "../../app/hooks"; | ||
import axios from "axios"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wondering if there would be value in creating custom axios instances where we could set reusable configs like baseUrl, withCredentials, headers etc. Could setup interceptors for responses too to simplify error handling
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems interesting to me - Love to chat about error handling overall!
name: { type: String, trim: true, required: true }, | ||
email: { type: String, trim: true, required: true }, | ||
linkedIn: { type: String, trim: true }, | ||
campus: { type: String, trim: true }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could maybe set this up as an enum depending on how we parse the alumni sheet we're working with
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Like use enums for some of the fields? Like campus? Maybe Im misunderstanding. Lets chat!
linkedIn: { type: String, trim: true }, | ||
campus: { type: String, trim: true }, | ||
cohort: { | ||
type: Schema.Types.Mixed, //TODO Better way to do this? Spreadhsete has strings and numbers... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since the only non number are the PTRI 'beta' maybe we could just call them 0 and bypass the Mixed type. But also we should probably think of a way to validate campus+cohort# combos?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Like convert the data as we parse it? That's probably doable but what if Codesmith decides to use that type naming again at some point?
7543e95
into
CHE-15/story/Create-Alumni-Directory
Description
This ticket is part of CHE-15 Create Alumni Directory story and sets up the back end route and controller files including the initial get all alumni data route.
Jira Task
CHE-17
Testing Instructions
git fetch
git checkout CHE-17/subtask/Create-Backend-Alumni-Routes
npm run docker-dev
--> verify core functionality is present and navigation to the Directory pagereceives alumni data on the initial load (it will show in the console log for now)
Checklist
All Team Members
npm run docker-test
in my local environment to check that this PR passes all unit tests.Additional Notes, Images, etc.
N/A