Add endpoint for unpublished external events to CalendarJS API #2228
Conversation
|
Code Climate has analyzed commit 3d31ee4 and detected 3 issues on this pull request. Here's the issue category breakdown:
View more on Code Climate. |
| return ["unpublished-event"] | ||
|
|
||
| def _url(self, instance): | ||
| return reverse("admin:events_externalevent_change", kwargs={"object_id": instance.id}) |
There was a problem hiding this comment.
Should this not use pk?
There was a problem hiding this comment.
pk gave an error
| @@ -25,4 +26,10 @@ | |||
| CalendarJSUnpublishedEventListView.as_view(), | |||
| name="calendarjs-unpublished", | |||
| ), | |||
| path( | |||
| "external/unpublished/", | |||
There was a problem hiding this comment.
| "external/unpublished/", | |
| "external-events/unpublished/", |
There was a problem hiding this comment.
I simply followed the conventions set by the other endpoints, and there is no external-events/ endpoint, only external/
| @@ -56,7 +57,7 @@ def get_queryset(self): | |||
| class CalendarJSUnpublishedEventListView(ListAPIView): | |||
| """Define a custom route that outputs the correctly formatted external events information for CalendarJS, unpublished events only.""" | |||
|
|
|||
| queryset = ExternalEvent.objects.filter(published=False) | |||
| queryset = Event.objects.filter(published=False) | |||
There was a problem hiding this comment.
This is the actual required bugfix @pingiun
There was a problem hiding this comment.
I think this something with the rebase that went wrong when fixing conflicts
There was a problem hiding this comment.
The rest is technically not required to fix the bug and get back to original functionality
There was a problem hiding this comment.
I think we should only have this one line fix then
There was a problem hiding this comment.
Correct that this is the fix, but I did not want to remove unpublished external events from the calendar, hence the rest of the PR.
There was a problem hiding this comment.
The rest of the PR is untested code, I just want to merge the fix so we can release on time
|
Have we dealt with this issue of not being able to see unpublished externals events yet? If not, do we still want to do this by bringing this PR up to date? |
DeD1rk
force-pushed
the
master
branch
2 times, most recently
from
41d42a7
to
18edd09
Compare
Fixes the bug that CalendarJS did not show unpublished internal events, and external events had a wrong link (to the internal event with the same ID).
Summary
A clear and concise description of the changes that you made. What bug did you solve? Or what feature did you add?
How to test
Steps to test the changes you made: