Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for creation of associated conversations with message via params #1301

Merged
merged 1 commit into from
Dec 18, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion config/dev.exs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ config :code_corps, CodeCorps.Repo,
pool_size: 10

# CORS allowed origins
config :code_corps, allowed_origins: ["http://localhost:4200"]
config :code_corps, allowed_origins: ["http://localhost:4200", "chrome-extension://pfdhoblngboilpfeibdedpjgfnlcodoo"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs removed.


config :code_corps, CodeCorps.Guardian,
secret_key: "e62fb6e2746f6b1bf8b5b735ba816c2eae1d5d76e64f18f3fc647e308b0c159e"
Expand Down
8 changes: 6 additions & 2 deletions lib/code_corps/messages/conversation_parts.ex
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,21 @@ defmodule CodeCorps.Messages.ConversationParts do
ConversationPart,
Repo
}
alias CodeCorpsWeb.ConversationChannel

@spec create(map) :: ConversationPart.t | Ecto.Changeset.t
def create(attrs) do
%ConversationPart{} |> create_changeset(attrs) |> Repo.insert()
with {:ok, %ConversationPart{} = conversation_part} <- %ConversationPart{} |> create_changeset(attrs) |> Repo.insert() do
ConversationChannel.broadcast_new_conversation_part(conversation_part)
{:ok, conversation_part}
end
end

@doc false
@spec create_changeset(ConversationPart.t, map) :: Ecto.Changeset.t
def create_changeset(%ConversationPart{} = conversation_part, attrs) do
conversation_part
|> cast(attrs, [:author_id, :body, :conversation_id, :read_at])
|> cast(attrs, [:author_id, :body, :conversation_id])
|> validate_required([:author_id, :body, :conversation_id])
|> assoc_constraint(:author)
|> assoc_constraint(:conversation)
Expand Down
19 changes: 19 additions & 0 deletions lib/code_corps/messages/conversations.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
defmodule CodeCorps.Messages.Conversations do
@moduledoc ~S"""
Subcontext aimed at managing `CodeCorps.Conversation` records aimed at a
specific user belonging to a `CodeCorps.Message`.
"""

alias Ecto.Changeset

alias CodeCorps.{Conversation}

@doc false
@spec create_changeset(Conversation.t, map) :: Ecto.Changeset.t
def create_changeset(%Conversation{} = conversation, %{} = attrs) do
conversation
|> Changeset.cast(attrs, [:user_id])
|> Changeset.validate_required([:user_id])
|> Changeset.assoc_constraint(:user)
end
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This changeset is needed to cast_assoc. The message_id is provided automatically by cast_assoc and is not part of the params.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will need a test.

end
5 changes: 5 additions & 0 deletions lib/code_corps/messages/messages.ex
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,11 @@ defmodule CodeCorps.Messages do
def create(%{} = params) do
%Message{}
|> Message.changeset(params)
|> Changeset.cast(params, [:author_id, :project_id])
|> Changeset.validate_required([:author_id, :project_id])
|> Changeset.assoc_constraint(:author)
|> Changeset.assoc_constraint(:project)
|> Changeset.cast_assoc(:conversations, with: &Messages.Conversations.create_changeset/2)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

create requires more work than the default changeset does. We should probably add more tests for the create function to make sure these changes work. I don't think there's an explicit need to extract a create changeset, though.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's an argument to be made here that this belongs in its own changeset, akin to Messages.Conversations.create_changeset.

Copy link
Contributor

@snewcomer snewcomer Dec 15, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@begedin this is the nested POST, right? I remember doing this once and I had to create a changeset.

I think what I did was take the nested ids, pass it through |> Enum.map(&Changeset.change/1) and then put_assoc(changeset, :conversations, conversation_changesets) or something like that. But this looks much better if I am understanding it right.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But that required the nested ids to already exist in the DB. Not sure what the case is here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@snewcomer The ids are not in the database in this case. The child record is created alongside the parent.

|> Repo.insert()
end

Expand Down
34 changes: 34 additions & 0 deletions lib/code_corps_web/channels/conversation_channel.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
defmodule CodeCorpsWeb.ConversationChannel do
use Phoenix.Channel

alias CodeCorps.{Conversation, Policy, Repo, User}
alias Phoenix.Socket

@spec join(String.t, map, Socket.t) :: {:ok, Socket.t} | {:error, map}
def join("conversation:" <> id, %{}, %Socket{} = socket) do
with %Conversation{} = conversation <- Conversation |> Repo.get(id),
%User{} = current_user <- socket.assigns[:current_user],
{:ok, :authorized} <- current_user |> Policy.authorize(:show, conversation, %{}) do

{:ok, socket}
else
nil -> {:error, %{reason: "unauthenticated"}}
{:error, :not_authorized} -> {:error, %{reason: "unauthorized"}}
end
end

def event("new:conversation-part", socket, message) do
broadcast socket, "new:conversation-part", message
{:ok, socket}
end

def broadcast_new_conversation_part(conversation_part) do
channel = "conversation:#{conversation_part.conversation_id}"
event = "new:conversation-part"
payload = %{
id: conversation_part.id
}

CodeCorpsWeb.Endpoint.broadcast(channel, event, payload)
end
end
11 changes: 8 additions & 3 deletions lib/code_corps_web/channels/user_socket.ex
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ defmodule CodeCorpsWeb.UserSocket do
use Phoenix.Socket

## Channels
# channel "room:*", CodeCorps.RoomChannel
channel "conversation:*", CodeCorpsWeb.ConversationChannel

## Transports
transport :websocket, Phoenix.Transports.WebSocket,
Expand All @@ -20,8 +20,13 @@ defmodule CodeCorpsWeb.UserSocket do
#
# See `Phoenix.Token` documentation for examples in
# performing token verification on connect.
def connect(_params, socket) do
{:ok, socket}
def connect(%{"token" => token}, socket) do
with {:ok, claims} <- CodeCorps.Guardian.decode_and_verify(token),
{:ok, user} <- CodeCorps.Guardian.resource_from_claims(claims) do
{:ok, assign(socket, :current_user, user)}
else
_ -> {:ok, socket}
end
end

# Socket id's are topics that allow you to identify all sockets for a given user:
Expand Down
10 changes: 8 additions & 2 deletions lib/code_corps_web/controllers/conversation_controller.ex
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,23 @@ defmodule CodeCorpsWeb.ConversationController do
@spec index(Conn.t, map) :: Conn.t
def index(%Conn{} = conn, %{} = params) do
with %User{} = current_user <- conn |> CodeCorps.Guardian.Plug.current_resource,
conversations <- Conversation |> Policy.scope(current_user) |> Messages.list_conversations(params) do
conversations <- Conversation |> Policy.scope(current_user) |> Messages.list_conversations(params) |> preload() do
conn |> render("index.json-api", data: conversations)
end
end

@spec show(Conn.t, map) :: Conn.t
def show(%Conn{} = conn, %{"id" => id}) do
with %User{} = current_user <- conn |> CodeCorps.Guardian.Plug.current_resource,
%Conversation{} = conversation <- Messages.get_conversation(id),
%Conversation{} = conversation <- Messages.get_conversation(id) |> preload(),
{:ok, :authorized} <- current_user |> Policy.authorize(:show, conversation, %{}) do
conn |> render("show.json-api", data: conversation)
end
end

@preloads [:conversation_parts, :message, :user]

def preload(data) do
Repo.preload(data, @preloads)
end
end
8 changes: 4 additions & 4 deletions lib/code_corps_web/controllers/message_controller.ex
Original file line number Diff line number Diff line change
Expand Up @@ -9,21 +9,21 @@ defmodule CodeCorpsWeb.MessageController do
}

action_fallback CodeCorpsWeb.FallbackController
plug CodeCorpsWeb.Plug.DataToAttributes
plug CodeCorpsWeb.Plug.DataToAttributes, [includes_many: ~w(conversation)]
plug CodeCorpsWeb.Plug.IdsToIntegers

@spec index(Conn.t, map) :: Conn.t
def index(%Conn{} = conn, %{} = params) do
with %User{} = current_user <- conn |> CodeCorps.Guardian.Plug.current_resource,
messages <- Message |> Policy.scope(current_user) |> Messages.list(params) do
messages <- Message |> Policy.scope(current_user) |> Messages.list(params) |> preload() do
conn |> render("index.json-api", data: messages)
end
end

@spec show(Conn.t, map) :: Conn.t
def show(%Conn{} = conn, %{"id" => id}) do
with %User{} = current_user <- conn |> CodeCorps.Guardian.Plug.current_resource,
%Message{} = message <- Message |> Repo.get(id),
%Message{} = message <- Message |> Repo.get(id) |> preload(),
{:ok, :authorized} <- current_user |> Policy.authorize(:show, message, %{}) do
conn |> render("show.json-api", data: message)
end
Expand All @@ -40,7 +40,7 @@ defmodule CodeCorpsWeb.MessageController do
end
end

@preloads [:author, :project]
@preloads [:author, :project, :conversations]

def preload(data) do
Repo.preload(data, @preloads)
Expand Down
37 changes: 32 additions & 5 deletions lib/code_corps_web/plugs/data_to_attributes.ex
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
defmodule CodeCorpsWeb.Plug.DataToAttributes do
@moduledoc ~S"""
Converts params in the JSON api "data" format into flat params convient for
Converts params in the JSON api format into flat params convient for
changeset casting.

This is done using `JaSerializer.Params.to_attributes/1`
For base parameters, this is done using `JaSerializer.Params.to_attributes/1`

For included records, this is done using custom code.
"""

alias Plug.Conn
Expand All @@ -12,13 +14,38 @@ defmodule CodeCorpsWeb.Plug.DataToAttributes do
def init(opts), do: opts

@spec call(Conn.t, Keyword.t) :: Plug.Conn.t
def call(%Conn{params: %{"data" => data} = params} = conn, _opts) do
def call(%Conn{params: %{} = params} = conn, opts \\ []) do
attributes =
params
|> Map.delete("data")
|> Map.merge(data |> JaSerializer.Params.to_attributes)
|> Map.delete("included")
|> Map.merge(params |> parse_data())
|> Map.merge(params |> parse_included(opts))

conn |> Map.put(:params, attributes)
end
def call(%Conn{} = conn, _opts), do: conn

@spec parse_data(map) :: map
defp parse_data(%{"data" => data}), do: data |> JaSerializer.Params.to_attributes
defp parse_data(%{}), do: %{}

@spec parse_included(map, Keyword.t) :: map
defp parse_included(%{"included" => included}, opts) do
included |> Enum.reduce(%{}, fn (%{"data" => %{"type" => type}} = params, parsed) ->
attributes = params |> parse_data()

if opts |> Keyword.get(:includes_many, []) |> Enum.member?(type) do
# this is an explicitly specified has_many,
# update existing data by adding new record
pluralized_type = type |> Inflex.pluralize
parsed |> Map.update(pluralized_type, [attributes], fn data ->
data ++ [attributes]
end)
else
# this is a belongs to, put a new submap into payload
parsed |> Map.put(type, attributes)
end
end)
end
defp parse_included(%{}, _opts), do: %{}
end
2 changes: 2 additions & 0 deletions lib/code_corps_web/views/conversation_view.ex
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,6 @@ defmodule CodeCorpsWeb.ConversationView do

has_one :user, type: "user", field: :user_id
has_one :message, type: "message", field: :message_id

has_many :conversation_parts, serializer: CodeCorpsWeb.ConversationPartView, identifiers: :always
end
2 changes: 2 additions & 0 deletions lib/code_corps_web/views/message_view.ex
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,6 @@ defmodule CodeCorpsWeb.MessageView do

has_one :author, type: "user", field: :author_id
has_one :project, type: "project", field: :project_id

has_many :conversations, serializer: CodeCorpsWeb.ConversationView, identifiers: :always
end
4 changes: 2 additions & 2 deletions priv/repo/structure.sql
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
-- PostgreSQL database dump
--

-- Dumped from database version 9.5.10
-- Dumped by pg_dump version 10.1
-- Dumped from database version 10.0
-- Dumped by pg_dump version 10.0

SET statement_timeout = 0;
SET lock_timeout = 0;
Expand Down
Loading