feat: Unify slide upload and proposal (ietf-tools#7787)

* attempt at optional approval

* Update of meeting slides propose/upload

* Fix tests and residual coding bugs

* Remove gratuitous blank lines

Author: jimfenton

ietf/meeting/forms.py

@@ -489,9 +489,12 @@ class UploadAgendaForm(ApplyToAllFileUploadForm):
 class UploadSlidesForm(ApplyToAllFileUploadForm):
     doc_type = 'slides'
     title = forms.CharField(max_length=255)
+    approved = forms.BooleanField(label='Auto-approve', initial=True, required=False)
 
-    def __init__(self, session, *args, **kwargs):
-        super().__init__(*args, **kwargs)
+    def __init__(self, session, show_apply_to_all_checkbox, can_manage, *args, **kwargs):
+        super().__init__(show_apply_to_all_checkbox, *args, **kwargs)
+        if not can_manage:
+            self.fields.pop('approved')
         self.session = session
 
     def clean_title(self):

ietf/meeting/tests_views.py

@@ -6454,7 +6454,7 @@ def test_upload_slides(self, mock_slides_manager_cls):
         self.assertFalse(session1.presentations.filter(document__type_id='slides'))
         test_file = BytesIO(b'this is not really a slide')
         test_file.name = 'not_really.txt'
-        r = self.client.post(url,dict(file=test_file,title='a test slide file',apply_to_all=True))
+        r = self.client.post(url,dict(file=test_file,title='a test slide file',apply_to_all=True,approved=True))
         self.assertEqual(r.status_code, 302)
         self.assertEqual(session1.presentations.count(),1) 
         self.assertEqual(session2.presentations.count(),1) 
@@ -6477,7 +6477,7 @@ def test_upload_slides(self, mock_slides_manager_cls):
         url = urlreverse('ietf.meeting.views.upload_session_slides',kwargs={'num':session2.meeting.number,'session_id':session2.id})
         test_file = BytesIO(b'some other thing still not slidelike')
         test_file.name = 'also_not_really.txt'
-        r = self.client.post(url,dict(file=test_file,title='a different slide file',apply_to_all=False))
+        r = self.client.post(url,dict(file=test_file,title='a different slide file',apply_to_all=False,approved=True))
         self.assertEqual(r.status_code, 302)
         self.assertEqual(session1.presentations.count(),1)
         self.assertEqual(session2.presentations.count(),2)
@@ -6501,7 +6501,7 @@ def test_upload_slides(self, mock_slides_manager_cls):
         self.assertIn('Revise', str(q("title")))
         test_file = BytesIO(b'new content for the second slide deck')
         test_file.name = 'doesnotmatter.txt'
-        r = self.client.post(url,dict(file=test_file,title='rename the presentation',apply_to_all=False))
+        r = self.client.post(url,dict(file=test_file,title='rename the presentation',apply_to_all=False, approved=True))
         self.assertEqual(r.status_code, 302)
         self.assertEqual(session1.presentations.count(),1)
         self.assertEqual(session2.presentations.count(),2)
@@ -6597,7 +6597,7 @@ def test_propose_session_slides(self):
             newperson = PersonFactory()
 
             session_overview_url = urlreverse('ietf.meeting.views.session_details',kwargs={'num':session.meeting.number,'acronym':session.group.acronym})
-            propose_url = urlreverse('ietf.meeting.views.propose_session_slides', kwargs={'session_id':session.pk, 'num': session.meeting.number})    
+            upload_url = urlreverse('ietf.meeting.views.upload_session_slides', kwargs={'session_id':session.pk, 'num': session.meeting.number})    
 
             r = self.client.get(session_overview_url)
             self.assertEqual(r.status_code,200)
@@ -6612,13 +6612,13 @@ def test_propose_session_slides(self):
             self.assertTrue(q('.proposeslides'))
             self.client.logout()
 
-            login_testing_unauthorized(self,newperson.user.username,propose_url)
-            r = self.client.get(propose_url)
+            login_testing_unauthorized(self,newperson.user.username,upload_url)
+            r = self.client.get(upload_url)
             self.assertEqual(r.status_code,200)
             test_file = BytesIO(b'this is not really a slide')
             test_file.name = 'not_really.txt'
             empty_outbox()
-            r = self.client.post(propose_url,dict(file=test_file,title='a test slide file',apply_to_all=True))
+            r = self.client.post(upload_url,dict(file=test_file,title='a test slide file',apply_to_all=True,approved=False))
             self.assertEqual(r.status_code, 302)
             session = Session.objects.get(pk=session.pk)
             self.assertEqual(session.slidesubmission_set.count(),1)
@@ -6639,6 +6639,25 @@ def test_propose_session_slides(self):
             self.assertEqual(len(q('.proposedslidelist p')), 2)
             self.client.logout()
 
+            login_testing_unauthorized(self,chair.user.username,upload_url)
+            r = self.client.get(upload_url)
+            self.assertEqual(r.status_code,200)
+            test_file = BytesIO(b'this is not really a slide either')
+            test_file.name = 'again_not_really.txt'
+            empty_outbox()
+            r = self.client.post(upload_url,dict(file=test_file,title='a selfapproved test slide file',apply_to_all=True,approved=True))
+            self.assertEqual(r.status_code, 302)
+            self.assertEqual(len(outbox),0)
+            self.assertEqual(session.slidesubmission_set.count(),2)
+            self.client.logout()
+
+            self.client.login(username=chair.user.username, password=chair.user.username+"+password")
+            r = self.client.get(session_overview_url)
+            self.assertEqual(r.status_code, 200)
+            q = PyQuery(r.content)
+            self.assertEqual(len(q('.uploadslidelist p')), 0)
+            self.client.logout()
+
     def test_disapprove_proposed_slides(self):
         submission = SlideSubmissionFactory()
         submission.session.meeting.importantdate_set.create(name_id='revsub',date=date_today() + datetime.timedelta(days=20))
@@ -6759,12 +6778,12 @@ def test_submit_and_approve_multiple_versions(self, mock_slides_manager_cls):
         session.meeting.importantdate_set.create(name_id='revsub',date=date_today()+datetime.timedelta(days=20))
         newperson = PersonFactory()
 
-        propose_url = urlreverse('ietf.meeting.views.propose_session_slides', kwargs={'session_id':session.pk, 'num': session.meeting.number})          
+        upload_url = urlreverse('ietf.meeting.views.upload_session_slides', kwargs={'session_id':session.pk, 'num': session.meeting.number})          
 
-        login_testing_unauthorized(self,newperson.user.username,propose_url)
+        login_testing_unauthorized(self,newperson.user.username,upload_url)
         test_file = BytesIO(b'this is not really a slide')
         test_file.name = 'not_really.txt'
-        r = self.client.post(propose_url,dict(file=test_file,title='a test slide file',apply_to_all=True))
+        r = self.client.post(upload_url,dict(file=test_file,title='a test slide file',apply_to_all=True,approved=False))
         self.assertEqual(r.status_code, 302)
         self.client.logout()
 
@@ -6787,15 +6806,15 @@ def test_submit_and_approve_multiple_versions(self, mock_slides_manager_cls):
 
         self.assertEqual(session.presentations.first().document.rev,'00')
 
-        login_testing_unauthorized(self,newperson.user.username,propose_url)
+        login_testing_unauthorized(self,newperson.user.username,upload_url)
         test_file = BytesIO(b'this is not really a slide, but it is another version of it')
         test_file.name = 'not_really.txt'
-        r = self.client.post(propose_url,dict(file=test_file,title='a test slide file',apply_to_all=True))
+        r = self.client.post(upload_url,dict(file=test_file,title='a test slide file',apply_to_all=True))
         self.assertEqual(r.status_code, 302)
 
         test_file = BytesIO(b'this is not really a slide, but it is third version of it')
         test_file.name = 'not_really.txt'
-        r = self.client.post(propose_url,dict(file=test_file,title='a test slide file',apply_to_all=True))
+        r = self.client.post(upload_url,dict(file=test_file,title='a test slide file',apply_to_all=True))
         self.assertEqual(r.status_code, 302)
         self.client.logout()       
 

ietf/meeting/urls.py

@@ -22,7 +22,6 @@ def get_redirect_url(self, *args, **kwargs):
     url(r'^session/(?P<session_id>\d+)/narrativeminutes$', views.upload_session_narrativeminutes),
     url(r'^session/(?P<session_id>\d+)/agenda$', views.upload_session_agenda),
     url(r'^session/(?P<session_id>\d+)/import/minutes$', views.import_session_minutes),
-    url(r'^session/(?P<session_id>\d+)/propose_slides$', views.propose_session_slides),
     url(r'^session/(?P<session_id>\d+)/slides(?:/%(name)s)?$' % settings.URL_REGEXPS, views.upload_session_slides),
     url(r'^session/(?P<session_id>\d+)/add_to_session$', views.ajax_add_slides_to_session),
     url(r'^session/(?P<session_id>\d+)/remove_from_session$', views.ajax_remove_slides_from_session),

ietf/meeting/views.py

@@ -1702,7 +1702,7 @@ def api_get_session_materials(request, session_id=None):
 
     minutes = session.minutes()
     slides_actions = []
-    if can_manage_session_materials(request.user, session.group, session):
+    if can_manage_session_materials(request.user, session.group, session) or not session.is_material_submission_cutoff():
         slides_actions.append(
             {
                 "label": "Upload slides",
@@ -1712,16 +1712,6 @@ def api_get_session_materials(request, session_id=None):
                 ),
             }
         )
-    elif not session.is_material_submission_cutoff():
-        slides_actions.append(
-            {
-                "label": "Propose slides",
-                "url": reverse(
-                    "ietf.meeting.views.propose_session_slides",
-                    kwargs={"num": session.meeting.number, "session_id": session.pk},
-                ),
-            }
-        )
     else:
         pass  # no action available if it's past cutoff
 
@@ -2920,17 +2910,15 @@ def upload_session_agenda(request, session_id, num):
                   })
 
 
+@login_required
 def upload_session_slides(request, session_id, num, name=None):
     """Upload new or replacement slides for a session
     
     If name is None or "", expects a new set of slides. Otherwise, replaces the named slides with a new rev.
     """
     # num is redundant, but we're dragging it along an artifact of where we are in the current URL structure
     session = get_object_or_404(Session, pk=session_id)
-    if not session.can_manage_materials(request.user):
-        permission_denied(
-            request, "You don't have permission to upload slides for this session."
-        )
+    can_manage = session.can_manage_materials(request.user)
     if session.is_material_submission_cutoff() and not has_role(
         request.user, "Secretariat"
     ):
@@ -2955,14 +2943,54 @@ def upload_session_slides(request, session_id, num, name=None):
 
     if request.method == "POST":
         form = UploadSlidesForm(
-            session, show_apply_to_all_checkbox, request.POST, request.FILES
+            session, show_apply_to_all_checkbox, can_manage, request.POST, request.FILES
         )
         if form.is_valid():
             file = request.FILES["file"]
             _, ext = os.path.splitext(file.name)
             apply_to_all = session.type_id == "regular"
             if show_apply_to_all_checkbox:
                 apply_to_all = form.cleaned_data["apply_to_all"]
+            if can_manage:
+                approved = form.cleaned_data["approved"]
+            else:
+                approved = False
+
+            # Propose slides if not auto-approved
+            if not approved:
+                title = form.cleaned_data['title']
+                submission = SlideSubmission.objects.create(session = session, title = title, filename = '', apply_to_all = apply_to_all, submitter=request.user.person)
+
+                if session.meeting.type_id=='ietf':
+                    name = 'slides-%s-%s' % (session.meeting.number, 
+                                         session.group.acronym) 
+                    if not apply_to_all:
+                        name += '-%s' % (session.docname_token(),)
+                else:
+                    name = 'slides-%s-%s' % (session.meeting.number, session.docname_token())
+                name = name + '-' + slugify(title).replace('_', '-')[:128]
+                filename = '%s-ss%d%s'% (name, submission.id, ext)
+                destination = io.open(os.path.join(settings.SLIDE_STAGING_PATH, filename),'wb+')
+                for chunk in file.chunks():
+                    destination.write(chunk)
+                destination.close()
+
+                submission.filename = filename
+                submission.save()
+
+                (to, cc) = gather_address_lists('slides_proposed', group=session.group, proposer=request.user.person).as_strings()
+                msg_txt = render_to_string("meeting/slides_proposed.txt", {
+                        "to": to,
+                        "cc": cc,
+                        "submission": submission,
+                        "settings": settings,
+                     })
+                msg = infer_message(msg_txt)
+                msg.by = request.user.person
+                msg.save()
+                send_mail_message(request, msg)
+                messages.success(request, 'Successfully submitted proposed slides.')
+                return redirect('ietf.meeting.views.session_details',num=num,acronym=session.group.acronym)
 
             # Handle creation / update of the Document (but do not save yet)
             if doc is not None:
@@ -3076,7 +3104,7 @@ def upload_session_slides(request, session_id, num, name=None):
         initial = {}
         if doc is not None:
             initial = {"title": doc.title}
-        form = UploadSlidesForm(session, show_apply_to_all_checkbox, initial=initial)
+        form = UploadSlidesForm(session, show_apply_to_all_checkbox, can_manage, initial=initial)
 
     return render(
         request,
@@ -3085,77 +3113,12 @@ def upload_session_slides(request, session_id, num, name=None):
             "session": session,
             "session_number": session_number,
             "slides_sp": session.presentations.filter(document=doc).first() if doc else None,
+            "manage": session.can_manage_materials(request.user),
             "form": form,
         },
     )
 
 
-@login_required
-def propose_session_slides(request, session_id, num):
-    session = get_object_or_404(Session,pk=session_id)
-    if session.is_material_submission_cutoff() and not has_role(request.user, "Secretariat"):
-        permission_denied(request, "The materials cutoff for this session has passed. Contact the secretariat for further action.")
-
-    session_number = None
-    sessions = get_sessions(session.meeting.number,session.group.acronym)
-    show_apply_to_all_checkbox = len(sessions) > 1 if session.type_id == 'regular' else False
-    if len(sessions) > 1:
-       session_number = 1 + sessions.index(session)
-
-    
-    if request.method == 'POST':
-        form = UploadSlidesForm(session, show_apply_to_all_checkbox,request.POST,request.FILES)
-        if form.is_valid():
-            file = request.FILES['file']
-            _, ext = os.path.splitext(file.name)
-            apply_to_all = session.type_id == 'regular'
-            if show_apply_to_all_checkbox:
-                apply_to_all = form.cleaned_data['apply_to_all']
-            title = form.cleaned_data['title']
-
-            submission = SlideSubmission.objects.create(session = session, title = title, filename = '', apply_to_all = apply_to_all, submitter=request.user.person)
-
-            if session.meeting.type_id=='ietf':
-                name = 'slides-%s-%s' % (session.meeting.number, 
-                                         session.group.acronym) 
-                if not apply_to_all:
-                    name += '-%s' % (session.docname_token(),)
-            else:
-                name = 'slides-%s-%s' % (session.meeting.number, session.docname_token())
-            name = name + '-' + slugify(title).replace('_', '-')[:128]
-            filename = '%s-ss%d%s'% (name, submission.id, ext)
-            destination = io.open(os.path.join(settings.SLIDE_STAGING_PATH, filename),'wb+')
-            for chunk in file.chunks():
-                destination.write(chunk)
-            destination.close()
-
-            submission.filename = filename
-            submission.save()
-
-            (to, cc) = gather_address_lists('slides_proposed', group=session.group, proposer=request.user.person).as_strings()
-            msg_txt = render_to_string("meeting/slides_proposed.txt", {
-                    "to": to,
-                    "cc": cc,
-                    "submission": submission,
-                    "settings": settings,
-                 })
-            msg = infer_message(msg_txt)
-            msg.by = request.user.person
-            msg.save()
-            send_mail_message(request, msg)
-            messages.success(request, 'Successfully submitted proposed slides.')
-            return redirect('ietf.meeting.views.session_details',num=num,acronym=session.group.acronym)
-    else: 
-        initial = {}
-        form = UploadSlidesForm(session, show_apply_to_all_checkbox, initial=initial)
-
-    return render(request, "meeting/propose_session_slides.html", 
-                  {'session': session,
-                   'session_number': session_number,
-                   'form': form,
-                  })
-
-
 def remove_sessionpresentation(request, session_id, num, name):
     sp = get_object_or_404(
         SessionPresentation, session_id=session_id, document__name=name
@@ -5072,6 +5035,7 @@ def approve_proposed_slides(request, slidesubmission_id, num):
                     "cc": cc,
                     "submission": submission,
                     "settings": settings,
+                    "approver": request.user.person
                 })
                 send_mail_text(request, to, None, subject, body, cc=cc)
                 return redirect('ietf.meeting.views.session_details',num=num,acronym=acronym)

ietf/templates/meeting/propose_session_slides.html

@@ -187,7 +187,7 @@ <h3 class="mt-4">Slides</h3>
             </a>
         {% elif request.user.is_authenticated and not session.is_material_submission_cutoff %}
             <a class="btn btn-primary proposeslides"
-               href="{% url 'ietf.meeting.views.propose_session_slides' session_id=session.pk num=session.meeting.number %}">
+               href="{% url 'ietf.meeting.views.upload_session_slides' session_id=session.pk num=session.meeting.number %}">
                 Propose slides
             </a>
         {% endif %}

ietf/templates/meeting/session_details_panel.html

@@ -1,4 +1,4 @@
-{% load ietf_filters %}{% autoescape off %}Your proposed slides have been approved for {{ submission.session.meeting }} : {{ submission.session.group.acronym }}{% if submission.session.name %} : {{submission.session.name}}{% endif %}
+{% load ietf_filters %}{% autoescape off %}Your proposed slides have been approved for {{ submission.session.meeting }} : {{ submission.session.group.acronym }}{% if submission.session.name %} : {{submission.session.name}}{% endif %} by {{approver}}
 
 Title: {{submission.title}}