Improve podcast handling #330
Open
Conversation
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 is a fairly conservative change; I think a lot of this can be pushed up into the Controller logic.
The check occurs at the model level, so anything that retrieves podcasts will now ignore unpublished ones. This commit doesn't implement 404 handling for missing podcasts in the player, so this currently just triggers a 200 and a blank screen. Fixes #269.
There was a missing return - harmless but inefficient, I think?
This is better than nothing.
This might be a bit controversial, so I've left it til the end. The general idea here is that podcasts and not_found should agree on 404 error handling, and the template rendering helper I wrote earlier is generalisable to other controllers, so into controller.go they go. Comments and feedback welcomed.
It seems strange to construct the model afresh every request, when we have everything needed to construct it available in the ctor.
markspolakovs
approved these changes
Jan 22, 2024
|
|
||
| // we should not show unpublished podcasts, regardless of situation | ||
| // (https://github.com/UniversityRadioYork/2016-site/issues/269) | ||
| if pod.Status != "Published" { |
There was a problem hiding this comment.
Do we also want to make this change (i.e. invert the conditional) in GetAllPodcasts, or is the conditional there sufficient?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Predominantly an attempt to fix #269, but I've also done a few minor changes (mostly DRY) round the edges.
In ascending order of potential controversy:
http.NotFoundif it 404s, so at least there's an error thrown, even if it's ugly as sin. (I presumed that using the normal URY 404 would be silly - it won't render well in an embed.)controllersbut can be moved.Controller, which might be useful, or it might be overly coupling templates and controllers, comments welcome.I'd love to do more to improve the error handling here, but I think I need more structured error information from
myradio-go.