Fix House Meeting Attendance Status Display by liam-middlebrook · Pull Request #151 · ComputerScienceHouse/conditional · GitHub
Skip to content

Fix House Meeting Attendance Status Display#151

Merged
mbillow merged 2 commits into
ComputerScienceHouse:developfrom
liam-middlebrook:fix-hm-display
Aug 28, 2017
Merged

Fix House Meeting Attendance Status Display#151
mbillow merged 2 commits into
ComputerScienceHouse:developfrom
liam-middlebrook:fix-hm-display

Conversation

@liam-middlebrook

Copy link
Copy Markdown
Member

This probably works. But it's currently untested

This probably works. But it's currently untested
Comment thread conditional/util/member.py Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Are SQLAlchemy queries lazily loaded? If so h_meetings should produce a SQLAlchemy Query object but not actually hit the database until it is used. IF that is the case

if only_absent:
    h_meetings = h_meetings.filter(MemberHouseMeetingAttendance.attendance_status == "Absent")

will work nicely and not require evaluating the entire queryset and filtering it in memory

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yup it does. Good catch, thanks.

@mbillow

mbillow commented Aug 28, 2017

Copy link
Copy Markdown
Member

@mbillow mbillow changed the title UNTESTED: Fix House Meeting Attendance Status Display Fix House Meeting Attendance Status Display Aug 28, 2017
@mbillow mbillow added the bug label Aug 28, 2017
@mbillow mbillow merged commit 9ff04ed into ComputerScienceHouse:develop Aug 28, 2017
@liam-middlebrook liam-middlebrook deleted the fix-hm-display branch August 29, 2017 19:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants