Improve podcast handling

controllers/controller.go

@@ -5,9 +5,11 @@ package controllers
 */
 
 import (
+	"log"
 	"net/http"
 
 	"github.com/UniversityRadioYork/2016-site/structs"
+	"github.com/UniversityRadioYork/2016-site/utils"
 	"github.com/UniversityRadioYork/myradio-go"
 )
 
@@ -31,48 +33,78 @@ type Controller struct {
 // Get handles a HTTP GET request r, writing to w.
 //
 // Unless overridden, controllers refuse this method.
-func (c *Controller) Get(w http.ResponseWriter, r *http.Request) {
-	http.Error(w, "Method Not Allowed", 405)
+func (*Controller) Get(w http.ResponseWriter, _ *http.Request) {
+	notAllowed(w)
 }
 
 // Post handles a HTTP POST request r, writing to w.
 //
 // Unless overridden, controllers refuse this method.
-func (c *Controller) Post(w http.ResponseWriter, r *http.Request) {
-	http.Error(w, "Method Not Allowed", 405)
+func (*Controller) Post(w http.ResponseWriter, _ *http.Request) {
+	notAllowed(w)
 }
 
 // Delete handles a HTTP DELETE request r, writing to w.
 //
 // Unless overridden, controllers refuse this method.
-func (c *Controller) Delete(w http.ResponseWriter, r *http.Request) {
-	http.Error(w, "Method Not Allowed", 405)
+func (*Controller) Delete(w http.ResponseWriter, _ *http.Request) {
+	notAllowed(w)
 }
 
 // Put handles a HTTP PUT request r, writing to w.
 //
 // Unless overridden, controllers refuse this method.
-func (c *Controller) Put(w http.ResponseWriter, r *http.Request) {
-	http.Error(w, "Method Not Allowed", 405)
+func (*Controller) Put(w http.ResponseWriter, _ *http.Request) {
+	notAllowed(w)
 }
 
 // Head handles a HTTP HEAD request r, writing to w.
 //
 // Unless overridden, controllers refuse this method.
-func (c *Controller) Head(w http.ResponseWriter, r *http.Request) {
-	http.Error(w, "Method Not Allowed", 405)
+func (*Controller) Head(w http.ResponseWriter, _ *http.Request) {
+	notAllowed(w)
 }
 
 // Patch handles a HTTP PATCH request r, writing to w.
 //
 // Unless overridden, controllers refuse this method.
-func (c *Controller) Patch(w http.ResponseWriter, r *http.Request) {
-	http.Error(w, "Method Not Allowed", 405)
+func (*Controller) Patch(w http.ResponseWriter, _ *http.Request) {
+	notAllowed(w)
 }
 
 // Options handles a HTTP OPTIONS request r, writing to w.
 //
 // Unless overridden, controllers refuse this method.
-func (c *Controller) Options(w http.ResponseWriter, r *http.Request) {
-	http.Error(w, "Method Not Allowed", 405)
+func (*Controller) Options(w http.ResponseWriter, _ *http.Request) {
+	notAllowed(w)
+}
+
+// notAllowed sends a HTTP Method Not Allowed error.
+func notAllowed(w http.ResponseWriter) {
+	http.Error(w, "Method Not Allowed", http.StatusMethodNotAllowed)
+}
+
+// notFound renders a 404 Not Found error originating from this controller.
+// It takes an optional error that represents the failure to find something,
+// and passes it into the template.
+//
+// This shouldn't be confused with NotFoundController, which handles 404s
+// originating from the user asking for a URL that doesn't exist.
+func (c *Controller) notFound(w http.ResponseWriter, err error) {
+	// TODO(@MattWindsor91): what about non-404 errors?  how do we handle those?
+	if err != nil {
+		log.Println(err)
+	}
+
+	w.WriteHeader(http.StatusNotFound)
+	c.renderTemplate(w, err, "404.tmpl")
+}
+
+// renderTemplate renders a template from this Controller's page context.
+func (c *Controller) renderTemplate(w http.ResponseWriter, data interface{}, mainTmpl string, addTmpls ...string) {
+	if err := utils.RenderTemplate(w, c.config.PageContext, data, mainTmpl, addTmpls...); err != nil {
+		// TODO(@MattWindsor91): handle error more gracefully
+		log.Println(err)
+		http.Error(w, "Internal server error", http.StatusInternalServerError)
+	}
 }

controllers/not_found.go

@@ -2,12 +2,13 @@ package controllers
 
 import (
 	"fmt"
-	"github.com/UniversityRadioYork/2016-site/models"
-	"github.com/UniversityRadioYork/myradio-go"
 	"log"
 	"net"
 	"net/http"
 
+	"github.com/UniversityRadioYork/2016-site/models"
+	"github.com/UniversityRadioYork/myradio-go"
+
 	"github.com/UniversityRadioYork/2016-site/structs"
 	"github.com/UniversityRadioYork/2016-site/utils"
 )
@@ -56,10 +57,5 @@ func (sc *NotFoundController) Get(w http.ResponseWriter, r *http.Request) {
 		http.Redirect(w, r, shortUrl.RedirectTo, http.StatusTemporaryRedirect)
 		return
 	}
-	w.WriteHeader(404)
-	err := utils.RenderTemplate(w, sc.config.PageContext, nil, "404.tmpl")
-	if err != nil {
-		log.Println(err)
-		return
-	}
+	sc.notFound(w, nil)
 }

controllers/podcasts.go

@@ -9,26 +9,26 @@ import (
 
 	"github.com/UniversityRadioYork/2016-site/models"
 	"github.com/UniversityRadioYork/2016-site/structs"
-	"github.com/UniversityRadioYork/2016-site/utils"
-	myradio "github.com/UniversityRadioYork/myradio-go"
+	"github.com/UniversityRadioYork/myradio-go"
 )
 
 // PodcastController is the controller for the URYPlayer Podcast pages.
 type PodcastController struct {
 	Controller
+	model *models.PodcastModel
 }
 
 // NewPodcastController returns a new PodcastController with the MyRadio session s
 // and configuration context c.
 func NewPodcastController(s *myradio.Session, c *structs.Config) *PodcastController {
-	return &PodcastController{Controller{session: s, config: c}}
+	return &PodcastController{
+		Controller: Controller{session: s, config: c},
+		model:      models.NewPodcastModel(s),
+	}
 }
 
 // GetAllPodcasts handles the HTTP GET request r for the all postcasts page, writing to w.
 func (podcastsC *PodcastController) GetAllPodcasts(w http.ResponseWriter, r *http.Request) {
-
-	podcastm := models.NewPodcastModel(podcastsC.session)
-
 	vars := mux.Vars(r)
 
 	pageNumber, _ := strconv.Atoi(vars["page"])
@@ -39,21 +39,21 @@ func (podcastsC *PodcastController) GetAllPodcasts(w http.ResponseWriter, r *htt
 	pageNumberNext := pageNumber + 1
 
 	//podcast page offset is indexed from 0, URL's are from 1.
-	podcasts, err := podcastm.GetAllPodcasts(10, pageNumber-1)
-
+	podcasts, err := podcastsC.model.GetAllPodcasts(10, pageNumber-1)
 	if podcasts == nil {
-		utils.RenderTemplate(w, podcastsC.config.PageContext, err, "404.tmpl")
+		podcastsC.notFound(w, err)
+		return
 	}
+
 	//see if it's possible to load another podcast for a possible next page.
-	nextPodcasts, _ := podcastm.GetAllPodcasts(1, pageNumber)
+	nextPodcasts, _ := podcastsC.model.GetAllPodcasts(1, pageNumber)
 
 	var pageNext = false
 	if nextPodcasts != nil {
 		pageNext = true
 	}
 	if err != nil {
-		log.Println(err)
-		utils.RenderTemplate(w, podcastsC.config.PageContext, err, "404.tmpl")
+		podcastsC.notFound(w, err)
 		return
 	}
 
@@ -71,79 +71,53 @@ func (podcastsC *PodcastController) GetAllPodcasts(w http.ResponseWriter, r *htt
 		Podcasts:       podcasts,
 	}
 
-	err = utils.RenderTemplate(w, podcastsC.config.PageContext, data, "podcasts.tmpl")
-	if err != nil {
-		log.Println(err)
-		return
-	}
-
+	podcastsC.renderTemplate(w, data, "podcasts.tmpl")
 }
 
 // Get handles the HTTP GET request r for a singular podcast page, writing to w.
 func (podcastsC *PodcastController) Get(w http.ResponseWriter, r *http.Request) {
+	podcast, err := podcastsC.getPodcast(r)
+	if podcast == nil || err != nil {
+		// TODO(@MattWindsor91): what if the error is not 404?
+		podcastsC.notFound(w, err)
+		return
+	}
 
-	podcastm := models.NewPodcastModel(podcastsC.session)
-
-	vars := mux.Vars(r)
-
-	id, _ := strconv.Atoi(vars["id"])
-
-	podcast, err := podcastm.Get(id)
+	podcastsC.renderPodcast(w, podcast, "podcast.tmpl")
+}
 
+// GetEmbed handles the HTTP GET request r for a singular podcast embed, writing to w.
+func (podcastsC *PodcastController) GetEmbed(w http.ResponseWriter, r *http.Request) {
+	podcast, err := podcastsC.getPodcast(r)
 	if err != nil {
 		log.Println(err)
-		utils.RenderTemplate(w, podcastsC.config.PageContext, nil, "404.tmpl")
+		http.NotFound(w, r)
 		return
 	}
 
-	if podcast.Status != "Published" {
-		utils.RenderTemplate(w, podcastsC.config.PageContext, nil, "404.tmpl")
+	// No error, but podcast is not available
+	if podcast == nil {
+		http.NotFound(w, r)
 		return
 	}
 
-	data := struct {
-		Podcast *myradio.Podcast
-	}{
-		Podcast: podcast,
-	}
-
-	err = utils.RenderTemplate(w, podcastsC.config.PageContext, data, "podcast.tmpl")
+	podcastsC.renderPodcast(w, podcast, "podcast_player.tmpl")
+}
 
+func (podcastsC *PodcastController) getPodcast(r *http.Request) (*myradio.Podcast, error) {
+	id, err := strconv.Atoi(mux.Vars(r)["id"])
 	if err != nil {
-		log.Println(err)
-		return
+		return nil, err
 	}
 
+	return podcastsC.model.Get(id)
 }
 
-// GetEmbed handles the HTTP GET request r for a singular podcast embed, writing to w.
-func (podcastsC *PodcastController) GetEmbed(w http.ResponseWriter, r *http.Request) {
-
-	podcastm := models.NewPodcastModel(podcastsC.session)
-
-	vars := mux.Vars(r)
-
-	id, _ := strconv.Atoi(vars["id"])
-
-	podcast, err := podcastm.Get(id)
-
-	if err != nil {
-		//@TODO: Do something proper here, render 404 or something
-		log.Println(err)
-		return
-	}
-
+func (podcastsC *PodcastController) renderPodcast(w http.ResponseWriter, podcast *myradio.Podcast, tmpl string) {
 	data := struct {
 		Podcast *myradio.Podcast
 	}{
 		Podcast: podcast,
 	}
-
-	err = utils.RenderTemplate(w, podcastsC.config.PageContext, data, "podcast_player.tmpl")
-
-	if err != nil {
-		log.Println(err)
-		return
-	}
-
+	podcastsC.renderTemplate(w, data, tmpl)
 }

models/podcasts.go

@@ -33,6 +33,18 @@ func (m *PodcastModel) GetAllPodcasts(number int, page int) (podcasts []myradio.
 }
 
 // Get gets the data required for the Podcast controller from MyRadio.
-func (m *PodcastModel) Get(id int) (podcast *myradio.Podcast, err error) {
-	return m.session.GetPodcastWithShow(id)
+// It does not retrieve a podcast if the podcast is unpublished.
+func (m *PodcastModel) Get(id int) (*myradio.Podcast, error) {
+	pod, err := m.session.GetPodcastWithShow(id)
+	if err != nil {
+		return nil, err
+	}
+
+	// we should not show unpublished podcasts, regardless of situation
+	// (https://github.com/UniversityRadioYork/2016-site/issues/269)
+	if pod.Status != "Published" {
+		return nil, nil
+	}
+
+	return pod, nil
 }