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

DRY common response object database fields #107

Merged
merged 13 commits into from
Aug 22, 2023
Merged

DRY common response object database fields #107

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
c2a7b79
Create ObjectMixin and beginning work on applying it
rmarow Aug 18, 2023
f30a7cb
Update macro name
rmarow Aug 21, 2023
5b4ea9d
WIP applying mixin to data product
rmarow Aug 21, 2023
f305228
Merge branch 'main' into update_response_objects
rmarow Aug 21, 2023
8aa1c26
WIP application fields update
rmarow Aug 21, 2023
2124e30
Fix spelling error
rmarow Aug 21, 2023
c0e32cd
Update role_id to role based on form change (bugfix)
rmarow Aug 21, 2023
9cd0a45
Update website_url to just website
rmarow Aug 21, 2023
7b510a8
Update name -> short_name
rmarow Aug 21, 2023
91c4993
Create list for relatioship fields
rmarow Aug 22, 2023
e53c974
Name -> short_name cont.
rmarow Aug 22, 2023
99dc8ec
Final spot for name -> short_name
rmarow Aug 22, 2023
69180f2
Update list of fields
rmarow Aug 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 41 additions & 9 deletions usaon_vta_survey/forms.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,24 +62,56 @@ def conv_String(self, field_args, **extra):
ResponseObservingSystem: model_form(
ResponseObservingSystem,
only=[
'name',
'url',
'author_name',
'author_email',
'short_name',
'full_name',
'organization',
'funder',
'funding_country',
'funding_agency',
'references_citations',
'notes',
'website',
'description',
'contact_name',
'contact_title',
'contact_email',
'tags',
'version',
],
),
# TODO: Restrict "rating" values to correct range
ResponseDataProduct: model_form(
ResponseDataProduct,
only=['name', 'performance_rating'],
only=[
'short_name',
'full_name',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@MattF-NSIDC is there a way to reduce the repetitiveness here?

Copy link
Member

Choose a reason for hiding this comment

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

We could save these as a re-usable list of strings!

'organization',
'funder',
'funding_country',
'website',
'description',
'contact_name',
'contact_title',
'contact_email',
'tags',
'version',
],
),
ResponseApplication: model_form(
ResponseApplication,
only=['name'],
only=[
'short_name',
'full_name',
'organization',
'funder',
'funding_country',
'website',
'description',
'contact_name',
'contact_title',
'contact_email',
'tags',
'version',
'performance_criteria',
'performance_rating',
],
),
ResponseSocietalBenefitArea: model_form(
ResponseSocietalBenefitArea,
Expand Down
43 changes: 26 additions & 17 deletions usaon_vta_survey/models/tables.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,23 @@ def __io__(cls) -> IORelationship:
return io


class ResponseObjectFieldMixin:
rmarow marked this conversation as resolved.
Show resolved Hide resolved
"""Provide shared fields between all relationship objects to reduce repition."""

short_name = Column(String(256), nullable=False)
full_name = Column(String(256), nullable=False)
organization = Column(String(256), nullable=False)
funder = Column(String(256), nullable=False)
rmarow marked this conversation as resolved.
Show resolved Hide resolved
funding_country = Column(String(256), nullable=False)
website = Column(String(256), nullable=True)
description = Column(String(512), nullable=True)
contact_name = Column(String(256), nullable=False)
contact_title = Column(String(256), nullable=True)
contact_email = Column(String(256), nullable=False)
tags = Column(String, nullable=False)
Copy link
Member

Choose a reason for hiding this comment

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

@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)!

Copy link
Collaborator

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

I agree, let's go with explanatory text and treat the actual validation as icing for later :)

version = Column(String(64), nullable=True)


class User(BaseModel, UserMixin):
__tablename__ = 'user'
id = Column(
Expand Down Expand Up @@ -185,7 +202,7 @@ class Response(BaseModel):
)


class ResponseObservingSystem(BaseModel, IORelationshipMixin):
class ResponseObservingSystem(BaseModel, IORelationshipMixin, ResponseObjectFieldMixin):
__tablename__ = 'response_observing_system'
__table_args__ = (UniqueConstraint('name', 'response_id'),)
id = Column(
Expand Down Expand Up @@ -213,14 +230,6 @@ class ResponseObservingSystem(BaseModel, IORelationshipMixin):
'polymorphic_on': type,
}
Copy link
Member

Choose a reason for hiding this comment

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

id and response_id are also common fields. Can those go in the mixin as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sqlalchemy did not like when i tried to do this.


url = Column(String(256), nullable=False)
author_name = Column(String(256), nullable=False)
author_email = Column(String(256), nullable=False)
funding_country = Column(String(256), nullable=False)
funding_agency = Column(String(256), nullable=False)
references_citations = Column(String(512), nullable=False)
notes = Column(String(512), nullable=True)

response = relationship(
'Response',
back_populates='observing_systems',
Expand Down Expand Up @@ -262,7 +271,7 @@ class ResponseObservingSystemResearch(BaseModel):
intermediate_product = Column(String(256), nullable=False)


class ResponseDataProduct(BaseModel, IORelationshipMixin):
class ResponseDataProduct(BaseModel, IORelationshipMixin, ResponseObjectFieldMixin):
__tablename__ = 'response_data_product'
__table_args__ = (UniqueConstraint('name', 'response_id'),)
id = Column(
Expand All @@ -280,12 +289,6 @@ class ResponseDataProduct(BaseModel, IORelationshipMixin):
nullable=False,
)

# TODO: Constrain to 0-100
performance_rating = Column(
SmallInteger,
nullable=False,
)

response = relationship(
'Response',
back_populates='data_products',
Expand All @@ -300,7 +303,7 @@ class ResponseDataProduct(BaseModel, IORelationshipMixin):
)


class ResponseApplication(BaseModel, IORelationshipMixin):
class ResponseApplication(BaseModel, IORelationshipMixin, ResponseObjectFieldMixin):
__tablename__ = 'response_application'
__table_args__ = (UniqueConstraint('name', 'response_id'),)
id = Column(
Expand All @@ -318,6 +321,12 @@ class ResponseApplication(BaseModel, IORelationshipMixin):
nullable=False,
)

performance_criteria = Column(
String(256),
)
# limit 0-100
performance_rating = Column(Integer, nullable=False)

response = relationship(
'Response',
back_populates='applications',
Expand Down
2 changes: 1 addition & 1 deletion usaon_vta_survey/routes/user.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ def load_user(user_id: str) -> User:


def _validate_role_change(user: User, form) -> None:
if not form.data['role_id'] == user.role_id and not current_user.role_id == 'admin':
if not form.data['role'] == user.role_id and not current_user.role_id == 'admin':
raise RuntimeError("Only admins can edit users roles.")


Expand Down
Loading