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

fixed for non-regular primary key #40

Merged
merged 2 commits into from
Feb 17, 2018
Merged

fixed for non-regular primary key #40

merged 2 commits into from
Feb 17, 2018

Conversation

devvit
Copy link

@devvit devvit commented Feb 16, 2018

solve #33

@izelnakri
Copy link
Owner

This makes sense, thank you! One question before I merge this, why do you think my uuid tests didnt catch this? I guess our test coverage doesn't cover this bug?

Could you look into this as well? I think we should release this fix along with its relevant test case.

@@ -322,7 +322,7 @@ defmodule PaperTrail do
%Version{
event: "update",
item_type: changeset.data.__struct__ |> Module.split() |> List.last(),
item_id: changeset.data.id,
item_id: get_model_id_from_changeset(changeset),
Copy link
Owner

Choose a reason for hiding this comment

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

This could simply be get_model_id(changeset.data) instead of a new function I think

@devvit
Copy link
Author

devvit commented Feb 16, 2018

Yeah, the function is unnecessary, removed.

I'm working for a legacy database that there are some non-regular named fields, hoping to be useful to others :)

@izelnakri izelnakri merged commit 794bace into izelnakri:master Feb 17, 2018
@izelnakri
Copy link
Owner

This is very nice thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants