-
Notifications
You must be signed in to change notification settings - Fork 4
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
FP-35 #37
base: main
Are you sure you want to change the base?
FP-35 #37
Conversation
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.
@Smarsden007 . It was great to check out your code update.
It looks nice and concise. some comments :
- Wondering a couple of questions as sent you trough it.
- Also wondering which jira in FE side is implementing this route.
- Finally wondering about performance once it gets hundreds / thousands entries ?
Thanks !
@@ -7,7 +7,7 @@ const Ethnicity = require('../ethnicities/ethnicitiesModel'); | |||
const EducationHistory = require('../education_histories/educationHistoriesModel'); | |||
const EmailAddress = require('../email_addresses/emailAddressesModel'); | |||
const PhoneNumber = require('../phone_numbers/phoneNumbersModel'); | |||
|
|||
const PlaceHolder = require('./clientModel'); |
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.
Hi @Smarsden007 : wondering why this is needed since you already have a 'Clients' var accessing the same model.
@@ -19,6 +19,39 @@ router.get('/', (req, res) => { | |||
}); | |||
}); | |||
|
|||
router.get('/placeholder', async (req, res) => { | |||
try { | |||
const [ |
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 why some other tables related like 'phone_numbers', 'locations' , 'householders' , 'email_addresses' are missing here
Description
Fixes #
Loom Link of Postman API Call
https://www.loom.com/share/beb43cff5d634c33b7c6826b540bbbb2
Type of change
Please delete options that are not relevant.
Change Status
Has This Been Tested
Checklist