More Code Examples from the Programming Phoenix book #42

nelsonic · 2017-02-27T12:23:09Z

@iteles adding code examples for the Programming Phoenix book
(I've personally written and tested the examples and been opening issues along the way e.g #38 so other people can learn faster...) 👍

Fixes #35, #49 & #50

Please review/merge when you can thanks.

… but: #35 ...)

…@Shouston3

…@Shouston3 #34 (comment) fixes #34

…t all tests pass! 🎉

…ted_to

…lates)

samhstn

Just a couple of comments, but looks good.

Happy to merge after those are addressed.

I have been thinking about the syntax a little and would like your thoughts on this:

We use Word Sigils throughout the book, but I feel that using just a normal List of strings instead would be more readable in general (perhaps I will find these to be nicer when I'm more used to the syntax).
The String.contains syntax is initally more readable, but I have found the =~ syntax to be much nicer once you are used to it

Lastly, since we are running tests in the project, perhaps we could get these hosted?

samhstn · 2017-03-01T13:05:13Z

rumbl/priv/repo/seeds.exs

+
+for category <- ~w(Action Drama Romance Comedy Sci-fi) do
+  Repo.insert!(%Category{name: category}) ||
+    Repo.insert!(%Category{name: category})


I think these should be different, I have:

Repo.get_by(Category, name: category) || Repo.insert!(%Category{name: category})

instead

@Shouston3 good shout. please feel free to edit the file in PR. 👍
else I will fix tomorrow morning. 😉

samhstn · 2017-03-01T13:07:05Z

rumbl/priv/repo/migrations/20170218093549_create_category.exs

+    create table(:categories) do
+      add :name, :string, null: false
+
+      timestamps() # function call parenthesis not in book!


samhstn · 2017-03-01T13:09:14Z

rumbl/test/controllers/auth_test.exs

+  end
+
+  test "login with password missmatch", %{conn: conn} do
+    _ = insert_user(username: "me", password: "secrete")


"secrete" typo

yeah, worth fixing. thanks. 👍

samhstn · 2017-03-01T13:13:29Z

rumbl/test/models/user_test.exs

+  # test "changeset with invalid attributes" do
+  #   changeset = User.changeset(%User{}, @invalid_attrs)
+  #   refute changeset.valid?
+  # end


I also have these comments in mine 👍

samhstn · 2017-03-01T13:17:04Z

rumbl/web/controllers/video_controller.ex

-
+  def create(conn, %{"video" => video_params}, user) do
+    IO.inspect video_params
+    IO.inspect user


Perhaps remove these IO.inspects

samhstn · 2017-03-01T13:17:12Z

rumbl/web/controllers/video_controller.ex

+    user
+      |> build_assoc(:videos)
+      |> Video.changeset(video_params)
+    IO.inspect changeset


IO.inspect

Will remove. in next commit. 👍

samhstn · 2017-03-01T13:18:40Z

rumbl/web/models/video.ex


-    timestamps()
+    timestamps() # invocation parenthesis not in book


Ugh this has been confusing us with warnings

@des-des are you running the latest version of Phoenix?

@nelsonic yeah 1.2.1

des-des · 2017-03-05T09:31:41Z

rumbl/web/static/js/player.js

@@ -0,0 +1,29 @@
+let Player = {


JS -> Elm?!

@des-des this example is from "The Book" ...
I don't intend to keep the ES6 after I've finished writing out the code. 😉

…sts pass!

…otocol

…com/dwyl/learn-travis

….com/questions/14468464/how-to-specify-a-build-folder-in-travis

…vis-ci.org/dwyl/learn-phoenix-framework/builds/207893410#L189

…nd of rumbl/web/models/video.ex

…set.cast/4` is deprecated, please use `cast/3` + `validate_required/3` instead

…StackOverflow question ... see: #52

…bert

…bert in http://stackoverflow.com/questions/42609234/defimpl-phoenix-param-to-override-to-param-causes-functionclauseerror-no-funct?noredirect=1#comment72348042_42609234

nelsonic · 2017-03-05T20:17:37Z

Totes works!

…42 (comment)

@Shouston3

…d by @Shouston3 in #42 (comment)

@des-des

… noted by @des-des in #42 (review)

nelsonic · 2017-03-06T02:34:26Z

@iteles I've addressed the review comments from @Shouston3 & @des-des
and added Travis-CI to confirm that the demo app is working.
Please take a look at the code when you have time and merge if ok.
thanks! 👍

#35 (comment) fixes #35

des-des · 2017-03-06T07:17:16Z

rumbl/web/static/js/player.js

-let Player = {
-  player: null,
-
+let Player = {  // JS works for small scripts like this


Also loving this pragmatism 👍

iteles

This has been pretty fascinating to review. Nothing to add for now, thanks for putting it together!

nelsonic added 13 commits February 18, 2017 09:29

adds update to controllers/video_controller.ex from ch 6 (p. 105)

a2f798d

enforce categories reference in videos. p.109

68adc2b

adds snapshot of rumbl app at end of chapter 7 (p.128)

e2065b3

snapshot of rumbl code before adding integration tests p.134 (passing…

dffbb51

… but: #35 ...)

adds comment to basic-test.exs for how to run a single test. thanks to …

a034625

…@Shouston3 #34 (comment) fixes #34

remove un-required _build files

14bfa97

snapshot of rumbl code before testing for logged-in users on p.138

63efdf3

[WiP] adds code @tag login_as: "max" (sadly test failing! see: #38 ...)

e83b58f

fix #38 test for logged-in user on p.139 still seeing #35 but at leas…

f11e96d

…t all tests pass! 🎉

all tests passing on p.140 but see issue #40 so commented out redirec…

fca2449

…ted_to

adds snapshot of "rumbl" code on p.147 (before Testing Views and Temp…

cd0cb73

…lates)

adds "rumbl" View tests from p.147 & p.148

30d637c

adds final user and category tests for rumbl. all pass on p.153 🚀

cb701c6

nelsonic self-assigned this Feb 27, 2017

fix merge conflicts from master

147514c

nelsonic assigned iteles and unassigned nelsonic Feb 27, 2017

nelsonic requested a review from iteles February 27, 2017 18:56

nelsonic assigned samhstn and unassigned iteles Feb 28, 2017

nelsonic requested a review from samhstn February 28, 2017 10:22

p.159 code snapshot

69fcb56

samhstn reviewed Mar 1, 2017

View reviewed changes

samhstn assigned nelsonic and unassigned samhstn Mar 1, 2017

p.163 code snapshot including web/static/js/player.js

f2a17a2

des-des reviewed Mar 5, 2017

View reviewed changes

nelsonic added 3 commits March 5, 2017 11:09

p.166 slugify_title and slugify working as expected also fixed #50 te…

08f92dc

…sts pass!

p.166 adds |> slugify_title() to models/video.ex changeset

09e9092

p.167 adds defimpl for creating slugs "#{id}-#{slug}" using Elixir pr…

0ab4581

…otocol

nelsonic added 9 commits March 5, 2017 12:47

adds .travis.yml file so we can track the build! see: https://github.…

c7075e1

…com/dwyl/learn-travis

use before_install instead of before_script see: http://stackoverflow…

eebd057

….com/questions/14468464/how-to-specify-a-build-folder-in-travis

give full path for "change directory" in .travis.yml see: https://tra…

aaf3cda

…vis-ci.org/dwyl/learn-phoenix-framework/builds/207893410#L189

one-liner for .travis.yml before_install script

837db9e

p.167 adds defimpl Phoenix.Param, for: Rumbl.Video as instructed at e…

5860104

…nd of rumbl/web/models/video.ex

update changeset in web/models/vide.ex to avoid warning: `Ecto.Change…

96c9f64

…set.cast/4` is deprecated, please use `cast/3` + `validate_required/3` instead

add defimpl Phoenix.Param to_param back into web/models/video.ex for …

db72acb

…StackOverflow question ... see: #52

adds field :slug, :string to web/models/video.ex as suggested by @dog…

27c4a11

…bert in http://stackoverflow.com/questions/42609234/defimpl-phoenix-param-to-override-to-param-causes-functionclauseerror-no-funct?noredirect=1#comment72348042_42609234

adds clean_id for: #52 (comment)

954fab8

nelsonic added 5 commits March 5, 2017 20:18

p.171 everything works. tests pass. 🚀 #42 (comment)

873465a

update rumbl/priv/repo/seeds.exs in response to @Shouston3 comment: #…

0dc090e

…42 (comment)

fix typo in rumbl/test/controllers/auth_test.exs "secrete" > "secret" #…

b2e0da3

…42 (comment)

remove IO.inspect from rumbl/web/controllers/video_controller.ex note…

b0ea31b

…d by @Shouston3 in #42 (comment)

adds clarifying comment to player.js explaining that we prefer Elm as…

cfe5333

… noted by @des-des in #42 (review)

nelsonic assigned iteles and unassigned nelsonic Mar 6, 2017

adds validate_required check for web/models/user.ex thanks to @untra in

7d222ef

#35 (comment) fixes #35

des-des approved these changes Mar 6, 2017

View reviewed changes

iteles approved these changes Mar 6, 2017

View reviewed changes

iteles merged commit 59cb5d2 into master Mar 6, 2017

iteles deleted the code-examples branch March 6, 2017 09:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

More Code Examples from the Programming Phoenix book #42

More Code Examples from the Programming Phoenix book #42

nelsonic commented Feb 27, 2017 •

edited

Loading

samhstn left a comment •

edited

Loading

samhstn Mar 1, 2017

nelsonic Mar 1, 2017

samhstn Mar 1, 2017

samhstn Mar 1, 2017

nelsonic Mar 1, 2017

samhstn Mar 1, 2017 •

edited

Loading

samhstn Mar 1, 2017

samhstn Mar 1, 2017

nelsonic Mar 1, 2017

samhstn Mar 1, 2017

des-des Mar 2, 2017

nelsonic Mar 3, 2017

des-des Mar 3, 2017

des-des Mar 5, 2017

nelsonic Mar 5, 2017

des-des Mar 5, 2017

nelsonic commented Mar 5, 2017

nelsonic commented Mar 6, 2017

des-des Mar 6, 2017

iteles Mar 6, 2017

iteles left a comment


		timestamps()
		timestamps() # invocation parenthesis not in book

More Code Examples from the Programming Phoenix book #42

More Code Examples from the Programming Phoenix book #42

Conversation

nelsonic commented Feb 27, 2017 • edited Loading

samhstn left a comment • edited Loading

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

samhstn Mar 1, 2017 • edited Loading

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

nelsonic commented Mar 5, 2017

nelsonic commented Mar 6, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iteles left a comment

Choose a reason for hiding this comment

nelsonic commented Feb 27, 2017 •

edited

Loading

samhstn left a comment •

edited

Loading

samhstn Mar 1, 2017 •

edited

Loading