[WIP] Contact Us API #74
Conversation
| verbose_name_plural = "Contacts" | ||
|
|
||
| def __str__(self): | ||
| return f"Contact {self.id}: {self.email} {self.text} {self.logo_url} {self.name} {self.organization} {self.project_url}" |
There was a problem hiding this comment.
Ummm that's the entire model. Not really best practice. Why are you returning EVERYTHING in str?
| @@ -81,6 +81,21 @@ def __str__(self): | |||
| return f"Alias {self.id}: {self.tag} {self.alias}" | |||
|
|
|||
|
|
|||
| class Contact(models.Model): | |||
There was a problem hiding this comment.
You added a model but where is the migration? You can create it using ./manage.py makemigrations. The name generated automatically may or may not say 'added_contact_model'. If you end up with on with a name like "0004_auto_" please rename it to be more descriptive.
I am puzzled by the failing tests in this PR. Hopefully adding a migration file will fix - or at least give us a more understandable error.
| @@ -81,6 +81,21 @@ def __str__(self): | |||
| return f"Alias {self.id}: {self.tag} {self.alias}" | |||
|
|
|||
|
|
|||
| class Contact(models.Model): | |||
| email = models.CharField(max_length=128, unique=True) | |||
| text = models.TextField(max_length=4096, null=True, blank=True) | |||
There was a problem hiding this comment.
For TextField and CharFields, best practice is to ONLY use "blank=True" - that way the row will always have an empty string for missing data in that field. If you allow null, then you would have to check for null and '' to decide if you don't have a value.
|
@emecas I went to the CTI meeting 7/29. The contact information form is going to take information in but won't be displaying the information anywhere. So rather than using ReadOnlyModelViewSet in your view, you need something that creates records. You can either use a function view like we do in our organization create view: Or you can make a class based view somewhat like this set of docs: Also, for your model, you probably want to be pretty loose in the model validations, e.g. remove all uniqueness constraints and most of the required fields - except email. If we do want to enforce filling in some fields, we can have the Serializer insist you give us project or parent org. |
|
@emecas would you happen to have any updates for this PR? Thanks |
@bruceplai I am not sure if it could be close since civictechindex/CTI-website-frontend#604 is already closed and labeled as Done!! are you aware of any requirement in pending? |
Fix civictechindex/CTI-website-frontend#604