Skip to content

Add check for empty comments and notes#838

Merged
vigneshhari merged 2 commits into
ohcnetwork:masterfrom
Ashesh3:fix-empty-notes
Jun 10, 2022
Merged

Add check for empty comments and notes#838
vigneshhari merged 2 commits into
ohcnetwork:masterfrom
Ashesh3:fix-empty-notes

Conversation

@Ashesh3
Copy link
Copy Markdown
Contributor

@Ashesh3 Ashesh3 commented Jun 8, 2022

Builds on ohcnetwork/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
Copy Markdown
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

Comment thread care/facility/api/serializers/patient.py Outdated
Comment thread care/facility/api/serializers/patient.py Outdated
@Ashesh3 Ashesh3 requested a review from sainak June 8, 2022 13:19
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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 ohcnetwork: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.

3 participants