-
Notifications
You must be signed in to change notification settings - Fork 86
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 part_type to conversation_part #1325
Conversation
0253fb9
to
970c1c2
Compare
@@ -24,9 +25,14 @@ defmodule CodeCorps.Messages.ConversationParts do | |||
@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]) | |||
|> cast(attrs, [:author_id, :body, :conversation_id, :part_type]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm guessing this will need an update cast as well? Don't see an update action however.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we would cast this in an update
, since the part type should never change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some quick changes and then should be good to go!
@@ -24,9 +25,14 @@ defmodule CodeCorps.Messages.ConversationParts do | |||
@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]) | |||
|> cast(attrs, [:author_id, :body, :conversation_id, :part_type]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we would cast this in an update
, since the part type should never change.
@@ -19,6 +19,7 @@ defmodule CodeCorps.ConversationPart do | |||
schema "conversation_parts" do | |||
field :body, :string, null: false | |||
field :read_at, :utc_datetime, null: true | |||
field :part_type, :string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would make this default: "comment"
so it doesn't have to be specified for most cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does it get set to any other type? If I set it to a default, I can figure out a way to test the part_type since it doesn't get included in the changeset...
|
||
def change do | ||
alter table(:conversation_parts) do | ||
add :part_type, :string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With above comment, should add default: "comment"
.
@@ -8,14 +8,42 @@ defmodule CodeCorps.Messages.ConversationPartsTest do | |||
} | |||
|
|||
@valid_attrs %{ | |||
body: "Test body." | |||
body: "Test body.", | |||
part_type: "comment" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can remove this line and the tests should still pass, with the changes mentioned above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I set to a default, do I need to do something in the changeset chain to include it in the changes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No it’ll just get set if it’s not set. Remember Ecto is still just an adapter to SQL.
7ba4e64
to
9f7c7aa
Compare
attrs = @valid_attrs |> Map.merge(%{author_id: 1, conversation_id: 1}) | ||
{:ok, conversation} = ConversationParts.create_changeset(%ConversationPart{}, attrs) |> Repo.insert | ||
assert conversation.body == "Test body." | ||
assert conversation.part_type == "comment" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you also probably want to assert the conversation.user_id here is equal to the user.id.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
Fixes #1318