Merge pull request #196 from stride-nyc/refactor/remove-author-column…

…-from-idea

Refactor/remove author column from idea

priv/repo/migrations/20170713190446_remove_author_from_idea.exs

@@ -0,0 +1,9 @@
+defmodule RemoteRetro.Repo.Migrations.RemoveAuthorFromIdea do
+  use Ecto.Migration
+
+  def change do
+    alter table(:ideas) do
+      remove :author
+    end
+  end
+end

test/actions/idea_actions_test.js

@@ -2,7 +2,7 @@ import * as actionCreators from "../../web/static/js/actions/idea"
 
 describe("addIdea", () => {
   it("creates an action to add idea to store", () => {
-    const idea = { body: "we have a linter!", category: "happy", author: "Kimberly Suazo" }
+    const idea = { body: "we have a linter!", category: "happy", user_id: 1 }
 
     expect(actionCreators.addIdea(idea)).to.deep.equal({ type: "ADD_IDEA", idea })
   })

test/channels/retro_channel_test.exs

@@ -83,9 +83,9 @@ defmodule RemoteRetro.RetroChannelTest do
     @tag user: @mock_user
     test "results in the broadcast of the new idea to all connected clients", %{socket: socket, user: user} do
       user_id = user.id
-      push(socket, "new_idea", %{category: "happy", body: "we're pacing well", author: "Travis", userId: user_id})
+      push(socket, "new_idea", %{category: "happy", body: "we're pacing well", userId: user_id})
 
-      assert_broadcast("new_idea_received", %{category: "happy", body: "we're pacing well", id: _, author: "Travis", user_id: ^user_id})
+      assert_broadcast("new_idea_received", %{category: "happy", body: "we're pacing well", id: _, user_id: ^user_id})
     end
   end
 
@@ -129,7 +129,7 @@ defmodule RemoteRetro.RetroChannelTest do
     setup [:persist_user_for_retro, :persist_idea_for_retro, :join_the_retro_channel]
 
     @tag user: @mock_user
-    @tag idea: %Idea{category: "sad", body: "JavaScript", author: "Maryanne"}
+    @tag idea: %Idea{category: "sad", body: "JavaScript"}
     test "results in the broadcast of the edited idea to all connected clients", %{socket: socket, idea: idea} do
       idea_id = idea.id
       push(socket, "idea_edited", %{id: idea_id, body: "hell's bells"})
@@ -139,7 +139,7 @@ defmodule RemoteRetro.RetroChannelTest do
 
 
     @tag user: @mock_user
-    @tag idea: %Idea{category: "sad", body: "doggone keeper", author: "Maryanne"}
+    @tag idea: %Idea{category: "sad", body: "doggone keeper"}
     test "results in the idea being updated in the database", %{socket: socket, idea: idea} do
       idea_id = idea.id
       push(socket, "idea_edited", %{id: idea_id, body: "hell's bells"})
@@ -154,7 +154,7 @@ defmodule RemoteRetro.RetroChannelTest do
     setup [:persist_user_for_retro, :join_the_retro_channel, :persist_idea_for_retro]
 
     @tag user: @mock_user
-    @tag idea: %Idea{category: "sad", body: "WIP commits on master", author: "Zander"}
+    @tag idea: %Idea{category: "sad", body: "WIP commits on master"}
     test "results in a broadcast of the id of the deleted idea to all clients", %{socket: socket, idea: idea} do
       idea_id = idea.id
       push(socket, "delete_idea", idea_id)

test/components/category_column_test.js

@@ -13,14 +13,17 @@ describe("CategoryColumn", () => {
       id: 5,
       body: "should be third",
       category: "confused",
+      user: mockUser,
     }, {
       id: 2,
       body: "should be first",
       category: "confused",
+      user: mockUser,
     }, {
       id: 4,
       body: "should be second",
       category: "confused",
+      user: mockUser,
     }]
 
     it("it renders them sorted by id ascending", () => {
@@ -46,10 +49,12 @@ describe("CategoryColumn", () => {
         id: 1,
         body: "tests!",
         category: "happy",
+        user: mockUser,
       }, {
         id: 2,
         body: "winter break!",
         category: "happy",
+        user: mockUser,
       }]
 
       const wrapper = shallow(
@@ -70,6 +75,7 @@ describe("CategoryColumn", () => {
         id: 1,
         body: "still no word on tests",
         category: "confused",
+        user: mockUser,
       }]
 
       const differentCategory = "happy"

test/components/idea_controls_test.js

@@ -5,7 +5,7 @@ import sinon from "sinon"
 import IdeaControls from "../../web/static/js/components/idea_controls"
 
 describe("<IdeaControls />", () => {
-  const idea = { id: 666, category: "sad", body: "redundant tests", author: "Trizzle" }
+  const idea = { id: 666, category: "sad", body: "redundant tests", user_id: 1 }
 
   describe("on click of the removal icon", () => {
     it("pushes an `delete_idea` event to the retro channel, passing the given idea's id", () => {

test/components/idea_edit_form_test.js

@@ -5,7 +5,7 @@ import sinon from "sinon"
 import IdeaEditForm from "../../web/static/js/components/idea_edit_form"
 
 describe("<IdeaEditForm />", () => {
-  const idea = { id: 999, category: "sad", body: "redundant tests", author: "Trizzle" }
+  const idea = { id: 999, category: "sad", body: "redundant tests", userId: 1 }
   const mockRetroChannel = { on: () => {}, push: () => {} }
   const defaultProps = { idea, retroChannel: mockRetroChannel }
 

test/components/idea_submission_form_test.js

@@ -32,7 +32,6 @@ describe("IdeaSubmissionForm component", () => {
         retroChannel.push.calledWith("new_idea", {
           category: "happy",
           body: "",
-          author: "Mugatu",
           userId: 1,
         }
       )).to.equal(true)

test/components/idea_test.js

@@ -6,7 +6,14 @@ import IdeaControls from "../../web/static/js/components/idea_controls"
 import IdeaEditForm from "../../web/static/js/components/idea_edit_form"
 
 describe("Idea component", () => {
-  const idea = { category: "sad", body: "redundant tests", author: "Trizzle" }
+  const idea = {
+    category: "sad",
+    body: "redundant tests",
+    user_id: 1,
+    user: {
+      given_name: "Phil",
+    },
+  }
   const mockRetroChannel = { on: () => {}, push: () => {} }
   const mockUser = {}
 
@@ -134,6 +141,9 @@ describe("Idea component", () => {
     const editedIdea = {
       inserted_at: "2017-04-14T17:30:10",
       updated_at: "2017-04-14T17:30:12",
+      user: {
+        given_name: "Liz",
+      },
     }
 
     const wrapper = shallow(

test/features/facilitator_highlights_an_idea_test.exs

@@ -8,7 +8,7 @@ defmodule FacilitatorHighlightsAnIdeaTest do
     setup [:persist_user_for_retro, :persist_idea_for_retro]
 
     @tag user: Map.put(@mock_user, "email", "hiro@protagonist.com")
-    @tag idea: %Idea{category: "happy", body: "Teams worked well together", author: "Participant"}
+    @tag idea: %Idea{category: "happy", body: "Teams worked well together"}
     test "the idea that the facilitator clicked on toggles highlighted class for everyone", %{session: facilitator_session, retro: retro} do
       idea_body = "Teams worked well together"
       participant_session = new_browser_session()

test/features/preexisting_retro_state_rendered_on_joining_retro_test.exs

@@ -1,9 +1,10 @@
 defmodule PreExistingRetroStateRenderedOnJoiningRetroTest do
   use RemoteRetro.IntegrationCase, async: false
-  alias RemoteRetro.{Idea, Retro}
+  alias RemoteRetro.{Idea, Retro, User}
 
   test "the rendering of ideas submitted prior to the user joining", %{session: session, retro: retro} do
-    Repo.insert!(%Idea{ category: "happy", body: "continuous delivery!", retro_id: retro.id, author: "Travis" })
+    user = Repo.get_by(User, email: "mistertestuser@gmail.com")
+    Repo.insert!(%Idea{ category: "happy", body: "continuous delivery!", retro_id: retro.id, user_id: user.id})
 
     retro_path = "/retros/" <> retro.id
     session = visit(session, retro_path)
@@ -15,8 +16,9 @@ defmodule PreExistingRetroStateRenderedOnJoiningRetroTest do
 
   describe "when a retro has already progressed to the `action-items` stage" do
     test "new visitors see the action item interface", %{session: session} do
+      user = Repo.get_by(User, email: "mistertestuser@gmail.com")
       retro = Repo.insert!(%Retro{stage: "action-items"})
-      Repo.insert!(%Idea{category: "action-item", body: "set up CI", retro_id: retro.id, author: "Travis"})
+      Repo.insert!(%Idea{category: "action-item", body: "set up CI", retro_id: retro.id, user_id: user.id})
 
       retro_path = "/retros/" <> retro.id
       session = visit(session, retro_path)

test/features/retro_idea_realtime_update_test.exs

@@ -24,7 +24,7 @@ defmodule RetroIdeaRealtimeUpdateTest do
     setup [:persist_user_for_retro, :persist_idea_for_retro]
 
     @tag user: Map.put(@mock_user, "email", "hiro@protagonist.com")
-    @tag idea: %Idea{category: "sad", body: "no linter", author: "Participant"}
+    @tag idea: %Idea{category: "sad", body: "no linter"}
     test "the immediate update of ideas edited by the facilitator", %{session: facilitator_session, retro: retro} do
       participant_session = new_browser_session()
 
@@ -43,7 +43,7 @@ defmodule RetroIdeaRealtimeUpdateTest do
     end
 
     @tag user: Map.put(@mock_user, "email", "hiro@protagonist.com")
-    @tag idea: %Idea{category: "happy", body: "slack time!", author: "Participant"}
+    @tag idea: %Idea{category: "happy", body: "slack time!"}
     test "the immediate removal of an idea deleted by the facilitator", %{session: facilitator_session, retro: retro} do
       participant_session = new_browser_session()
 

test/reducers/idea_reducer_test.js

@@ -15,7 +15,7 @@ describe("idea reducer", () => {
 
     describe("and there is initial state", () => {
       it("should return that initial state", () => {
-        const initialState = [{ body: "we have a linter!", category: "happy", author: "Kimberly Suazo" }]
+        const initialState = [{ body: "we have a linter!", category: "happy", user_id: 1 }]
         const unhandledAction = { type: "IHAVENOIDEAWHATSHAPPENING" }
 
         expect(ideaReducer(initialState, {})).to.deep.equal(initialState)
@@ -27,9 +27,9 @@ describe("idea reducer", () => {
   describe("the handled actions", () => {
     describe("when the action is ADD_IDEA", () => {
       it("should add an idea to list of ideas", () => {
-        const initialState = [{ body: "i'm an old idea!", category: "happy", author: "Morty" }]
+        const initialState = [{ body: "i'm an old idea!", category: "happy", user_id: 2 }]
         deepFreeze(initialState)
-        const idea = { body: "we have a linter!", category: "happy", author: "Kimberly Suazo" }
+        const idea = { body: "we have a linter!", category: "happy", user_id: 1 }
         const action = { type: "ADD_IDEA", idea }
 
         expect(ideaReducer(initialState, action)).to.deep.equal([...initialState, idea])
@@ -38,37 +38,37 @@ describe("idea reducer", () => {
 
     describe("when the action is SET_INITIAL_STATE", () => {
       it("should replace the state with the ideas passed in the action's inialState object", () => {
-        const initialIdeas = [{ body: "i'm an old idea!", category: "happy", author: "Morty" }]
+        const initialIdeas = [{ body: "i'm an old idea!", category: "happy", user_id: 2 }]
         deepFreeze(initialIdeas)
 
-        const newIdeas = [{ body: "modern convenience", category: "confused", author: "Kimberly Suazo" }]
+        const newIdeas = [{ body: "modern convenience", category: "confused", user_id: 1 }]
         const action = { type: "SET_INITIAL_STATE", initialState: { ideas: newIdeas } }
 
         expect(ideaReducer(initialIdeas, action)).to.deep.equal([...newIdeas])
       })
     })
 
     describe("when the action is UPDATE_IDEA", () => {
-      const initialIdeas = [{ id: 666, category: "happy", author: "Kimberly" }, { id: 22, category: "n/a", author: "Travis" }]
+      const initialIdeas = [{ id: 666, category: "happy", user_id: 1 }, { id: 22, category: "n/a", user_id: 2 }]
       deepFreeze(initialIdeas)
 
       it("returns an updated set of ideas, where the idea with matching id has updated attributes", () => {
         const action = { type: "UPDATE_IDEA", ideaId: 666, newAttributes: { category: "sad" } }
         expect(ideaReducer(initialIdeas, action)).to.deep.equal([
-          { id: 666, category: "sad", author: "Kimberly" },
-          { id: 22, category: "n/a", author: "Travis" },
+          { id: 666, category: "sad", user_id: 1 },
+          { id: 22, category: "n/a", user_id: 2 },
         ])
       })
     })
 
     describe("when the action is DELETE_IDEA", () => {
-      const initialIdeas = [{ id: 667, category: "happy", author: "Kimberly" }, { id: 22, category: "n/a", author: "Travis" }]
+      const initialIdeas = [{ id: 667, category: "happy", user_id: 1 }, { id: 22, category: "n/a", user_id: 2 }]
       deepFreeze(initialIdeas)
 
       it("returns an updated set of ideas, where the idea with matching id has been removed", () => {
         const action = { type: "DELETE_IDEA", ideaId: 667 }
         expect(ideaReducer(initialIdeas, action)).to.deep.equal([
-          { id: 22, category: "n/a", author: "Travis" },
+          { id: 22, category: "n/a", user_id: 2 },
         ])
       })
     })

web/channels/retro_channel.ex

@@ -5,7 +5,7 @@ defmodule RemoteRetro.RetroChannel do
 
   def join("retro:" <> retro_id, _, socket) do
     socket = assign(socket, :retro_id, retro_id)
-    retro = Repo.get!(Retro, retro_id) |> Repo.preload(:ideas)
+    retro = Repo.get!(Retro, retro_id) |> Repo.preload(ideas: :user)
 
     send self(), :after_join
     {:ok, retro, socket}
@@ -42,17 +42,17 @@ defmodule RemoteRetro.RetroChannel do
     {:noreply, socket}
   end
 
-  def handle_in("new_idea", %{"body" => body, "category" => category, "author" => author, "userId" => user_id}, socket) do
+  def handle_in("new_idea", %{"body" => body, "category" => category, "userId" => user_id}, socket) do
     idea =
       %Idea{
         body: body,
         category: category,
         retro_id: socket.assigns.retro_id,
-        author: author,
         user_id: user_id
       }
       |> Idea.changeset
       |> Repo.insert!
+      |> Repo.preload(:user)
 
     broadcast! socket, "new_idea_received", idea
     {:noreply, socket}
@@ -63,6 +63,7 @@ defmodule RemoteRetro.RetroChannel do
       Repo.get(Idea, id)
       |> Idea.changeset(%{body: body})
       |> Repo.update!
+      |> Repo.preload(:user)
 
     broadcast! socket, "idea_edited", idea
     {:noreply, socket}

web/models/idea.ex

@@ -5,15 +5,14 @@ defmodule RemoteRetro.Idea do
   schema "ideas" do
     field :category, :string
     field :body, :string
-    field :author, :string
 
     belongs_to :retro, RemoteRetro.Retro, type: Ecto.UUID
     belongs_to :user, RemoteRetro.User
 
     timestamps()
   end
 
-  @required_fields [:category, :body, :retro_id, :author, :user_id]
+  @required_fields [:category, :body, :retro_id, :user_id]
 
   def changeset(struct, params \\ %{}) do
     struct

web/static/js/components/idea.jsx

@@ -23,7 +23,7 @@ const Idea = props => {
       }
       { isFacilitator && <IdeaControls idea={idea} retroChannel={retroChannel} /> }
       <span className={styles.authorAttribution}>
-        {idea.author}:
+        {idea.user.given_name}:
       </span> {idea.liveEditText || idea.body}
       {isEdited && <span className={styles.editedIndicator}> (edited)</span>}
     </div>

web/static/js/components/idea_submission_form.jsx

@@ -37,7 +37,7 @@ class IdeaSubmissionForm extends Component {
   handleSubmit(event) {
     const { currentUser } = this.props
     event.preventDefault()
-    const newIdea = { ...this.state, author: currentUser.given_name, userId: currentUser.id }
+    const newIdea = { ...this.state, userId: currentUser.id }
     this.props.retroChannel.push("new_idea", newIdea)
     this.setState({ body: "" })
   }

web/static/js/prop_types.js

@@ -21,7 +21,7 @@ export const retroChannel = PropTypes.shape({
 
 export const idea = PropTypes.shape({
   id: PropTypes.number,
-  author: PropTypes.string,
+  user: PropTypes.object,
   body: PropTypes.string,
   category,
 })