feat: Show bluesheets using Attended tables#7003
feat: Show bluesheets using Attended tables#7003pselkirk wants to merge 11 commits intoietf-tools:mainfrom
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #7003 +/- ##
==========================================
+ Coverage 88.91% 88.98% +0.06%
==========================================
Files 289 291 +2
Lines 40454 40771 +317
==========================================
+ Hits 35971 36281 +310
- Misses 4483 4490 +7 ☔ View full report in Codecov by Sentry. |
jennifer-richards
left a comment
jennifer-richards
left a comment
There was a problem hiding this comment.
I think there are a couple of sharp corners that should be fixed - at least putting a check before accepting a self-declared attendance POST is a must-fix.
jennifer-richards
left a comment
jennifer-richards
left a comment
There was a problem hiding this comment.
One remaining question: what happens if save_bluesheet() returns an error?
I came across it noticing that generate_bluesheet() now returns None on one branch and returns the outcome of save_bluesheet() on the other.
- Rename the live "bluesheet" to "attendance", add some explanatory text. - Add attendance links in materials view and pre-finalized proceedings view. - Don't allow users to add themselves after the corrections cutoff date.
jennifer-richards
left a comment
jennifer-richards
left a comment
There was a problem hiding this comment.
My concerns are addressed.
rjsparks
left a comment
rjsparks
left a comment
There was a problem hiding this comment.
The new attendance view needs slightly tighter gating.
| meeting = session.meeting | ||
| return [{'name':attended.person.name, 'affiliation':affiliation(meeting, attended.person)} for attended in attendance] | ||
|
|
||
| def session_attendance(request, session_id, num): |
There was a problem hiding this comment.
This needs to not allow someone to say "I attended" before the session happened. It should probably not let people in at all until at least one Attended record exists for a session. That's counting on a side-effect since the only other way such a record would be created is MeetEcho's upload right now, but we should take advantage of that for now - otherwise we have to define when the view should open based on some calculation of "the session is over".
There was a problem hiding this comment.
Using that "side effect" has the nice property that it mostly prevents impatient people from self-attesting their attendance before MeetEcho uploads their records. I.e., I think it's pretty close to the right signal to open this mechanism up.
|
|
||
| cor_cut_off_date = session.meeting.get_submission_correction_date() | ||
| today_utc = date_today(datetime.timezone.utc) | ||
| person = request.user.person |
There was a problem hiding this comment.
Guard here against personless users. In particular, guard against AnonymousUser
|
|
||
| attendance = Attended.objects.filter(session=session) | ||
| meeting = session.meeting | ||
| return [{'name':attended.person.name, 'affiliation':affiliation(meeting, attended.person)} for attended in attendance] |
There was a problem hiding this comment.
This should use person.plain_name()
| data = bluesheet_data(session) | ||
| if not data: | ||
| return | ||
| text = render_to_string('meeting/bluesheet.txt', { |
There was a problem hiding this comment.
This generates names in a different order than what MeetEcho uploads for bluesheets. There is timing data that MeetEcho has that is not available to us yet, so this may not be avoidable. What MeetEcho uploads for bluesheets is essentially in "joined the session" order. @alexamirante I wonder if join time could be added to the session attendee upload API - we could then use that in the new time field this PR adds to the Attended table.
There was a problem hiding this comment.
@rjsparks yes, we can send join times as well if needed.
| try: | ||
| person = request.user.person | ||
| was_there = Attended.objects.filter(session=session, person=person).exists() | ||
| can_add = today_utc <= cor_cut_off_date and MeetingRegistration.objects.filter(meeting=session.meeting, person=person).exists() and Attended.objects.filter(session=session).exists() and not was_there | ||
| except AttributeError: | ||
| was_there = can_add = False |
There was a problem hiding this comment.
I think something like
if user.is_authenticated and user.person is not None:
would be a lot better
There was a problem hiding this comment.
avoid a=b=c constructs. These are hard for new contributors to deal with.
|
This became feat/sess-apis |
4 participants
Fixes #6898
Fixes #6454