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

Seed clinician messages #142

Merged
merged 3 commits into from
Oct 1, 2024
Merged

Conversation

arkadiuszbachorski
Copy link
Contributor

Seed clinician messages

♻️ Current situation & Problem

As a part of dev process, it's great to see all possible messages. This is very minimal implementation of clinician messages seed.

Code of Conduct & Contributing Guidelines

By submitting creating this pull request, you agree to follow our Code of Conduct and Contributing Guidelines:

Copy link
Collaborator

@pauljohanneskraft pauljohanneskraft left a comment

Choose a reason for hiding this comment

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

This is fine for now, but I would actually prefer an implementation where only the assigned clinician gets the messages of the patient, so that it reflects actual behavior - we can either implement it here in that form (e.g. by also defining assigned clinicians in the seeding data already) - or we postpone it in a separate issue - you decide 😊

@arkadiuszbachorski
Copy link
Contributor Author

This is fine for now, but I would actually prefer an implementation where only the assigned clinician gets the messages of the patient, so that it reflects actual behavior - we can either implement it here in that form (e.g. by also defining assigned clinicians in the seeding data already) - or we postpone it in a separate issue - you decide 😊

Agree, it would be great for this to reflect actual app behavior as close as possible. I am on it, let's not merge it yet!

@pauljohanneskraft
Copy link
Collaborator

Agree, it would be great for this to reflect actual app behavior as close as possible. I am on it, let's not merge it yet!

Thank you very much!

@arkadiuszbachorski
Copy link
Contributor Author

@pauljohanneskraft Add clinician-patient relationship to seeding

Now, clinician receives notifications for his own patients only. Some clinicians have multiple patients, some have just one, I've added some "chaos" there.

@pauljohanneskraft
Copy link
Collaborator

Thank you very much for these changes - There is also a way to just seed certain users and currently that one just calls _seedPatientCollections - do you have an idea, how we could incorporate that as well? We could just fetch the user right before calling the function and checking its type for example?

Copy link
Collaborator

@pauljohanneskraft pauljohanneskraft left a comment

Choose a reason for hiding this comment

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

Minor remark about possibly adapting this one case, where we would just seed any user with patient data when specifically seeding certain userIds. It may be out of scope of this PR though, so feel free to also merge directly, if preferred.

@arkadiuszbachorski
Copy link
Contributor Author

Minor remark about possibly adapting this one case, where we would just seed any user with patient data when specifically seeding certain userIds. It may be out of scope of this PR though, so feel free to also merge directly, if preferred.

I created a separate issue for that. #147

@arkadiuszbachorski arkadiuszbachorski merged commit 488c829 into main Oct 1, 2024
6 checks passed
@arkadiuszbachorski arkadiuszbachorski deleted the arek/add-clinician-seeding branch October 1, 2024 16: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
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants