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.

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

Resolve duplicate memberships, superiors/subordinates, and areas of operation #775

Merged
merged 5 commits into from
Sep 22, 2021
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
4 changes: 2 additions & 2 deletions association/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@
from complex_fields.base_models import BaseModel
from organization.models import Organization
from location.models import Location
from sfm_pc.models import GetComplexFieldNameMixin
from sfm_pc.models import GetComplexFieldNameMixin, SuperlativeDateMixin
from source.mixins import SourcesMixin


class Association(models.Model, BaseModel, SourcesMixin, GetComplexFieldNameMixin):
class Association(models.Model, BaseModel, SourcesMixin, SuperlativeDateMixin, GetComplexFieldNameMixin):
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
self.startdate = ComplexFieldContainer(self, AssociationStartDate)
Expand Down
4 changes: 2 additions & 2 deletions emplacement/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@
from complex_fields.base_models import BaseModel
from organization.models import Organization
from location.models import Location
from sfm_pc.models import GetComplexFieldNameMixin
from sfm_pc.models import GetComplexFieldNameMixin, SuperlativeDateMixin
from source.mixins import SourcesMixin


class Emplacement(models.Model, BaseModel, SourcesMixin, GetComplexFieldNameMixin):
class Emplacement(models.Model, BaseModel, SourcesMixin, SuperlativeDateMixin, GetComplexFieldNameMixin):
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
self.startdate = ComplexFieldContainer(self, EmplacementStartDate)
Expand Down
15 changes: 7 additions & 8 deletions organization/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import reversion

from django.db import models
from django.db.models import Min, Max
from django.db.models.functions import Coalesce
from django.utils.translation import ugettext as _
from django.conf import settings
Expand Down Expand Up @@ -85,10 +86,9 @@ def associations(self):
with nulls last.
'''
assocs = self.associationorganization_set\
.annotate(lcd=Coalesce('object_ref__associationstartdate__value',
'object_ref__associationenddate__value',
models.Value('1000-0-0')))\
.order_by('-lcd')
.annotate(min_start_date=Min('object_ref__associationstartdate__value'),
max_end_date=Max('object_ref__associationenddate__value'))\
.order_by(Coalesce('min_start_date', 'max_end_date').desc(nulls_last=True))

return assocs

Expand All @@ -101,10 +101,9 @@ def emplacements(self):
with nulls last.
'''
empls = self.emplacementorganization_set\
.annotate(lcd=Coalesce('object_ref__emplacementstartdate__value',
'object_ref__emplacementenddate__value',
models.Value('1000-0-0')))\
.order_by('-lcd')
.annotate(min_start_date=Min('object_ref__emplacementstartdate__value'),
max_end_date=Max('object_ref__emplacementenddate__value'))\
.order_by(Coalesce('min_start_date', 'max_end_date').desc(nulls_last=True))

return empls

Expand Down
7 changes: 2 additions & 5 deletions organization/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -167,12 +167,9 @@ def get_context_data(self, **kwargs):
)

associations = context['organization'].associations
context['associations'] = [ass.object_ref for ass in associations]
context['associations'] = [a.object_ref for a in associations]

area_ids = [
association.object_ref.area.get_value().value.id
for association in associations
]
area_ids = associations.values_list('object_ref__associationarea__value')

context['areas'] = serialize(
'geojson',
Expand Down
13 changes: 10 additions & 3 deletions person/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,19 @@ def get_context_data(self, **kwargs):

memberships = tuple(mem.object_ref for mem in affiliations)

# A person can have more than one membership in a particular
# organization. Generate a unique list of organizations to which this
# person belongs, in order to determine superiors and subordinates.
member_organizations = MembershipPerson.objects.filter(
id__in=context['person'].memberships.values_list('object_ref')
).distinct('membershippersonorganization__value')

context['memberships'] = memberships
context['subordinates'] = []
context['superiors'] = []
context['command_chain'] = []

for membership in memberships:
for membership in member_organizations:

# Grab the org object
org = membership.organization.get_value().value
Expand Down Expand Up @@ -132,8 +139,8 @@ def get_context_data(self, **kwargs):
if parent_commanders:
context['superiors'] += parent_commanders

context['subordinates'] = sort_commanders(context['subordinates'])
context['superiors'] = sort_commanders(context['superiors'])
context['subordinates'] = list(sort_commanders(context['subordinates']))
context['superiors'] = list(sort_commanders(context['superiors']))

context['command_chain'].reverse()
context['events'] = []
Expand Down
25 changes: 15 additions & 10 deletions sfm_pc/management/commands/import_google_doc.py
Original file line number Diff line number Diff line change
Expand Up @@ -1733,7 +1733,7 @@ def create_person(self, person_data):
role = None
else:
if role_name:
Copy link
Collaborator

Choose a reason for hiding this comment

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

if role_name can have whitespace in it, seems safest to do the strip before evaluating the truthyness of role_name.

Copy link
Author

Choose a reason for hiding this comment

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

Great call, @fgregg, done for both role and rank in 4a37057.

role, _ = Role.objects.get_or_create(value=role_name)
role, _ = Role.objects.get_or_create(value=role_name.strip())
role = role.id
else:
role = None
Expand All @@ -1744,24 +1744,29 @@ def create_person(self, person_data):
rank = None
else:
if rank_name:
rank, _ = Rank.objects.get_or_create(value=rank_name)
rank, _ = Rank.objects.get_or_create(value=rank_name.strip())
rank = rank.id
else:
rank = None

try:
title = person_data[membership_positions['Title']['value']] or None
title = person_data[membership_positions['Title']['value']].strip() or None
except IndexError:
title = None

membership_kwargs = {
'membershippersonmember__value': person,
'membershippersonorganization__value': organization,
'membershippersonfirstciteddate__value': fcd,
'membershippersonlastciteddate__value': lcd,
'membershippersonrole__value': role,
'membershippersonrank__value': rank,
'membershippersontitle__value': title,
}

try:
membership = MembershipPerson.objects.get(membershippersonmember__value=person,
membershippersonorganization__value=organization,
membershippersonfirstciteddate__value=fcd,
membershippersonlastciteddate__value=lcd,
membershippersonrole__value=role,
membershippersonrank__value=rank,
membershippersontitle__value=title)
membership = MembershipPerson.objects.get(**membership_kwargs)

sources = set(self.sourcesList(membership, 'member') + self.sourcesList(membership, 'organization'))
membership_data['MembershipPerson_MembershipPersonMember']['sources'] += sources
membership.update(membership_data)
Expand Down
13 changes: 13 additions & 0 deletions sfm_pc/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,3 +76,16 @@ def get_spreadsheet_source_field_name(cls, field_name):
# If no source field name is specified, the default is usually
# the spreadsheet field name with ":source" appended on
return cls.get_spreadsheet_field_name(field_name) + ':source'


class SuperlativeDateMixin:
Copy link
Author

Choose a reason for hiding this comment

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

get_value seems to return an arbitrary object if there is more than one value for a complex field. This mixin adds properties to classes with start and end dates so you can retrieve the earliest start date and latest end date. This is handy for displaying information sorted by either/both dates, else the sort appears nonsensical.


@property
def first_startdate(self):
return self.startdate.field_model.objects.filter(object_ref=self)\
.order_by('value').first()

@property
def last_enddate(self):
return self.enddate.field_model.objects.filter(object_ref=self)\
.order_by('-value').first()
16 changes: 8 additions & 8 deletions templates/organization/view.html
Original file line number Diff line number Diff line change
Expand Up @@ -136,12 +136,12 @@ <h3 id="areas-of-operation"><i class="fa fa-fw fa-globe"></i>
{% endif %}
</td>
<td class="cited">
{% datetype association.startdate.get_value 'start' %}
{% cite association.startdate.get_value %}
{% datetype association.first_startdate 'start' %}
{% cite association.first_startdate %}
</td>
<td class="cited">
{% datetype association.enddate.get_value 'end' %}
{% cite association.enddate.get_value %}
{% datetype association.last_enddate 'end' %}
{% cite association.last_enddate %}
</td>
</td>
{% endfor %}
Expand Down Expand Up @@ -190,12 +190,12 @@ <h3 id="sites">
{% cite emplacement.site.get_value %}
</td>
<td class="cited">
{% datetype emplacement.startdate.get_value 'start' %}
{% cite emplacement.startdate.get_value %}
{% datetype emplacement.first_startdate 'start' %}
{% cite emplacement.first_startdate %}
</td>
<td class="cited">
{% datetype emplacement.enddate.get_value 'end' %}
{% cite emplacement.enddate.get_value %}
{% datetype emplacement.last_enddate 'end' %}
{% cite emplacement.last_enddate %}
</td>
</td>
{% endfor %}
Expand Down