Skip to content

The cancer is gone -- phase 1#38

Open
justinrporter wants to merge 5 commits intomasterfrom
alt-phone
Open

The cancer is gone -- phase 1#38
justinrporter wants to merge 5 commits intomasterfrom
alt-phone

Conversation

@justinrporter
Copy link
Contributor

@justinrporter justinrporter commented Jan 18, 2021

Phase 1 generates a new class, PatientPhoneNumber, that represents a phone number for a patient. Adding phone numbers is now done with a '+' sign on the patient detail view.

Phase 2 will actually remove the old phone numbers, after we copy everything over using the python shell.

  • Add PatientPhoneNumber with link to Patient object
  • Add phone_number to User
  • Add PatientPhoneForm,
  • Add package phonenumber_field for handling phone numbers.
  • PHONENUMBER_DEFAULT_REGION controls default country code
  • modify local.yml to add delegated, cached maybe improving MacOS performance?

@justinrporter justinrporter added the enhancement New feature or request label Jan 18, 2021
@justinrporter justinrporter self-assigned this Jan 18, 2021
@justinrporter justinrporter changed the title The cancer is gone The cancer is gone -- phase 1 Jan 18, 2021
@justinrporter justinrporter requested a review from wwick January 18, 2021 05:14
- local_postgres_data:/var/lib/postgresql/data
- local_postgres_data_backups:/backups
- local_postgres_data:/var/lib/postgresql/data:cached
- local_postgres_data_backups:/backups:cached
Copy link
Member

Choose a reason for hiding this comment

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

How often is the backup updated. Does it make sense to cache it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I'm totally honest, I have no idea. According to that article, the difference should be imperceptible to the user. I haven't really pushed on it that much but after I did this (and restarted Docker) it really improved the performance.

Copy link
Member

Choose a reason for hiding this comment

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

I'm going to say to uncache the backups but keep postgresql cached, since the backup isn't read from very often.

@zhouminerva
Copy link
Member

Hahahahahahaha nice. Looks great

Copy link
Member

@wwick wwick left a comment

Choose a reason for hiding this comment

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

I like these changes a lot

- local_postgres_data:/var/lib/postgresql/data
- local_postgres_data_backups:/backups
- local_postgres_data:/var/lib/postgresql/data:cached
- local_postgres_data_backups:/backups:cached
Copy link
Member

Choose a reason for hiding this comment

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

I'm going to say to uncache the backups but keep postgresql cached, since the backup isn't read from very often.

@wwick
Copy link
Member

wwick commented Jan 29, 2021

I want to wait to merge this until phase 2 is done so that there isn't confusion about duplicate phone number fields

@justinrporter
Copy link
Contributor Author

Totally agree

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

None yet

Development

Successfully merging this pull request may close these issues.

3 participants