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

Add check for empty comments and notes #838

Merged
merged 2 commits into from
Jun 10, 2022

Conversation

Ashesh3
Copy link
Member

@Ashesh3 Ashesh3 commented Jun 8, 2022

Builds on coronasafe/care_fe#2525

Adds a check for empty comments and notes from the backend and rejects with proper validation message when needed.

Also some minor formatting change performed automatically by pre-commit-hooks

Copy link
Member

@sainak sainak left a comment

Choose a reason for hiding this comment

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

We can do this without regex

care/facility/api/serializers/patient.py Outdated Show resolved Hide resolved
care/facility/api/serializers/patient.py Outdated Show resolved Hide resolved
@Ashesh3 Ashesh3 requested a review from sainak June 8, 2022 13:19
Copy link
Member

@sainak sainak left a comment

Choose a reason for hiding this comment

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

LGTM

@vigneshhari
Copy link
Member

I would create a new serializer field that has the empty check validator built-in and use that everywhere instead of re-writing the logic multiple times. Create a PR with this or create an issue to create such a field.

Copy link
Member

@vigneshhari vigneshhari left a comment

Choose a reason for hiding this comment

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

Changes look good, Suggestions for the next iteration provided above, Untested.

@vigneshhari vigneshhari merged commit 2bb7eee into coronasafe:master Jun 10, 2022
@Ashesh3 Ashesh3 deleted the fix-empty-notes branch June 10, 2022 18:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants