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.

Sign up for GitHub

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

Jump to bottom

Fixed: Added occupation field for text support #1058

Closed
wants to merge 2 commits into from
Closed

Fixed: Added occupation field for text support #1058

wants to merge 2 commits into from

Conversation

aeswibon
Copy link
Member

@aeswibon aeswibon commented Oct 9, 2022 •
edited
Loading

Proposed Changes

  • Altered the occupation field from IntegerField to CharField

Associated Issue

Associated PRs

@coronasafe/code-reviewers

@aeswibon aeswibon requested a review from a team as a code owner October 9, 2022 04:47
@codecov-commenter
Copy link

codecov-commenter commented Oct 9, 2022 •
edited
Loading

Codecov Report

Base: 56.06% // Head: 56.02% // Decreases project coverage by -0.04% ⚠️

Coverage data is based on head (c4a376a) compared to base (0beaa72).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #1058      +/-   ##
==========================================
- Coverage   56.06%   56.02%   -0.05%     
==========================================
  Files         175      175              
  Lines        8629     8620       -9     
  Branches     1462     1475      +13     
==========================================
- Hits         4838     4829       -9     
  Misses       3739     3739              
  Partials       52       52
Impacted Files Coverage Δ
care/facility/api/serializers/patient.py 55.81% <100.00%> (-0.21%) ⬇️
care/facility/models/patient.py 92.59% <100.00%> (-0.27%) ⬇️
care/users/api/viewsets/change_password.py 61.53% <0.00%> (+1.53%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@nihal467
Copy link
Member

@vigneshhari can you review it

@sainak sainak removed the done label Jul 5, 2023
@aeswibon aeswibon added the done label Jul 20, 2023
sainak
sainak requested changes Jul 20, 2023
View reviewed changes
Copy link
Member

@sainak sainak left a comment •
edited
Loading

Choose a reason for hiding this comment

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

The migrations can be improved

  • create the new occupation field with name occupation_char
  • use the PatientMetaInfo directly to get all objects
  • populate the occupation_char field and add the objects to a list
  • bulk update the PatientMetaInfo objects
  • remove the old occupation field
  • rename the occupation field

@sainak
Copy link
Member

sainak commented Jul 20, 2023

@cp-coder add test cases

@aeswibon
Copy link
Member Author

aeswibon commented Aug 7, 2023

Will open a new PR

@aeswibon aeswibon closed this Aug 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sainak sainak sainak requested changes

@vigneshhari vigneshhari vigneshhari requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add occupation field in patient model
5 participants
@aeswibon @codecov-commenter @nihal467 @sainak @vigneshhari