datatracker icon indicating copy to clipboard operation
datatracker copied to clipboard

Session presentation created with a null revision

Open jennifer-richards opened this issue 1 year ago • 6 comments

Describe the issue

Following IETF-117, the slides document slides-117-dult-detecting-unwanted-location-trackers-rev-e-00 was linked to a SessionPresentation with rev=None. The rev field should have matched that from the Document, which had rev="00".

The event history on the document included

[{'time': datetime.datetime(2023, 7, 26, 23, 38, 8, 664466, tzinfo=datetime.timezone.utc),
  'type': 'added_comment',
  'desc': 'Added to session: IETF-117: dult  Thu-1630'},
 {'time': datetime.datetime(2023, 7, 26, 23, 25, 33, 384553, tzinfo=datetime.timezone.utc),
  'type': 'added_comment',
  'desc': 'Removed from session: IETF-117: dult  Thu-1630'},
 {'time': datetime.datetime(2023, 7, 26, 23, 25, 27, 655974, tzinfo=datetime.timezone.utc),
  'type': 'new_revision',
  'desc': 'New revision available: 00'}]

The "Added to session:" event can be created either in add_session_drafts() or ajax_add_slides_to_session(). It's not clear how either of these could have been used to create the state where rev=None. In add_session_drafts(), SessionPresentation.rev is set to doc.rev, which was "00". The add_session_drafts() UI is limited to draft doc types via SearchableDocumentsField (though perhaps this could be worked around with a custom query)

@rjsparks, any idea what happened? We should be sure the interface makes it difficult/impossible to create this situation.

Code of Conduct

jennifer-richards avatar Aug 04 '23 20:08 jennifer-richards

@rjsparks added "To help diagnose completely - I used the “On agenda” edit button on the slides main document page to reattach the slides to the session."

jennifer-richards avatar Aug 07 '23 17:08 jennifer-richards

It is intentional that rev can be empty for SessionPresentation objects - that means "current version". See https://github.com/ietf-tools/datatracker/blob/main/ietf/doc/views_doc.py#L1967-L1978.

But we should go through the views that use it and make sure it's used consistently - for slides, the materials views (in particular in session_details_panel) always show current even if a version was actually specified.

rjsparks avatar Aug 07 '23 20:08 rjsparks

I saw that we use the absence of a rev for drafts, but this was the only instance of a SessionPresentation for slides that had a null field.

When looking at this, we should consider disallowing nulls for the rev field. It's currently a CharField(blank=True, null=True) which is generally discouraged unless you need two ways to leave the field empty. I think using an empty string for the rev would be adequate for our needs.

jennifer-richards avatar Aug 08 '23 16:08 jennifer-richards

So two goals identified for this so far:

  • Make it so that SessionPresentation object connected only to docs of type draft can set an empty rev - all other doc types must set an explicit version.
  • Make it so that SessionPresentation doesn't accept Null for the rev field and migrate any that are currently set.

rjsparks avatar Aug 11 '23 21:08 rjsparks

But we should go through the views that use it and make sure it's used consistently - for slides, the materials views (in particular in session_details_panel) always show current even if a version was actually specified.

I just bumped into this while working on #7005 - is it desirable that the slides always show the current rev, or is that a bug?

jennifer-richards avatar Mar 06 '24 17:03 jennifer-richards

Slides should always show current rev.

rjsparks avatar Mar 06 '24 17:03 rjsparks