-
Notifications
You must be signed in to change notification settings - Fork 56
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
Automatically configuring video settings for the Talk project. #242
Open
odkhang
wants to merge
33
commits into
fossasia:development
Choose a base branch
from
odkhang:auto-fill-talk-link
base: development
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+151
−21
Open
Changes from 22 commits
Commits
Show all changes
33 commits
Select commit
Hold shift + click to select a range
ecd2a5c
Enable video plugin
lcduong 2579880
Fix black in pipeline
lcduong 07fc00b
Fix black in pipeline
lcduong af2191e
Add comment
lcduong c5d487a
Update code
lcduong b57dda0
Configure video settings for talk
lcduong c87348f
Merge branch 'development' of github.com:odkhang/eventyay-talk into a…
odkhang 0adbf20
Fix black pipeline
odkhang 2b18319
Update code
odkhang d0d81cd
Update code
odkhang 06ee69e
Update code
odkhang 0a7f382
Update code
odkhang 9cfb2f0
Fix black in pipeline
odkhang a340e38
Add comment
odkhang 6dd84ae
Update code
odkhang 9fe1191
Update code
odkhang 463cd05
Update code
odkhang 1d3617f
using constant from http module instead of string
odkhang b2a9bd3
using constant from http, using httpstatus
odkhang 8766a3e
Merge branch 'development' into auto-fill-talk-link
odkhang 7bcca21
No need to bother with this outside of CI.
odkhang bc5a53a
fix black code style
odkhang 60df621
rework code
odkhang 68f221f
fix code style, remove unused import
odkhang bea82c0
fix inconsistence for API response
odkhang a845cf4
format code
odkhang 3eb6d17
handle case input form invalid
odkhang a3ef9fc
fix isort, black code style
odkhang d98fbde
fix isort, black code style
odkhang 7947d31
re format API response for case validation error
odkhang 1b6ce53
fix pipeline
odkhang 99717c9
make error message more clear for validation error
odkhang f28ab8e
coding style fix
odkhang File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,27 @@ | ||
import logging | ||
from http import HTTPMethod, HTTPStatus | ||
|
||
import jwt | ||
from django.conf import settings | ||
from django.http import Http404 | ||
from django_scopes import scopes_disabled | ||
from pretalx_venueless.forms import VenuelessSettingsForm | ||
from rest_framework import viewsets | ||
from rest_framework.authentication import get_authorization_header | ||
from rest_framework.decorators import ( | ||
api_view, | ||
authentication_classes, | ||
permission_classes, | ||
) | ||
from rest_framework.response import Response | ||
|
||
from pretalx.api.serializers.event import EventSerializer | ||
from pretalx.common import exceptions | ||
from pretalx.common.exceptions import AuthenticationFailedError | ||
from pretalx.event.models import Event | ||
|
||
logger = logging.getLogger(__name__) | ||
|
||
|
||
class EventViewSet(viewsets.ReadOnlyModelViewSet): | ||
serializer_class = EventSerializer | ||
|
@@ -24,3 +42,126 @@ def get_object(self): | |
if self.request.user.has_perm(self.permission_required, self.request.event): | ||
return self.request.event | ||
raise Http404() | ||
|
||
|
||
@api_view(http_method_names=[HTTPMethod.POST]) | ||
@authentication_classes([]) | ||
@permission_classes([]) | ||
def configure_video_settings(request): | ||
""" | ||
Configure video settings for an event | ||
@param request: request object | ||
@return response object | ||
""" | ||
try: | ||
video_settings = request.data.get("video_settings") | ||
|
||
if not video_settings or "secret" not in video_settings: | ||
raise ValueError("Video settings are missing or secret is not provided") | ||
|
||
payload = get_payload_from_token(request, video_settings) | ||
event_slug = payload.get("event_slug") | ||
video_tokens = payload.get("video_tokens") | ||
|
||
with scopes_disabled(): | ||
event_instance = Event.objects.get(slug=event_slug) | ||
save_video_settings_information(event_slug, video_tokens, event_instance) | ||
except Event.DoesNotExist: | ||
logger.error("Event with slug %s does not exist.", event_slug) | ||
return Response( | ||
{ | ||
"status": "error", | ||
"message": "Event with slug {} not found.".format(event_slug), | ||
}, | ||
status=HTTPStatus.NOT_FOUND, | ||
) | ||
except ValueError as e: | ||
logger.error("Error configuring video settings: %s", e) | ||
return Response( | ||
{"status": "error", "message": "Error configuring video settings."}, | ||
status=HTTPStatus.BAD_REQUEST, | ||
) | ||
except AuthenticationFailedError as e: | ||
logger.error("Authentication failed: %s", e) | ||
return Response( | ||
{"status": "error", "message": "Authentication failed."}, | ||
status=HTTPStatus.UNAUTHORIZED, | ||
) | ||
|
||
|
||
def get_payload_from_token(request, video_settings): | ||
""" | ||
Verify the token and return the payload | ||
@param request: request object | ||
@param video_settings: dict containing video settings | ||
@return: dict containing payload data from the token | ||
""" | ||
try: | ||
auth_header = get_authorization_header(request).split() | ||
if not auth_header: | ||
raise exceptions.AuthenticationFailedError("No authorization header") | ||
|
||
if len(auth_header) != 2 or auth_header[0].lower() != b"bearer": | ||
raise exceptions.AuthenticationFailedError( | ||
"Invalid token format. Must be 'Bearer <token>'" | ||
) | ||
|
||
token_decode = jwt.decode( | ||
odkhang marked this conversation as resolved.
Show resolved
Hide resolved
|
||
auth_header[1], video_settings.get("secret"), algorithms=["HS256"] | ||
) | ||
|
||
event_slug = token_decode.get("slug") | ||
video_tokens = token_decode.get("video_tokens") | ||
|
||
if not event_slug or not video_tokens: | ||
raise exceptions.AuthenticationFailedError("Invalid token payload") | ||
|
||
return {"event_slug": event_slug, "video_tokens": video_tokens} | ||
|
||
except jwt.ExpiredSignatureError: | ||
raise exceptions.AuthenticationFailedError("Token has expired") | ||
except jwt.InvalidTokenError: | ||
raise exceptions.AuthenticationFailedError("Invalid token") | ||
|
||
|
||
def save_video_settings_information(event_slug, video_tokens, event_instance): | ||
""" | ||
Save video settings information | ||
@param event_slug: A string representing the event slug | ||
@param video_tokens: A list of video tokens | ||
@param event_instance: An instance of the event | ||
@return: Response object | ||
""" | ||
|
||
if not video_tokens: | ||
raise ValueError("Video tokens list is empty") | ||
|
||
video_settings_data = { | ||
"token": video_tokens[0], | ||
odkhang marked this conversation as resolved.
Show resolved
Hide resolved
|
||
"url": "{}/api/v1/worlds/{}/".format( | ||
settings.EVENTYAY_VIDEO_BASE_PATH, event_slug | ||
), | ||
} | ||
|
||
video_settings_form = VenuelessSettingsForm( | ||
event=event_instance, data=video_settings_data | ||
) | ||
|
||
if video_settings_form.is_valid(): | ||
video_settings_form.save() | ||
logger.info("Video settings configured successfully for event %s.", event_slug) | ||
return Response({"status": "success"}, status=200) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use enum from |
||
else: | ||
logger.error( | ||
"Failed to configure video settings for event %s - Validation errors: %s.", | ||
event_slug, | ||
video_settings_form.errors, | ||
) | ||
return Response( | ||
{ | ||
"status": "error", | ||
"message": "Validation errors", | ||
"errors": video_settings_form.errors, | ||
}, | ||
status=400, | ||
) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this API endpoint suddenly behaves differently from DjangoRestFramework?
If our API is implemented with DjangoRestFramework, we should follow its convention, its style. Don't create inconsistency.
The HTTP status other than 2xx already indicates error, not need to add
status
field.In case of validation error (error due to user submitting data in wrong shape), DRF returns errors as a dict like this:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @hongquan, thanks for the comment.
I implement this REST API which will be called by Video system, it not show the error on template, and it behavior different with others API.
I'm not sure its answered your concern or not, please let me know if you have any else question.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't show errors on the template, but the errors will be looked at by developers when debugging. Never assume that you can just write once and every thing will work smoothly. When the system doesn't work as you expected, you will have to use Postman or some HTTP client tool to call this API and find out what is wrong.
Btw, if you follow DRF style, will the callee (video system) breaks. If it doesn't break, there is no reason to make it different.