fix: refactor api_new_meeting_registration. Fixes #7608

[rpcross]

• change to use JSON payload
• handle multiple records in one request

[rpcross]

I am working on tests for this. Using a JSON payload means the existing require_api_key decorator won't work. Assuming JSON payload is the way to go, are there other precedents for checking API key? Function can do it's own validation, or can update decorator.

[jennifer-richards]

are there other precedents for checking API key?

https://github.com/ietf-tools/datatracker/blob/b5ab4b66110ed0777dae170e87db9343876b2741/ietf/api/ietf_utils.py#L27-L46 is what we've started using for datatracker. This uses the X-Api-Key header to store a shared secret and keeps the auth concerns out of the API itself.

[rjsparks]

@rpcross This may have fallen into the cracks because of 120?

[rjsparks]

@rpcross - lets discuss this soon?

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 68.11594% with 22 lines in your changes missing coverage. Please review.

Project coverage is 88.78%. Comparing base (def346e) to head (55771d5). Report is 17 commits behind head on main.

Files with missing lines	Patch %	Lines
ietf/api/views.py	68.11%	22 Missing :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7724      +/-   ##
==========================================
+ Coverage   88.71%   88.78%   +0.06%     
==========================================
  Files         310      312       +2     
  Lines       40882    40945      +63     
==========================================
+ Hits        36270    36351      +81     
+ Misses       4612     4594      -18     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[rpcross]

Update: The new proposal is to return to having a single MeetingRegistration record per person per meeting. The various registrations a single person can have for one meeting will be moved to a related object like this:

class MeetingRegistrationDetail(models.Model): # name? meeting_registration = ForeignKey(MeetingRegistration) reg_type = ForeignKey(RegistrationTypeName) ticket_type = ForeignKey(RegistrationTicketTypeName, blank=True, null=True) # there are older registrations where this is empty

And use a custom object manager like this:

class MeetingRegistrationManager(models.Manager): def onsite(self): return self.get_queryset().filter(meetingregistrationdetail__reg_type__slug='onsite') def remote(self): # don't count people that also have a onsite registration return self.get_queryset().filter(meetingregistrationdetail__reg_type__slug='remote').exclude(meetingregistrationdetail__reg_type__slug='onsite')

And queries like this: MeetingRegistration.objects.onsite().filter(meeting__number=121)

We could combine reg_type and ticket_type, but the most frequent queries are those for specific reg_type “onsite” or “remote” so it seems reasonable to keep them separate. Are there better names for new classes, fields above?

Considerations: Should we keep “checkedin” and “affialiation” on the MeetingRegistration record? Checkedin would only be set from a reg_type=onsite record. Hackathon or remote registrations would not affect this field. In theory you could have a different affiliation value for each registration, maybe prefer value from onsite over remote, and regular onsite|remote over hackathon, and populated over empty?

For simplicity is it acceptable to clear and recreate the related objects on update?

The above implementation is most flexible. Allowing the capture of all registration types currently represented, and new ones with the addition of new name types. Would a simpler approach of using boolean fields has_onsite, has_remote (has_hackathon_onsite, has_hackathon_remote) be sufficient. Note, you couldn’t represent two onsite one-day passes in that implementation.

Thoughts?

[rjsparks]

Should we keep “checkedin” and “affialiation” on the MeetingRegistration record?

We need checkedin somewhere. I think it's fine to be on the MeetingRegistration record - we can make it clear in the model that it means "picked up a badge onsite at the meeting".

I think we should keep "affiliation", but need to read the code to see if the datatracker is using it. Keeping it on MeetingRegistration makes sense.

[jennifer-richards]

Update: The new proposal is to return to having a single MeetingRegistration record per person per meeting. The various registrations a single person can have for one meeting will be moved to a related object like this:

I like the concept. A couple key things to watch are, 1) that this is simplifying our lives, not just mooshing complexity/bugs around; and 2) that we're not replicating parts of the registration app that shouldn't leak into the datatracker.

class MeetingRegistrationDetail(models.Model): # name? meeting_registration = ForeignKey(MeetingRegistration) reg_type = ForeignKey(RegistrationTypeName) ticket_type = ForeignKey(RegistrationTicketTypeName, blank=True, null=True) # there are older registrations where this is empty

I guess RegistrationTicketTypeName includes things like "hackathon", "week", "day"?

Name-wise, I like the ticket metaphor and would be inclined to call it a MeetingRegistrationTicket and change RegistrationTypeName to AttendanceTypeName or something (plus a matching change to reg_type).

It might be too early for this level of minutiae, but I'd encourage avoiding blank=True, null=True on the ticket_type field. Instead, use an unknown or some other type name that's marked as used=False and is not an option for new work. This will be closer to a guarantee we don't start creating incomplete records in the future.

And use a custom object manager like this:

class MeetingRegistrationManager(models.Manager): def onsite(self): return self.get_queryset().filter(meetingregistrationdetail__reg_type__slug='onsite') def remote(self): # don't count people that also have a onsite registration return self.get_queryset().filter(meetingregistrationdetail__reg_type__slug='remote').exclude(meetingregistrationdetail__reg_type__slug='onsite')

And queries like this: MeetingRegistration.objects.onsite().filter(meeting__number=121)

Checking an assumption, do we definitely need to support both onsite and remote registrations at the same meeting? I'd say we do either if we know we need it and/or if the reg system supports it, just so the datatracker side is expressive enough to capture everything. If not, life would be easier if the reg_type could move onto the MeetingRegistration model.

We could combine reg_type and ticket_type, but the most frequent queries are those for specific reg_type “onsite” or “remote” so it seems reasonable to keep them separate. Are there better names for new classes, fields above?

As noted above, I think you're describing an attendance type and it should be labeled as such instead of the more vague "registration type". The business logic about what combinations make sense belongs on the reg side and datatracker should be able to represent whatever reg says and not second guess its records.

Considerations: Should we keep “checkedin” and “affialiation” on the MeetingRegistration record? Checkedin would only be set from a reg_type=onsite record. Hackathon or remote registrations would not affect this field. In theory you could have a different affiliation value for each registration, maybe prefer value from onsite over remote, and regular onsite|remote over hackathon, and populated over empty?

I don't know for sure about affiliation, but my instinct says it is a MeetingRegistration property that applies to the person / meeting combination. Our records afaik only show one affiliation per person at each meeting. I can imagine why we might want separate affiliations, but I wouldn't add it here unless it's something supported by the reg side that we need to allow for.

I think checkedin should be tied to the "ticket" that was checked in, not to the whole MeetingRegistration. For day passes, the datatracker needs to be able to tell that a day pass is active and to which day it's being applied.

For simplicity is it acceptable to clear and recreate the related objects on update?

This depends on what the datatracker is doing with the data. It'd be nice if they didn't move around, but might not be a hard requirement. I think we'll want to be able to refer at least to MeetingRegistrations so those should be stable. For things below that, we mostly need to be clear about which things we're allowed to point at with FKs and be careful about deletion policies. We don't want to get into a situation where the datatracker prevents reg from accurately recording the true registration data.

The above implementation is most flexible. Allowing the capture of all registration types currently represented, and new ones with the addition of new name types. Would a simpler approach of using boolean fields has_onsite, has_remote (has_hackathon_onsite, has_hackathon_remote) be sufficient. Note, you couldn’t represent two onsite one-day passes in that implementation.

I prefer the better-normalized approach with the additional model to a raft of boolean flags. Not being able to represent multiple one-day passes makes it a non-starter.

[jennifer-richards]

Robert posted his reply while I was writing my tome. His comment (and an out-of-band discussion) convinced me that the suggestion I have about putting "checkedin" on the "ticket" instances is really a different concept.

[rpcross]

This PR is replaced by https://github.com/ietf-tools/datatracker/pull/8408

[jennifer-richards]

Closing as it's been superseded