Skip to content

Conversation

divyanshkhetan
Copy link

Proposed changes

  • Appointments should have prescription in history section.
  • In doctor schedule add emergency contact number.

Types of changes

Put an x in the boxes that apply

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Other (please describe):

Checklist

Put an x in the boxes that apply

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have created new branch for this pull request
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • My changes does not break the current system and it passes all the current test cases.

Screenshots'

image
image

@divyanshkhetan divyanshkhetan changed the title Week4 OS-2 : 4 : Prescription in appointment history and emergency contact number in doctor Feb 11, 2023
@divyanshkhetan divyanshkhetan changed the title OS-2 : 4 : Prescription in appointment history and emergency contact number in doctor OS-2 : 4 : Prescription in appointment history and emergency contact number in doctor's schedule Feb 11, 2023
@divyanshkhetan divyanshkhetan changed the title OS-2 : 4 : Prescription in appointment history and emergency contact number in doctor's schedule OS-2 : Week - 4 : Prescription in appointment history and emergency contact number in doctor's schedule Feb 11, 2023
Copy link

@NuthanReddy646 NuthanReddy646 left a comment

Choose a reason for hiding this comment

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

Approve @akshatnema

@akshatnema akshatnema changed the base branch from os-2 to test March 28, 2023 06:16
@akshatnema
Copy link
Collaborator

@divyanshkhetan Kindly push the migrations of this PR as there is a change in models.py. Review required from @PranshuNayak and @samay-rgb.

@akshatnema
Copy link
Collaborator

@divyanshkhetan Also, resolve the merge conflicts in the PR.

Copy link
Collaborator

@PranshuNayak PranshuNayak left a comment

Choose a reason for hiding this comment

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

Push the migrations for the changes too.

doctor_phone = models.CharField(max_length=10)
specialization = models.CharField(max_length=100)
active = models.BooleanField(default=True)
contact_no = models.IntegerField(default=0)
Copy link
Collaborator

Choose a reason for hiding this comment

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

use a Charfield instead and keep a max_length and min_length 10

@divyanshkhetan
Copy link
Author

@PranshuNayak @samay-rgb please take a look.

doctor_phone = models.CharField(max_length=10)
specialization = models.CharField(max_length=100)
active = models.BooleanField(default=True)
contact_no = models.CharField(max_length=10, validators=[MinLengthValidator(10)], default=0000000000)
Copy link
Collaborator

Choose a reason for hiding this comment

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

The default must be a string for a CharField. Use min_length=10 instead of the validator.

Copy link
Author

@divyanshkhetan divyanshkhetan Apr 6, 2023

Choose a reason for hiding this comment

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

@PranshuNayak
That is used for forms.CharField() and not for models.CharField(). That is why I used a validator.

Copy link
Collaborator

Choose a reason for hiding this comment

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

you need to change the default , it must be string

Copy link
Author

Choose a reason for hiding this comment

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

@samay-rgb PTAL.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Migration file should be updated too.

Copy link
Collaborator

@PranshuNayak PranshuNayak left a comment

Choose a reason for hiding this comment

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

@divyanshkhetan push new migrations, since you have changed the default value for contact_no

@divyanshkhetan
Copy link
Author

@PranshuNayak done.

Copy link
Collaborator

@PranshuNayak PranshuNayak left a comment

Choose a reason for hiding this comment

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

@akshatnema changes in models.py are fine.

@akshatnema akshatnema merged commit 4f6eceb into FusionIIIT:test Apr 10, 2023
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.

5 participants