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.

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

feat: Add session recordings #8218

Open
wants to merge 22 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
d4052f9
feat: add session recordings
holloway Nov 15, 2024
420e296
feat: add session recordings
holloway Nov 17, 2024
061b68a
feat: deleting recordings
holloway Nov 17, 2024
2c15423
feat: deleting recordings and initial form values
holloway Nov 17, 2024
f0598e9
feat: use meeting date rather than today for initial title field. Fix…
holloway Nov 18, 2024
3cc968e
feat: confirm delete recordings modal. fix server utils delete recording
holloway Nov 18, 2024
ce04b71
fix: removing debug console.log
holloway Nov 19, 2024
606bf6d
feat: change button name from 'Ok' to 'Delete' for confirm deletion t…
holloway Nov 19, 2024
bda7061
feat: UTC time in string and delete modal text
holloway Nov 19, 2024
7b7caa2
fix: django html validation tests
holloway Nov 19, 2024
fd4547e
fix: django html validation tests
holloway Nov 19, 2024
7497306
fix: django html validation tests
holloway Nov 19, 2024
bca12a0
Merge branch 'main' into add-recordings-mk2
jennifer-richards Jan 14, 2025
01c8e8a
refactor: Work with SessionPresentations
jennifer-richards Jan 14, 2025
4783c43
fix: better ordering
jennifer-richards Jan 14, 2025
25a8eeb
chore: drop rev, hide table when empty
jennifer-richards Jan 14, 2025
761eb36
test: test delete_recordings method
jennifer-richards Jan 14, 2025
9b293b1
fix: debug delete_recordings
jennifer-richards Jan 14, 2025
4dca900
Merge branch 'ietf-tools:main' into add-recordings
holloway Jan 14, 2025
67536d6
test: test add_session_recordings view
jennifer-richards Jan 14, 2025
d0bc1ae
fix: better permissions handling
jennifer-richards Jan 14, 2025
33ae6b7
Merge remote-tracking branch 'holloway/add-recordings' into add-recor…
jennifer-richards Jan 14, 2025
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
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+)/recordings$', views.add_session_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
8 changes: 8 additions & 0 deletions ietf/meeting/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -870,6 +870,14 @@ def create_recording(session, url, title=None, user=None):

return doc

def delete_recording(session, pk):
'''
Delete a session recording
'''
document = Document.objects.filter(pk=pk, group=session.group).first()
if document:
document.delete()
Copy link
Member

Choose a reason for hiding this comment

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

This is what caught my eye. This function would be better accepting a Document object rather than a pk, and perhaps should live in the Session model so that the connection to Session is naturally enforced.


def get_next_sequence(group, meeting, type):
'''
Returns the next sequence number to use for a document of type = type.
Expand Down
57 changes: 55 additions & 2 deletions ietf/meeting/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
from functools import partialmethod
import jsonschema
from pathlib import Path
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 @@ -86,7 +86,7 @@
from ietf.meeting.utils import swap_meeting_schedule_timeslot_assignments, bulk_create_timeslots
from ietf.meeting.utils import preprocess_meeting_important_dates
from ietf.meeting.utils import new_doc_for_session, write_doc_for_session
from ietf.meeting.utils import get_activity_stats, post_process, create_recording
from ietf.meeting.utils import get_activity_stats, post_process, create_recording, delete_recording
from ietf.meeting.utils import participants_for_meeting, generate_bluesheet, bluesheet_data, save_bluesheet
from ietf.message.utils import infer_message
from ietf.name.models import SlideSubmissionStatusName, ProceedingsMaterialTypeName, SessionPurposeName
Expand All @@ -103,6 +103,7 @@
from ietf.utils.response import permission_denied
from ietf.utils.text import xslugify
from ietf.utils.timezone import datetime_today, date_today
from ietf.settings import YOUTUBE_DOMAINS

from .forms import (InterimMeetingModelForm, InterimAnnounceForm, InterimSessionModelForm,
InterimCancelForm, InterimSessionInlineFormSet, RequestMinutesForm,
Expand Down Expand Up @@ -2568,6 +2569,58 @@ def add_session_drafts(request, session_id, num):
'form': form,
})

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

def clean_url(self):
url = self.cleaned_data['url']
parsed_url = urlparse(url)
if parsed_url.hostname not in YOUTUBE_DOMAINS:
raise forms.ValidationError("Must be a YouTube URL")
return url


def add_session_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

session_number = None
sessions = get_sessions(session.meeting.number,session.group.acronym)
official_timeslotassignment = session.official_timeslotassignment()
assertion("official_timeslotassignment is not None")
initial = {
'title': f"Video recording for {session.group.acronym} on {official_timeslotassignment.timeslot.utc_start_time().strftime('%b-%d-%Y at %H:%M:%S')}"
}

if len(sessions) > 1:
session_number = 1 + sessions.index(session)

if request.method == 'POST':
delete = request.POST.get('delete', False)
if delete:
delete_recording(pk=delete, session=session)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should think through what a chair would do if they entered the wrong link and wanted to fix it. @rjsparks

I've added a delete button. Is that ok?

Screenshot from 2024-11-18 10-56-26

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Regarding the 'Revision' column is that valid for recordings? How would someone update the revision of a recording anyway?

Copy link
Member

Choose a reason for hiding this comment

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

It's valid as long as we're modeling recordings as Document objects.

To date, we have not revised a recording document. Doesn't mean we couldn't (with the current model anyhow). An -01 for youtube would just point to a different URL the same as an -01 for a draft points to a different filename.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thoughts on the delete button? Should it have a confirmation modal?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm going to assume we need a confirmation modal.

Copy link
Contributor Author

@holloway holloway Nov 18, 2024

Choose a reason for hiding this comment

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

Done. It looks like this. I did it in plain JS with a <dialog> rather than Bootstrap as we're looking to migrate away from that.

Screenshot from 2024-11-19 13-01-20

Copy link
Member

Choose a reason for hiding this comment

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

I have minor heartburn that we're allowing a Document object to be deleted. It violates an assumption about what Document objects are for, but I suppose we have that problem with these recording documents-that-are-just-external-links already and some future project should migrate them to some other model instead.

I'll request one change and we can move forward with this.

Copy link
Member

Choose a reason for hiding this comment

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

It would be better to utilize the form framework here to get back a Document instead of a pk. As written, I think a malicious post could delete an arbitrary document.

form = SessionRecordingsForm(initial=initial)
else:
form = SessionRecordingsForm(request.POST)
if form.is_valid():
title = form.cleaned_data['title']
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 = SessionRecordingsForm(initial=initial)

return render(request, "meeting/add_session_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
3 changes: 3 additions & 0 deletions ietf/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -1384,3 +1384,6 @@ def skip_unreadable_post(record):
CSRF_TRUSTED_ORIGINS += ['http://localhost:8000', 'http://127.0.0.1:8000', 'http://[::1]:8000']
SESSION_COOKIE_SECURE = False
SESSION_COOKIE_SAMESITE = 'Lax'


YOUTUBE_DOMAINS = ['www.youtube.com', 'youtube.com', 'youtu.be', 'm.youtube.com', 'youtube-nocookie.com', 'www.youtube-nocookie.com']
124 changes: 124 additions & 0 deletions ietf/templates/meeting/add_session_recordings.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
{% 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 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 %}
<h2 class="mt-5">Recordings already linked to this session</h2>
{% if already_linked %}
<form method="post" id="delete_recordings_form">
{% csrf_token %}
<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>
<th scope="col">Delete</th>
</tr>
</thead>
<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>
<td><button type="submit" aria-label="Delete {{ sp.title }}" class="btn btn-danger" name="delete" value="{{ sp.pk }}">Delete</button></td>
</tr>
{% endfor %}
</tbody>
</table>
</form>
{% else %}
<table class="table table-sm table-striped">
<thead>
<tr>
<th scope="col" data-sort="num">Revision</th>
<th scope="col" data-sort="document">Document</th>
<th scope="col">Delete</th>
</tr>
</thead>
<tbody>
<tr>
<td colspan="3" class="text-center font-italic">(none)</td>
</tr>
</tbody>
</table>
{% endif %}
<dialog id="delete_confirm_dialog">
<p>Really delete the link to <a href="#" id="delete_confirm_link">(default)</a>?</p>
<form method="post" class="d-flex justify-content-between">
{% csrf_token %}
<button class="btn btn-secondary" type="button" id="delete_confirm_cancel">Cancel</button>
<button class="btn btn-danger" type="submit" name="delete" id="delete_confirm_submit">Delete</button>
</form>
</dialog>
<h2 class="mt-5">Add additional 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">
Add recording
</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 }}

<script>
document.addEventListener('DOMContentLoaded', () => {
const form = document.getElementById('delete_recordings_form')
const dialog = document.getElementById('delete_confirm_dialog')
const dialog_link = document.getElementById('delete_confirm_link')
const dialog_submit = document.getElementById('delete_confirm_submit')
const dialog_cancel = document.getElementById('delete_confirm_cancel')

dialog.style.maxWidth = '30vw';

form.addEventListener('submit', (e) => {
e.preventDefault()
dialog_submit.value = e.submitter.value
const recording_link = e.submitter.closest('tr').querySelector('a')
dialog_link.setAttribute('href', recording_link.getAttribute('href'))
dialog_link.textContent = recording_link.textContent
dialog.showModal()
})

dialog_cancel.addEventListener('click', (e) => {
e.preventDefault()
dialog.close()
})

document.addEventListener('keydown', (e) => {
if (dialog.open && e.key === 'Escape') {
dialog.close()
}
})
})
</script>

{% endblock %}
8 changes: 8 additions & 0 deletions ietf/templates/meeting/session_details_panel.html
Original file line number Diff line number Diff line change
Expand Up @@ -368,5 +368,13 @@ <h3 class="mt-4">Notes and recordings</h3>
</tbody>
</table>
{% endif %}

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

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