DRY common response object database fields #107
Conversation
MattF-NSIDC
changed the title
| contact_title = Column(String(256), nullable=True) | ||
| contact_email = Column(String(256), nullable=False) | ||
| # how do i make it a minimum of 3 tags? | ||
| tags = Column(String, nullable=False) |
There was a problem hiding this comment.
@rmarow I'm not sure I understand this. I looked in the glossary and didn't see anything about a minimum number of tags. I think this should be an Array column (sqlalchemy)!
There was a problem hiding this comment.
If this is hard to code, we can just add explanatory text that says "recommend three or more tags." Sandy had suggested this last week, and I added a note in the front-end document, which @rmarow noted. I think the explanatory text is a good way to go. If we do some testing and folks aren't responding to that request, we can adjust.
There was a problem hiding this comment.
I agree, let's go with explanatory text and treat the actual validation as icing for later :)
usaon_vta_survey/forms.py
Outdated
| only=['name', 'performance_rating'], | ||
| only=[ | ||
| 'short_name', | ||
| 'full_name', |
There was a problem hiding this comment.
@MattF-NSIDC is there a way to reduce the repetitiveness here?
There was a problem hiding this comment.
We could save these as a re-usable list of strings!
usaon_vta_survey/forms.py
Outdated
| @@ -29,6 +29,21 @@ | |||
| User, | |||
| ) | |||
|
|
|||
| response_object_fields = [ | |||
There was a problem hiding this comment.
maybe common_response_object_fields?
There was a problem hiding this comment.
Also, is there a way we can generate this list from the mixin class? There are many ways to get the "fields" out of a class, but they're all pretty messy. We could make ResponseObjectFieldMixin a pydantic class, and then I think we have more intuitive access to its fields. Or we could use one of these methods:
https://stackoverflow.com/questions/9058305/getting-attributes-of-a-class
I don't feel we need to address this in this PR, except to leave a comment for future us that some DRYing needs to be done.
| @@ -213,14 +226,6 @@ class ResponseObservingSystem(BaseModel, IORelationshipMixin): | |||
| 'polymorphic_on': type, | |||
| } | |||
There was a problem hiding this comment.
id and response_id are also common fields. Can those go in the mixin as well?
There was a problem hiding this comment.
Sqlalchemy did not like when i tried to do this.
MattF-NSIDC
changed the title
resolves #103