fix: better Meetecho API "order" management

ietf/meeting/tests_views.py

@@ -6477,16 +6477,11 @@ def test_upload_slides(self, mock_slides_manager_cls):
         self.assertEqual(replacement_sp.document.rev,'01')
         self.assertEqual(mock_slides_manager_cls.call_count, 1)
         self.assertEqual(mock_slides_manager_cls.call_args, call(api_config="fake settings"))
-        self.assertEqual(mock_slides_manager_cls.return_value.delete.call_count, 1)
+        self.assertEqual(mock_slides_manager_cls.return_value.revise.call_count, 1)
         self.assertEqual(
-            mock_slides_manager_cls.return_value.delete.call_args,
+            mock_slides_manager_cls.return_value.revise.call_args,
             call(session=session2, slides=sp.document),
         )
-        self.assertEqual(mock_slides_manager_cls.return_value.add.call_count, 1)
-        self.assertEqual(
-            mock_slides_manager_cls.return_value.add.call_args,
-            call(session=session2, slides=replacement_sp.document, order=2),
-        )
 
     def test_upload_slide_title_bad_unicode(self):
         session1 = SessionFactory(meeting__type_id='ietf')

ietf/meeting/views.py

@@ -2955,8 +2955,7 @@ def upload_session_slides(request, session_id, num, name=None):
                     sp.rev = doc.rev
                     sp.save()
                     if sm is not None:
-                        sm.delete(session=sess, slides=doc)  # removes the existing deck with previous revision
-                        sm.add(session=sess, slides=doc, order=sp.order)
+                        sm.revise(session=sess, slides=doc)
                 else:
                     max_order = (
                         sess.presentations.filter(document__type="slides").aggregate(

ietf/utils/meetecho.py

@@ -480,6 +480,8 @@ class SlidesManager(Manager):
     avoid this requirement.) 
     """
     def add(self, session: "Session", slides: "Document", order: int):
+        # Would like to confirm that session.presentations includes the slides Document, but we can't
+        # (same problem regarding unsaved Documents discussed in the docstring)
         self.api.add_slide_deck(
             wg_token=self.wg_token(session.group),
             session=str(session.pk),
@@ -493,12 +495,36 @@ def add(self, session: "Session", slides: "Document", order: int):
         )
 
     def delete(self, session: "Session", slides: "Document"):
+        """Delete a slide deck from the session"""
+        if session.presentations.filter(document=slides).exists():
+            # "order" problems are very likely to result if we delete slides that are actually still
+            # linked to the session
+            raise MeetechoAPIError(
+                f"Slides {slides.pk} are still linked to session {session.pk}."
+            )
+        # remove, leaving a hole
         self.api.delete_slide_deck(
             wg_token=self.wg_token(session.group),
             session=str(session.pk),
             id=slides.pk,
         )
+        if session.presentations.filter(document__type_id="slides").exists():
+            self.send_update(session)  # adjust order to fill in the hole        
 
+    def revise(self, session: "Session", slides: "Document"):
+        """Replace existing deck with its current state"""
+        sp = session.presentations.filter(document=slides).first()
+        if sp is None:
+            raise MeetechoAPIError(f"Slides {slides.pk} not in session {session.pk}")
+        order = sp.order
+        # remove, leaving a hole in the order on Meetecho's side
+        self.api.delete_slide_deck(
+            wg_token=self.wg_token(session.group),
+            session=str(session.pk),
+            id=slides.pk,
+        )
+        self.add(session, slides, order)  # fill in the hole
+
     def send_update(self, session: "Session"):
         self.api.update_slide_decks(
             wg_token=self.wg_token(session.group),

ietf/utils/tests_meetecho.py

@@ -561,18 +561,84 @@ def test_add(self, mock_add, mock_wg_token):
             ),
         )
 
+    @patch("ietf.utils.meetecho.MeetechoAPI.update_slide_decks")
     @patch("ietf.utils.meetecho.MeetechoAPI.delete_slide_deck")
-    def test_delete(self, mock_delete, mock_wg_token):
+    def test_delete(self, mock_delete, mock_update, mock_wg_token):
         sm = SlidesManager(settings.MEETECHO_API_CONFIG)
-        slides = SessionPresentationFactory(document__type_id="slides")
-        sm.delete(slides.session, slides.document)
+        # Test scenario: we had a session with two slide decks and we already deleted the SessionPresentation
+        # for one and are now updating Meetecho
+        slides = SessionPresentationFactory(document__type_id="slides", order=1)  # still attached to the session
+        session = slides.session
+        slides_doc = slides.document
+        removed_slides_doc = DocumentFactory(type_id="slides")
+
+        with self.assertRaises(MeetechoAPIError):
+            sm.delete(session, slides_doc)  # can't remove slides still attached to the session
+        self.assertFalse(any([mock_wg_token.called, mock_delete.called, mock_update.called]))
+
+        sm.delete(session, removed_slides_doc)
+        self.assertTrue(mock_wg_token.called)
+        self.assertTrue(mock_delete.called)
+        self.assertEqual(
+            mock_delete.call_args,
+            call(wg_token="atoken", session=str(session.pk), id=removed_slides_doc.pk),
+        )
+        self.assertTrue(mock_update.called)
+        self.assertEqual(
+            mock_update.call_args,
+            call(
+                wg_token="atoken",
+                session=str(session.pk),
+                decks=[
+                    {
+                        "id": slides_doc.pk,
+                        "title": slides_doc.title,
+                        "url": slides_doc.get_versionless_href(),
+                        "rev": slides_doc.rev,
+                        "order": 1,
+                    },
+                ]
+            )
+        )
+        mock_delete.reset_mock()
+        mock_update.reset_mock()
+
+        # Delete the other session and check that we don't make the update call
+        slides.delete()
+        sm.delete(session, slides_doc)
+        self.assertTrue(mock_delete.called)
+        self.assertFalse(mock_update.called)
+
+    @patch("ietf.utils.meetecho.MeetechoAPI.delete_slide_deck")
+    @patch("ietf.utils.meetecho.MeetechoAPI.add_slide_deck")
+    def test_revise(self, mock_add, mock_delete, mock_wg_token):
+        sm = SlidesManager(settings.MEETECHO_API_CONFIG)
+        slides = SessionPresentationFactory(document__type_id="slides", order=23)
+        slides_doc = slides.document
+        sm.revise(slides.session, slides.document)
         self.assertTrue(mock_wg_token.called)
         self.assertTrue(mock_delete.called)
         self.assertEqual(
             mock_delete.call_args,
-            call(wg_token="atoken", session=str(slides.session.pk), id=slides.document_id),
+            call(wg_token="atoken", session=str(slides.session.pk), id=slides_doc.pk),
+        )
+        self.assertTrue(mock_add.called)
+        self.assertEqual(
+            mock_add.call_args,
+            call(
+                wg_token="atoken",
+                session=str(slides.session.pk),
+                deck={
+                    "id": slides_doc.pk,
+                    "title": slides_doc.title,
+                    "url": slides_doc.get_versionless_href(),
+                    "rev": slides_doc.rev,
+                    "order": 23,
+                },
+            ),
         )
 
+
     @patch("ietf.utils.meetecho.MeetechoAPI.update_slide_decks")
     def test_send_update(self, mock_send_update, mock_wg_token):
         sm = SlidesManager(settings.MEETECHO_API_CONFIG)