feat: explicit names for meetecho recordings#8062
Conversation
|
This gets us a step closer to not providing links when recordings don't exist. (see #7888) |
jennifer-richards
left a comment
jennifer-richards
left a comment
There was a problem hiding this comment.
Couple small nits.
I wonder if it'd make sense to simply make this the .../recording URL endpoint instead of specializing it. The structure of the payload would lend itself to other metadata relatively transparently. OTOH maybe it's better to plan on retiring this endpoint when that happens rather than mutating its behavior.
| self.assertContains(r, "Restricted to role: Recording Manager", status_code=403) | ||
|
|
||
| r = self.client.post(url, {"apikey": apikey.hash()}) | ||
| self.assertContains(r, "Too long since last regular login", status_code=400) |
There was a problem hiding this comment.
If we don't already, we ought to cover features like this in a unit test of the require_api_key() decorator and trust that. (Not actually asking for a change here unless you're enthusiastic about it, just pointing it out.)
There was a problem hiding this comment.
I felt the lack-of-reuse pain, but didn't pursue it as I think we might want to rework all of these to use DRF going forward maybe?
ietf/api/urls.py
Outdated
| # Let Meetecho set session video URLs | ||
| url(r'^meeting/session/video/url$', meeting_views.api_set_session_video_url), | ||
| # Let Meetecho tell us the name of its recordings | ||
| url(r'meeting/session/recordingname$', meeting_views.api_set_meetecho_recording_name), |
There was a problem hiding this comment.
Need a ^ at the start.
Maybe make it ^meeting/session/recording-name to better match recording_name in the model (and kebab-case that we've used in other URLs, like doc/draft-aliases/ at line 27)
There was a problem hiding this comment.
will get the ^.
I can kebob case it, but I don't think we have consistency there.
ietf/meeting/models.py
Outdated
| name = self.meetecho_recording_name | ||
| if name is None or name.strip() == "": | ||
| name = self._session_recording_url_label() | ||
| if url_formatter.strip()!="" and name is not None: |
There was a problem hiding this comment.
style nit: spacing around !=
|
we may need to use (build on top of) the recording url endpoint for non-meetecho generated youtube videos (e.g. those for MOQ right now). |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8062 +/- ##
==========================================
+ Coverage 88.78% 88.89% +0.10%
==========================================
Files 296 304 +8
Lines 41320 41345 +25
==========================================
+ Hits 36687 36754 +67
+ Misses 4633 4591 -42 ☔ View full report in Codecov by Sentry. |
2 participants
fixes #3454