From d58cb33fcb6b15656c819ba428b28fcb90365955 Mon Sep 17 00:00:00 2001 From: RobertAKARobin Date: Wed, 2 Sep 2015 08:36:17 -0400 Subject: [PATCH] robin's comments --- .gitignore | 2 +- app.js | 6 +++++- controllers/playlists.js | 6 ++++++ env.js | 2 ++ public/js/models/playlist.js | 1 + public/js/script.js | 8 +++++--- public/js/views/playlistView.js | 1 + public/js/views/userView.js | 1 + 8 files changed, 22 insertions(+), 5 deletions(-) diff --git a/.gitignore b/.gitignore index 5214d1b..ade7c86 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,3 @@ -env.js + node_modules .DS_Store diff --git a/app.js b/app.js index dca8675..888ac40 100644 --- a/app.js +++ b/app.js @@ -21,6 +21,7 @@ pg.connect(process.env.DATABASE_URL, function(err, client) { console.log(JSON.stringify(row)); }); }); +// Wow, this is interesting! You could use Sequelize to take care of all this... // var User = require("./db/connection").models.User; @@ -39,7 +40,7 @@ if (fs.existsSync("./env.js")){ else { var env = process.env; } - +// You shouldn't need FS to require a file. var passport = require("passport") var SpotifyStrategy = require("passport-spotify").Strategy; @@ -61,6 +62,7 @@ passport.use(new SpotifyStrategy({ // console.log(user.get({ // plain: true // })) + // Lots of commented-out stuff hanging around in here console.log(created) }); } @@ -134,3 +136,5 @@ app.get('/auth/spotify/callback', // app.listen(3000, function(){ // console.log("Listening on port 3000"); // }); + +// This is a really long app.js! Might be a good idea to extract stuff into other files. For instance, you could put all the Spotify Passport stuff in a separate file and `require` it. It would still load and run when you start your app. diff --git a/controllers/playlists.js b/controllers/playlists.js index 8de1eb7..50f89e9 100644 --- a/controllers/playlists.js +++ b/controllers/playlists.js @@ -21,6 +21,12 @@ router.get("/playlists", function(req, res){ }) }); + // User.findOne({ where: {spotifyId: req.session.profile.id}}) + // .then(function(user){ + // return Playlist.findAll({where: {userId: user.id}}) + // }).then(function(playlists, err){ + // res.json(playlists); + // }); }); //POST to playlists diff --git a/env.js b/env.js index 615c9cd..75e345b 100644 --- a/env.js +++ b/env.js @@ -3,3 +3,5 @@ consumerKey: "ebc5b7965dcd40d5ba4fe24e80e0aeb8", consumerSecret: "0812e8628dcd4752b05db36cc76ee833", callbackUrl: "https://theplaylistr.herokuapp.com/auth/spotify/callback" } + +// This was included in your Github repo. I'm not sure why .gitignore didn't work. On the plus side, it made it a lot easier to test your app! diff --git a/public/js/models/playlist.js b/public/js/models/playlist.js index 80f28e3..b6031c4 100644 --- a/public/js/models/playlist.js +++ b/public/js/models/playlist.js @@ -43,6 +43,7 @@ Playlist.prototype = { destroy: function() { // console.log("in destroy"); //console.log(data); + // So much commented-out code! var url = "https://theplaylistr.herokuapp.com/playlists/" + this.id; var request = $.ajax({ url: url, diff --git a/public/js/script.js b/public/js/script.js index 3f0ba46..1a79cec 100644 --- a/public/js/script.js +++ b/public/js/script.js @@ -112,7 +112,7 @@ function getArtistBio(artist) { appendArtistBio(artistBio); }) } - +// I would have extracted all of this "artist" stuff into a separate Artist model. Remember that models are just a way of organizing code related to a certain kind of data -- but they don't HAVE to connect to a database. // display the artist bio in the .biography div function appendArtistBio(artistBio){ $(".biography").html(""); @@ -179,7 +179,7 @@ function getConcertInfo(artist) { appendConcertInfo(events); }) } - +// In the same lieu as artists, a concert model might be a good idea. // display concert info, including link to event, venue and ticket purchase link function appendConcertInfo(events){ $(".concerts").children().remove(); @@ -306,7 +306,7 @@ $('.otherLocation').keypress(function(e) { } }); - +// Put this in a separate file // song lyric array for random song lyrics at bottom var songLyrics = ["There was something so pleasant about that place.", "I wanna dance tonight. I wanna toast tonight.", "My umi said shine your light on the world.", "Making momma so proud. But your voice is too loud.", "Crank up the Beach Boys baby, don\'t let the music stop.", "Showin\' how funky strong is your fight", "Somebody said you got a new friend.", "I get jealous, but I\'m too cool to admit it.", "You gotta give me everything, baby, ain\'t no doubt. Need U 100%.", "Et si je compte et je compterai pour toi, je te conterai mes histoires.", "You’re kind of amazing like a time machine.", "The Digital Buddha is coming for you.", "On my tricycle, I’m in heaven, she’s my tricycle, I’m her melon.", "My ship at sail can climb a mountain, Ride it to the sky.", "You look like someone I know, where did I meet you and where do we go?", "And hope fuels generations. And hope can start your car. And hope is the root of fantasy. It\'s nothing but a star.", "And I found peace on the waiting room floor.", "You make me feel alive like a parachute in the sky", "Welcome to the crazy part of town. We like to take your life and turn it upside down", "In the shadow of the digital buddha are the countless wise, to bring you inspiration that you cannot buy.", "In the shadow of the digital buddha are the countless wise, to lift our generation to the highest high.", "Cheers to you it\'s like a dream you live by seeking fortunes at the door", "Baby I\'m in space when I\'m with you... Floating around nothing to do, maybe we can take a walk on the moon", "I can see it in your eyes, that you don\'t know what to say, I say goodbye, and walk away", "Cause tonight we can\'t do anything wrong", "Take me down to 34th street, where The Disco Biscuits are the king of the beat.", "Is there a pot of gold, at the end of this sweet rainbow?", "Always knew my home was in paradise.", "Hey little baby, now the morning\'s here. Gonna open my eyes, don\'t disappear."] @@ -319,3 +319,5 @@ function getLyric() { $(document).ready(function(){ $(".songLyrics").html(getLyric()); }) + +// This file's like 250 lines too long! diff --git a/public/js/views/playlistView.js b/public/js/views/playlistView.js index 83fe7a2..774fbf8 100644 --- a/public/js/views/playlistView.js +++ b/public/js/views/playlistView.js @@ -29,6 +29,7 @@ PlaylistView.prototype = { self.playlist.destroy(data).then(function() { self.$el.fadeOut()}); }); }, + // If all these methods are part of the Playlist object, you don't really need to put "playlist" in the methods' names updatePlaylist: function() { var self = this; // console.log(this.playlist.id); diff --git a/public/js/views/userView.js b/public/js/views/userView.js index 1d7da3d..c17d9b8 100644 --- a/public/js/views/userView.js +++ b/public/js/views/userView.js @@ -2,6 +2,7 @@ var UserView = function(user){ this.user = user; this.$el = $("
"); }; +// Goodness! // // User.fetch = function(){ // // saving the ajax request to a local variable