Scheduling of Live Events(#77) #92

Deep-Chill · 2024-01-30T10:20:30Z

Early versions of API endpoints/URLs to add and delete LiveEvents.
Gets LiveEvent objects using their scheduled_at time, for adding/deleting as well as marking attendance.
Updates Mixin to add a rank property to help correctly add attributes to LiveEvent objects.

Issues:

On the URL: http://127.0.0.1:8000/sequences/steve/seq-cert-live-event/1/ if a user is not logged in, they are able to make POST requests and the timer updates, but if logged in as either steve or a superuser it returns djaodjin-pages-vue.js:581 Error sending ping: Forbidden Haven't investigated yet.

- Early versions of API endpoints/URLs to add and delete LiveEvents. - Gets LiveEvent objects using their scheduled_at time, for adding/deleting as well as marking attendance. - Updates Mixin to add a rank property to help correctly add attributes to LiveEvent objects. - Issues: - - On the URL: `http://127.0.0.1:8000/sequences/steve/seq-cert-live-event/1/` if a user is not logged in, they are able to make POST requests and the timer updates, but if logged in as either steve or a superuser it returns `djaodjin-pages-vue.js:581 Error sending ping: Forbidden` Haven't investigated yet.

- Added LiveEvent creation/deletion API Endpoints - Added ability to view multiple LiveEvents belonging to the same PageElement on the front-end view. Could implement this with an API endpoint instead and call that for views individual users. Needs logic updating to be functional, ie. dealing with PageElements without liveevents and other edge cases - Context-specific comments in code with comments and questions. To be removed before final version.

Currently working on: - fixing issue where EnumeratedProgress is not keeping track of the exact LiveEvent. Current approach is to add an extra field in the EnumeratedProgress model and use that. - Ensuring the delete method changes the status of the LiveEvent and doesn't delete it - Correct URL locations

Added soft delete to LiveEvents, updates to attendee retrieval logic and more. Notes: - Need further clarification on URL patterns. - Not sure if a soft delete is required for LiveEvent attendance. - A few comments in there, to be removed before merge.

smirolo

There is just really the 'live_event_rank' stored in the extra field that require some explanation.

smirolo · 2024-02-13T16:49:23Z

pages/api/progress.py

@@ -201,7 +204,7 @@ def post(self, request, *args, **kwargs):
        return api_response.Response(serializer.data, status=status_code)


-class LiveEventAttendanceAPIView(EnumeratedProgressRetrieveAPIView):
+class LiveEventAttendanceAPIView(EnumeratedProgressRetrieveAPIView, DestroyAPIView):


It seems that the inheritance should be (DestroyModelMixin, EnumeratedProgressRetrieveAPIView) otherwise we have two base View classes.

smirolo · 2024-02-13T16:50:40Z

pages/api/progress.py

+    event_rank_kwarg = 'event_rank'
+
+    def get_object(self):
+        return self.progress

    def get_serializer_class(self):


The whole function might be removed and serializer_class = ... added if necessary.

smirolo · 2024-02-13T16:53:16Z

pages/api/progress.py

+        progress = self.progress
+        live_event_rank = get_extra(progress, "live_event_rank")
+
+        if live_event_rank != self.kwargs.get(self.event_rank_kwarg):


I don't understand this code. Why is there a live_event_rank in progress.extra? Couldn't the code just be something like get_object_or_404(self.progress.live_events, rank=self.kwargs.get(self.event_rank_kwarg))`?

The idea here was to check that we're retrieving the correct EnumeratedProgress object, ie. one whose 'live_event_rank' in the extra field matches the live event rank in the URL, if it exists.

For example, one page element has 2 live events(ranks 1 and 2).
We access the URL to retrieve it, api_mark_attendance(args=page element, live event(rank=1))
If we retrieve the progress for that page element, this logic here returns it only if the enumeratedprogress's extra field contains live event(rank=1), meaning progress exists for that specific live event, and wouldn't return anything for api_mark_attendance(args=page element, live event(rank=2)).

Got it. Please add a comment along those lines where the EnumeratedProgress.extra field is updated, and make sure if we try to change the value of EnumeratedProgress.extra['live_event_rank'] after it has been set once, there is some kind of warning/error being raised. Thank you!

smirolo · 2024-02-13T16:54:02Z

pages/api/progress.py

        progress = self.get_object()
        element = progress.step
-        live_event = LiveEvent.objects.filter(element=element.content).first()
+        live_event = get_object_or_404(


This is the code I would have expected in previous comment.

smirolo · 2024-02-13T16:56:21Z

pages/api/progress.py

-            progress.viewing_duration <= element.min_viewing_duration):
-            progress.viewing_duration = element.min_viewing_duration
-            progress.save()
+        if live_event.scheduled_at >= datetime_or_now():


Typically prefer to add a statement at_time = datetime_or_now() at the entry point of the method, then use at_time everywhere else. This way we only have one 'now' in the whole HTTP request.

smirolo · 2024-02-13T17:08:50Z

pages/mixins.py

@@ -314,12 +317,18 @@ class EnumeratedProgressMixin(SequenceProgressMixin):

    rank_url_kwarg = 'rank'

+    @property


Great refactor!

smirolo · 2024-02-13T17:11:21Z

pages/models.py

    location = models.URLField(_("URL to the calendar event"), max_length=2083)
    max_attendees = models.IntegerField(default=0)
+    status = models.CharField(


Put the choices as constant defined at the class level. See definition of FORMAT_CHOICES as example.

I meant write code like:

CANCELED = 'Canceled' SCHEDULED = 'Scheduled' EVENT_STATUS_CHOICES = ( (SCHEDULED, 'Scheduled'), (CANCELED, 'Canceled') )

such that you write live_event.status = LiveEvent.CANCELED

The idea is that using a named "constant", it is a lot easier to maintain the code than if strings are used all over the place. Ideally you would define SCHEDULED = 0 and status = models.SmallPositiveInteger to speed up SQL queries in the database.

smirolo · 2024-02-13T17:13:06Z

pages/serializers.py

+        help_text=_("Username of the attendee"))
+
+    class Meta(EnumeratedProgressSerializer.Meta):
+        fields = ('user', 'content', 'viewing_duration', 'min_viewing_duration')


Why is this not fields = EnumeratedProgressSerializer.fields + ('user', ...?

Because it would show the 'rank' repeatedly for each EnumeratedProgress, even though the rank is provided in the URL and therefore it would be the same for each item. And also moved the user field in the front for easier readability.

smirolo · 2024-02-13T17:17:01Z

pages/serializers.py

+        fields = ('user', 'content', 'viewing_duration', 'min_viewing_duration')
+        read_only_fields = EnumeratedProgressSerializer.Meta.read_only_fields + ('user',)
+
+    def get_user(self, obj):


We might want to return a serialized User here at some point so it is easier for the UI to display the full name of the attendee. It has not yet been done in other parts of the API (ex: Comment serializer) so this most likely should be note/issue for a future refactoring. Good to keep consistency with existing APIs for now.

smirolo · 2024-02-13T17:18:18Z

pages/views/sequences.py

@@ -71,16 +71,20 @@ def get_context_data(self, **kwargs):
 class SequencePageElementView(EnumeratedProgressMixin, TemplateView):

    template_name = 'pages/app/sequences/pageelement.html'
-
+    # is_live_event and is_certificate flags not being added


What does this comment mean, an issue to be fixed or an implementation note to be aware of?

smirolo · 2024-02-16T17:50:31Z

pages/models.py

    location = models.URLField(_("URL to the calendar event"), max_length=2083)
    max_attendees = models.IntegerField(default=0)
+    status = models.CharField(


I meant write code like:

CANCELED = 'Canceled' SCHEDULED = 'Scheduled' EVENT_STATUS_CHOICES = ( (SCHEDULED, 'Scheduled'), (CANCELED, 'Canceled') )

such that you write live_event.status = LiveEvent.CANCELED

The idea is that using a named "constant", it is a lot easier to maintain the code than if strings are used all over the place. Ideally you would define SCHEDULED = 0 and status = models.SmallPositiveInteger to speed up SQL queries in the database.

Uses the set_extra method. It initializes extra as a dictionary if it doesn't exist. An alternative could be to initialize extra as and empty string if a progress object is created in `Mixins.py` on `line 333`.

smirolo

minor optimization regression within the definition of set_extra.

smirolo · 2024-03-01T17:32:04Z

pages/helpers.py

+        # is to set it to some value.
+        obj.extra = {}
+    obj.extra.update({attr_name: attr_value})
+    obj.extra = json.dumps(obj.extra)


We don't want to serialize the extra field here (i.e. json.dumps) since the whole idea of get_extra is to unserialize it only once. Serialization should happen automatically on obj.save().

Deep-Chill added 4 commits January 30, 2024 02:19

smirolo reviewed Feb 13, 2024

View reviewed changes

Cleans up logic to mark Live Event attendance

95b9d9c

smirolo requested changes Feb 16, 2024

View reviewed changes

Minor updates

ac519ce

Deep-Chill changed the title ~~[Work-in-Progress] Scheduling of Live Events(#77)~~ Scheduling of Live Events(#77) Feb 20, 2024

Implements set_extra and other refactors

0c40bc0

Uses the set_extra method. It initializes extra as a dictionary if it doesn't exist. An alternative could be to initialize extra as and empty string if a progress object is created in `Mixins.py` on `line 333`.

smirolo approved these changes Mar 1, 2024

View reviewed changes

Removes json.dumps to save obj.extra

df5f4e6

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Scheduling of Live Events(#77) #92

Scheduling of Live Events(#77) #92

Deep-Chill commented Jan 30, 2024

smirolo left a comment

smirolo Feb 13, 2024

smirolo Feb 13, 2024

smirolo Feb 13, 2024

Deep-Chill Feb 14, 2024

smirolo Feb 14, 2024

smirolo Feb 13, 2024

smirolo Feb 13, 2024

smirolo Feb 13, 2024

smirolo Feb 13, 2024

smirolo Feb 16, 2024

smirolo Feb 13, 2024

Deep-Chill Feb 14, 2024

smirolo Feb 13, 2024

smirolo Feb 13, 2024

smirolo Feb 16, 2024

smirolo left a comment

smirolo Mar 1, 2024

		@@ -314,12 +317,18 @@ class EnumeratedProgressMixin(SequenceProgressMixin):

		rank_url_kwarg = 'rank'

		@property

Scheduling of Live Events(#77) #92

Are you sure you want to change the base?

Scheduling of Live Events(#77) #92

Conversation

Deep-Chill commented Jan 30, 2024

smirolo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smirolo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment