Skip to content
This repository has been archived by the owner on Sep 5, 2019. It is now read-only.

Manual ordering #3

Merged
merged 11 commits into from Aug 26, 2019
Merged

Manual ordering #3

merged 11 commits into from Aug 26, 2019

Conversation

gh-PonyM
Copy link
Contributor

  • Adds possibility to order Membership of a person for an agency in the person view
  • Required onegov.people AgencyMembership model to get a new column order_within_person
  • Excel import was adapted for the new standard, having backwards compatibility
  • Backwards compatibility was tested by modifying the excel data in fixture export.xls

Lukas Burkhard added 11 commits August 20, 2019 16:02
- Adds migration
- Corrects tests
- order_within_person in AgencyMembership can not be Null
- Hence add this information to the excel fixtures
- Does not  add the additional info in excel everywhere to test backwards compatibility
- object_session was None since agency was not in session
@gh-PonyM gh-PonyM merged commit 33f1136 into master Aug 26, 2019
@gh-PonyM gh-PonyM deleted the manual_ordering branch August 26, 2019 09:03
Copy link
Contributor

@href href left a comment

Choose a reason for hiding this comment

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

👍


orders_for_person = session.query(
ExtendedAgencyMembership.order_within_person
).filter_by(person_id=person_id).all()
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as before, .all() is really almost never useful.

ExtendedAgencyMembership.order_within_person
).filter_by(person_id=person_id).all()

orders_for_person = list(
Copy link
Contributor

Choose a reason for hiding this comment

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

Use a tuple if the result is immutable.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants