-
Notifications
You must be signed in to change notification settings - Fork 0
query risk data #367
query risk data #367
Conversation
|
||
class Meta: | ||
model = WeatherEvent | ||
fields = ('name', 'coastalOnly', 'concern', 'indicators', 'displayClass') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To avoid making pointless API calls when using WeatherEventSerializer
as a part of OrganizationRiskSerializer
, consider splitting it into two classes like so:
class WeatherEventSerializer(serializers.ModelSerializer):
coastalOnly = serializers.BooleanField(source='coastal_only')
# Other fields
class Meta:
model = WeatherEvent
fields = ('name', 'coastalOnly', 'indicators', 'displayClass')
class WeatherEventWithConcernSerializer(WeatherEventSerializer):
concern = ConcernSerializer()
class Meta:
fields = ('name', 'coastalOnly', 'concern', 'indicators', 'displayClass')
Then you can continue to use WeatherEventSerializer
in OrganizationRiskSerializer
and use WeatherEventWithConcernSerializer
in the WeatherEventRankSerializer
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you saying we should exclude concerns from the weather events serialized with the organization risks?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes
|
||
constructor(private apiHttp: PlanItApiHttp) {} | ||
|
||
risks(): Observable<Risk[]> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we're going to follow-up to this with delete/create/update methods as well, consider naming this list()
so we can have a consistent naming scheme
To directly serialize all weather events in the system.
Serialze community systems in DRF API.
Serialize related community systems and weather events on risks DRF API endpoint, instead of just the object ID.
Also add risk service (not yet used).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉 Looks good - just make sure to fix the typo I noted and this is good to merge.
impactMagnitude?: string; | ||
impactDescription?: string; | ||
adaptiveCapacity?: string; | ||
relativeAdaptiveValues?: string[]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo: This should be relatedAdaptiveValues
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good find
def get_serializer_class(self): | ||
if self.action == 'update' or self.action == 'create' or self.action == 'partial_update': | ||
return OrganizationRiskCreateSerializer | ||
return OrganizationRiskSerializer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
Use actual risk endpoint response instead of dummy data.
For use in autocomplete controls.
Use related object IDs on create, but serialize full objects.
For ranked weather events, continue to serialize full related concern.
Use "list" for consistency.
Overview
Modifies API risk endpoint to serialize related models. Query for actual risk data in UI and use it in vulnerability assessment page.
Also adds endpoints for community systems and all (not just organization) weather events.
Demo
Notes
Breaks adaptive need boxes, as they expect numeric input, but API returns strings.
Connects #322