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

Conversation

hancush
Copy link

@hancush hancush commented Sep 1, 2021

Overview

This PR resolves three bugs that manifested as duplicates in the user interface:

  1. Updates the import so that actual duplicate memberships are not created (See Apparent duplicates in superiors/subordinates on person detail #697 (comment))
  2. Updates the person view to retrieve a distinct list of organizations to which a person belongs, in order to retrieve superiors and subordinates without duplicating them (Again, see Apparent duplicates in superiors/subordinates on person detail #697 (comment))
  3. Updates the sort methodology for areas and emplacements so locations cannot be potentially multiplied due to multiple start/end dates (See Areas of operation are sometimes duplicated on organization detail pages #774 (comment))

Connects #697, #774.

Testing Instructions

  • Confirm CI passes.

@@ -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.

@hancush hancush requested a review from fgregg September 21, 2021 16:16
@@ -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.

@hancush hancush requested a review from fgregg September 22, 2021 14:30
@hancush hancush merged commit d0f838c into hcg/import-patches Sep 22, 2021
@hancush hancush deleted the hcg/duplicates branch September 27, 2021 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants