Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: Session adding notes and recordings #8084

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
+189 −1
Draft

feat: Session adding notes and recordings #8084

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions ietf/meeting/urls.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ def get_redirect_url(self, *args, **kwargs):
safe_for_all_meeting_types = [
url(r'^session/(?P<acronym>[-a-z0-9]+)/?$', views.session_details),
url(r'^session/(?P<session_id>\d+)/drafts$', views.add_session_drafts),
url(r'^session/(?P<session_id>\d+)/notes_and_recordings$', views.add_session_notes_and_recordings),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lets make this endpoint just "recordings"

url(r'^session/(?P<session_id>\d+)/attendance$', views.session_attendance),
url(r'^session/(?P<session_id>\d+)/bluesheets$', views.upload_session_bluesheets),
url(r'^session/(?P<session_id>\d+)/minutes$', views.upload_session_minutes),
Expand Down
51 changes: 50 additions & 1 deletion ietf/meeting/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
from collections import OrderedDict, Counter, deque, defaultdict, namedtuple
from functools import partialmethod
import jsonschema
from urllib.parse import parse_qs, unquote, urlencode, urlsplit, urlunsplit
from urllib.parse import parse_qs, unquote, urlencode, urlsplit, urlunsplit, urlparse
from tempfile import mkstemp
from wsgiref.handlers import format_date_time

Expand Down Expand Up @@ -2538,6 +2538,55 @@ def add_session_drafts(request, session_id, num):
'form': form,
})

class SessionNotesAndRecordingsForm(forms.Form):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, just recordings

title = forms.CharField(max_length=255)
url = forms.URLField(label="Link to recording (YouTube only)")

def __init__(self, *args, **kwargs):
self.already_linked = kwargs.pop('already_linked')
super(self.__class__, self).__init__(*args, **kwargs)

def clean(self):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is really only validating the url field, it should probably be def clean_url(). In addition to a semantic hint about what you're validating, it'll correctly route your ValidationError exceptions into field-specific errors. You'll need to change the return value to be just the value for url rather than the entire cleaned_data.

Doc link for the form validation process: https://docs.djangoproject.com/en/5.1/ref/forms/validation/

(If you want to keep this as a clean() method, you may want to call self.add_error() so you can indicate the field to which errors apply)

url = self.cleaned_data['url']
parsed_url = urlparse(url)
if parsed_url.hostname not in ['www.youtube.com', 'youtube.com', 'youtu.be', 'm.youtube.com', 'youtube-nocookie.com', 'www.youtube-nocookie.com']:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make the allowed list into a settings entry?

raise forms.ValidationError("Must be a YouTube URL")
problems = set(url).intersection(set(self.already_linked))
if problems:
raise forms.ValidationError("Already linked: %s" % ', '.join([d.name for d in problems]))
Comment on lines +2554 to +2556
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has a different semantic than the meetecho upload API, which can update a link in case there was an error. We should think through what a chair would do if they entered the wrong link and wanted to fix it.

return self.cleaned_data

def add_session_notes_and_recordings(request, session_id, num):
# 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):
raise Http404
if session.is_material_submission_cutoff() and not has_role(request.user, "Secretariat"):
raise Http404

already_linked = [material.external_url for material in session.materials.filter(type="recording").exclude(states__type="recording", states__slug='deleted').order_by('presentations__order')]

session_number = None
sessions = get_sessions(session.meeting.number,session.group.acronym)
if len(sessions) > 1:
session_number = 1 + sessions.index(session)

if request.method == 'POST':
form = SessionNotesAndRecordingsForm(request.POST,already_linked=already_linked)
if form.is_valid():
title = form.cleaned_data['title']
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, for the meetecho uploads, the title is set by automata to look something like, e.g.,Video recording for tools on 2024-10-15 at 18:00:00. We should probably suggest a title of that form rather than starting with an empty control. Giving chairs the ability to pick a title here will be a new thing, but I think it's ok.

url = form.cleaned_data['url']
create_recording(session, url, title=title, user=request.user.person)
return redirect('ietf.meeting.views.session_details', num=session.meeting.number, acronym=session.group.acronym)
else:
form = SessionNotesAndRecordingsForm(already_linked=already_linked)

return render(request, "meeting/add_session_notes_and_recordings.html",
{ 'session': session,
'session_number': session_number,
'already_linked': session.materials.filter(type="recording").exclude(states__type="recording", states__slug='deleted').order_by('presentations__order'),
'form': form,
})

def session_attendance(request, session_id, num):
"""Session attendance view
Expand Down
70 changes: 70 additions & 0 deletions ietf/templates/meeting/add_session_notes_and_recordings.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
{% extends "base.html" %}
{# Copyright The IETF Trust 2015, All Rights Reserved #}
{% load origin static django_bootstrap5 %}
{% block title %}Add I-Ds to {{ session.meeting }} : {{ session.group.acronym }}{% endblock %}
{% block pagehead %}{{ form.media.css }}{% endblock %}
{% block content %}
{% origin %}
<h1>
Add Notes and Recordings to {{ session.meeting }}
{% if session_number %}: Session {{ session_number }}{% endif %}
<br>
<small class="text-body-secondary">{{ session.group.acronym }}
{% if session.name %}: {{ session.name }}{% endif %}
</small>
</h1>
{% comment %} TODO: put the session name here or calculate the number at the meeting {% endcomment %}
{% if session.is_material_submission_cutoff %}
<div class="alert alert-warning my-3">
The deadline for submission corrections has passed. This may affect published proceedings.
</div>
{% endif %}
<div class="alert alert-info my-3">
This form will link additional Notes and Recordings to this session with a revision of "Current at time of presentation". For more fine grained control of versions, or to remove Notes and Recordings from a session, adjust the sessions associated with an Internet-Draft from the Internet-Draft's main page.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry - missed commenting on this earlier: it's copied from the workflow that lets chairs associate drafts with sessions, and the lifecycle of such things is not the same. With drafts, chairs sometimes want to associate an older version with a session. Here, older recordings aren't really something to point to. It's a good indication that recording as a type of document may have been a mis-model. But in any case, please just remove this paragraph.

</div>
<h2 class="mt-5">Notes and Recordings already linked to this session</h2>
<table class="table table-sm table-striped tablesorter">
<thead>
<tr>
<th scope="col" data-sort="num">Revision</th>
<th scope="col" data-sort="document">Document</th>
</tr>
</thead>
{% if already_linked %}
<tbody>
{% for sp in already_linked %}
<tr>
<td>
{% if sp.rev %}
-{{ sp.rev }}
{% else %}
(current)
{% endif %}
</td>
<td><a href="{{ sp.external_url }}">{{ sp.title }}</a></td>
</tr>
{% endfor %}
</tbody>
{% else %}
<tbody>
<tr>
<td colspan="2" class="text-center font-italic">(none)</td>
</tr>
</tbody>
{% endif %}
</table>
<h2 class="mt-5">Add additional Notes and Recordings to this session</h2>
<form method="post">
{% csrf_token %}
{% bootstrap_form form %}
<button class="btn btn-{% if session.is_material_submission_cutoff %}warning{% else %}primary{% endif %}"
type="submit">
Save
</button>
<a class="btn btn-secondary float-end"
href="{% url 'ietf.meeting.views.session_details' num=session.meeting.number acronym=session.group.acronym %}">
Back
</a>
</form>
{% endblock %}
{% block js %}{{ form.media.js }}{% endblock %}
68 changes: 68 additions & 0 deletions ietf/templates/meeting/session_details_panel.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -366,8 +366,76 @@ <h3 class="mt-4">Notes and recordings</h3>
</tr>
{% endif %}
{% endif %}

{% with session.recordings as recordings %}
{% if recordings %}
{# There's no guaranteed order, so this is a bit messy: #}
{# Audio #}
{% for r in recordings %}
{% with href=r.get_href %}
{% if 'audio' in href %}
<tr>
<td>
<a href="{{ href }}">
{{ r.title }}
</a>
</td>
</tr>
{% endif %}
{% endwith %}
{% endfor %}
{# YouTube #}
{% for r in recordings %}
{% with href=r.get_href %}
{% if 'youtu' in href %}
<tr>
<td>
<a href="{{ href }}">
{{ r.title }}
</a>
</td>
</tr>
{% endif %}
{% endwith %}
{% endfor %}
{# Any other recordings #}
{% for r in recordings %}
{% with href=r.get_href %}
{% if not 'audio' in href and not 'youtu' in href %}
<tr>
<td>
<a href="{{ href }}">
{{ r.title }}
</a>
</td>
</tr>
{% endif %}
{% endwith %}
{% endfor %}
{% elif show_empty %}
{# <i class="bi"></i> #}
{% endif %}
{% if session.session_recording_url %}
<tr>
<td>
<a href="{{ session.session_recording_url }}">
Meetecho session recording
</a>
</td>
</tr>
{% endif %}
{% endwith %}

</tbody>
</table>

{% if can_manage_materials %}
<a class="btn btn-primary"
href="{% url 'ietf.meeting.views.add_session_notes_and_recordings' session_id=session.pk num=session.meeting.number %}">
Link additional notes and recordings to session
</a>
{% endif %}

{% endif %}
{% endwith %}{% endwith %}
{% endfor %}
Loading