Skip to content

Hide email from response for all but authenticated user - #213

Merged
begedin merged 1 commit into
developfrom
194-hide-email-for-all-but-authed-user
Sep 15, 2016
Merged

Hide email from response for all but authenticated user#213
begedin merged 1 commit into
developfrom
194-hide-email-for-all-but-authed-user

Conversation

@begedin

@begedin begedin commented Sep 15, 2016

Copy link
Copy Markdown
Contributor

Closes #194

I implemented this at the view level.

We can override record attributes by defining functions of the same name as the attribute. This function then receives the record as the first parameter and the conn as the second, so it's simple to compare the user being serialised with the current_user assign in the conn.

Took me a while to figure out this is possible, though.

@joshsmith joshsmith left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Overall looks good. Would just add some brief documentation stating what's going on since it's non-obvious at first glance.

Comment thread web/views/user_view.ex
CodeCorps.UserPhoto.url({user.photo, user}, :thumb)
end

def email(user, %Plug.Conn{assigns: %{current_user: current_user}}) do

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would add some def documentation of this.

@begedin

begedin commented Sep 15, 2016

Copy link
Copy Markdown
Contributor Author

@joshsmith Need another review for this.

@begedin
begedin force-pushed the 194-hide-email-for-all-but-authed-user branch from 1a69fa5 to d81922c Compare September 15, 2016 19:41
@begedin
begedin merged commit 9486363 into develop Sep 15, 2016
@begedin
begedin deleted the 194-hide-email-for-all-but-authed-user branch September 15, 2016 19:45
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.

2 participants