Skip to content
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

Fixing our build #239

Merged
merged 35 commits into from
Aug 29, 2023
Merged

Fixing our build #239

merged 35 commits into from
Aug 29, 2023

Conversation

ermish
Copy link
Collaborator

@ermish ermish commented Aug 28, 2023

Resolves #235
Resolves #236
Resolves #237
Resolves #238

#This PR fixes the current production build

What changed

  • Fixed the build which required many prerequisite fixes listed above.
  • This is a first step towards getting our CI/CD pipeline up and running!
  • The fake data and changes in services aren't perfect. It's just a starting point 😄 feel free to update
  • updated yarn to the latest version.

Testing instructions

  • run yarn prod:build and see no errors

ermish and others added 30 commits April 28, 2023 13:39
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Philip Ermish <[email protected]>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Philip Ermish <[email protected]>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: LaShawn Toyoda <[email protected]>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: LaShawn Toyoda <[email protected]>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: LaShawn Toyoda <[email protected]>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
* fix: add firebase service account key to env variable

Prior to this change, a new developer starting server would get an error message saying private key is missing. Installed dotenv to handle environment variables in the code. Updated README with instructions on how to get a private key from firebase and how to add it to the local env file.

* docs: Add instructions for setting up Firebase Service Account
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
The reason for this change is to allow users to submit incomplete information to be reviewed by moderators prior to being searchable on the website
* feat: implement mutation for addFacility

* feat: implement mutation for addHealthcareProfessional

* refactor: fix healthcareProfessionalService types

* refactor: fix types and move function
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
* chore: update healthcareProfessional service

* feat: add new Facility with HealthcareProfessional Ids

* refactor: clean up imports

* feat: add the isDeleted field to Facility and HealthcareProfessional
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: LaShawn Toyoda <[email protected]>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: LaShawn Toyoda <[email protected]>
)

Previously there were no service functions to allow users to get, add, or update submissions. There are now functions to perform these tasks. There is now also a Submission type alias in TypeDefs/dbschema.ts
* feat: implement end-to-end testing with Jest and Supertest

Prior to this commit the tests that were in place were legacy tests for when our tech stack included Prisma and Postgres. Now that we're using Firestore they're no longer needed.

* feat: add GitHub action to run tests

* feat: add mock firebase config for jest

Co-authored-by: Anissa Chadouli [email protected]

* docs: update testing instructions

* test: happy path is successful

* refactor: separate env variables for dev and prod

Co-authored-by: Philip Ermish <[email protected]>

* chore: fix docker in github actions

Co-authored-by: Philip Ermish <[email protected]>

* chore: fix docker in github actions

Co-authored-by: Philip Ermish <[email protected]>

* chore: fix yarn path

Co-authored-by: Philip Ermish <[email protected]>

* chore: check in dev and prod env

Co-authored-by: Philip Ermish <[email protected]>

* Update README.md

* Update __tests__/server.test.ts

* Update src/index.ts

---------

Co-authored-by: Philip Ermish <[email protected]>
@ermish ermish added the enhancement New feature or request label Aug 28, 2023
@ermish ermish self-assigned this Aug 28, 2023
@socket-security
Copy link

Removed dependencies detected. Learn more about Socket for GitHub ↗︎

🚮 Removed packages: @apollo/[email protected], @types/[email protected], [email protected], [email protected], [email protected], [email protected]

@theyokohamalife
Copy link
Collaborator

theyokohamalife commented Aug 28, 2023

@ermish When I run try to run yarn install or yarn prod:build it gives the following error. This is with node 20.5.1.

YAMLException: bad indentation of a mapping entry at line 366, column 23:
        "@babel/highlight": ^7.22.5
                          ^
        "@babel/highlight": ^7.22.5
                          ^

Copy link
Collaborator

@theyokohamalife theyokohamalife left a comment

Choose a reason for hiding this comment

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

Off to a good start! ✨ Let's separate these into different PRs since they address very different issues. Also, let's make sure that the fake data is correct before merging. We have more active members in the repo now with various skill levels and we want to make sure that everything that gets merged into main is functional.

@theyokohamalife
Copy link
Collaborator

Also, you might want to rebase because it's showing 33 commits.

@theyokohamalife
Copy link
Collaborator

@ermish Don't forget to include descriptive PR titles.

Comment on lines +1 to +26
import { Facility } from '../typeDefs/gqlTypes'

export const fakeFacilities = () => {
const facility : Facility = {
nameEn: 'Zoo',
nameJa: '動物園',
contact: {
address: {
// generate fake data from type PhysicalAddress in gqlTypes.ts file
addressLine1En: '1-1-1',
addressLine2En: 'Ueno',
cityEn: 'Taito',
prefectureEn: 'Tokyo',
postalCode: '100-0000'
},
email: '[email protected]',
phone: '08000000000',
website: 'https://zoo.test.com',
mapsLink: ''
},
healthcareProfessionalIds: [],
healthcareProfessionals: []
}

return [facility]
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

This file was removed in #227 because it wasn't being used. Is there a reason we need to reintroduce the fake data?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, we want fake data.

  1. this will be used for when we want to test endpoints by adding data, updating it, then deleting it.
  2. we will also want seed data for when we load up a fake/test database so the site isn't empty.

Keep in mind this is just a starting point and we'll need to update this iteratively.

Comment on lines +1 to +80
import { HealthcareProfessional, Locale, LocaleName, Degree,
Specialty, SpecialtyName, SpokenLanguage, Insurance } from '../typeDefs/gqlTypes'

export const fakeHealthcareProfessionals = () => {
const doctorDoctor : LocaleName = {
lastName: 'Doctor',
firstName: 'Doctor',
middleName: 'MD',
locale: Locale.English
}
const medicalDegree : Degree = {
nameJa: 'メヂカル',
nameEn: 'Medical',
abbreviation: 'MD'
}

const japanese : SpokenLanguage = {
iso639_3: 'ja',
nameJa: '日本語',
nameEn: 'Japanese',
nameNative: 'Japanese'
}

const neurologyEn : SpecialtyName = {
name: 'Neurology',
locale: Locale.English
}

const neurology : Specialty = {
names: [neurologyEn]
}

const healthcareProfessionalOne : HealthcareProfessional = {
names: [doctorDoctor],
degrees: [medicalDegree],
spokenLanguages: [japanese],
specialties: [neurology],
acceptedInsurance: [Insurance.InternationalHealthInsurance],
isDeleted: false
}

const name : LocaleName = {
lastName: 'チェ',
firstName: 'ジェイコブ',
middleName: 'ベイヤード',
locale: Locale.Japanese
}
const englishDegree : Degree = {
nameJa: '英語',
nameEn: 'English',
abbreviation: 'En'
}

const english : SpokenLanguage = {
iso639_3: 'en-US',
nameJa: '英語',
nameEn: 'English',
nameNative: 'English'
}

const specialtyName : SpecialtyName = {
name: 'Pandas',
locale: Locale.English
}

const pandaSpecialty : Specialty = {
names: [specialtyName]
}

const healthcareProfessionalTwo : HealthcareProfessional = {
names: [name],
degrees: [englishDegree],
spokenLanguages: [english],
specialties: [pandaSpecialty],
acceptedInsurance: [Insurance.InternationalHealthInsurance],
isDeleted: false
}

return [healthcareProfessionalOne, healthcareProfessionalTwo]
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

This file was also removed in #227

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah, I realized that we removed it. We shouldn't have 😅

Copy link
Collaborator

Choose a reason for hiding this comment

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

@ermish I asked this in the issue, but I'll ask it again here. What can this fake data do that auto-mocking from the schema can't? It seems like extra work to manually seed the db with files that will have to be updated and maintained.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

answered in the previous comment:

yes, we want fake data.
this will be used for when we want to test endpoints by adding data, updating it, then deleting it.
we will also want seed data for when we load up a fake/test database so the site isn't empty.
Keep in mind this is just a starting point and we'll need to update this iteratively.

import { addHealthcareProfessional } from './services/healthcareProfessionalService'
import { addFacility } from './services/facilityService'

import { fakeHealthcareProfessionals } from './fakeData/healthcareProfessional'
import { fakeHealthcareProfessionals } from './fakeData/healthcareProfessionals'
Copy link
Collaborator

@theyokohamalife theyokohamalife Aug 28, 2023

Choose a reason for hiding this comment

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

I think it's better to use mock data that's autogenerated instead of seed files that will have to be updated and maintained when schema changes are made. What are your thoughts?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

for tests, yes, but we're likely going to want initial seed data to have the site functional beyond testing a single endpoint. So basically, we'll want both.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Perhaps we should use real examples then?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

that's what these are 😆

@ermish ermish changed the title Ermish/cicd Fixing our build Aug 28, 2023
@ermish
Copy link
Collaborator Author

ermish commented Aug 28, 2023

Off to a good start! ✨ Let's separate these into different PRs since they address very different issues. Also, let's make sure that the fake data is correct before merging. We have more active members in the repo now with various skill levels and we want to make sure that everything that gets merged into main is functional.

I'd like to keep it together as these were the changes needed to get the build working. The only part that is extra is the package updates, but it's part of my work to get the build working with up to date packages

@ermish
Copy link
Collaborator Author

ermish commented Aug 28, 2023

Also, you might want to rebase because it's showing 33 commits.

yup! I might put this in draft as it was working, but I rebased and have to fix it again. I'll ping you when it's actually ready to be tested.

@theyokohamalife theyokohamalife marked this pull request as draft August 28, 2023 13:01
@ermish
Copy link
Collaborator Author

ermish commented Aug 28, 2023

ok @theyokohamalife, build works again! please check

@theyokohamalife
Copy link
Collaborator

ok @theyokohamalife, build works again! please check

@ermish I get the following error after your latest commit:

Screenshot 2023-08-28 at 10 20 19 PM

@ermish
Copy link
Collaborator Author

ermish commented Aug 28, 2023

ok @theyokohamalife, build works again! please check

@ermish I get the following error after your latest commit:

Screenshot 2023-08-28 at 10 20 19 PM

hmmm I don't think you have the latest commit. It fixes that. 🤔

@theyokohamalife
Copy link
Collaborator

theyokohamalife commented Aug 28, 2023

hmmm I don't think you have the latest commit. It fixes that. 🤔

@ermish You're right. Just pulled again and it built without errors. 👍🏾

I think this is good to go after you get CI to pass.

@ermish
Copy link
Collaborator Author

ermish commented Aug 29, 2023

hmmm I don't think you have the latest commit. It fixes that. 🤔

@ermish You're right. Just pulled again and it built without errors. 👍🏾

I think this is good to go after you get CI to pass.

CI Passed! It's just the in progress workflows that are failing. I can comment them out I guess to have it pass

@ermish
Copy link
Collaborator Author

ermish commented Aug 29, 2023

ok @theyokohamalife it's ready, please approve!

@ermish ermish marked this pull request as ready for review August 29, 2023 04:57
Copy link
Collaborator

@theyokohamalife theyokohamalife left a comment

Choose a reason for hiding this comment

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

LGTM! Nice work!

@theyokohamalife theyokohamalife merged commit 99576b1 into main Aug 29, 2023
6 checks passed
@theyokohamalife theyokohamalife deleted the ermish/cicd branch August 29, 2023 06:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
No open projects
Archived in project
3 participants