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

Adding documentation on conditional attributes #1734

Merged
merged 1 commit into from
May 20, 2016

Conversation

lambda2
Copy link
Contributor

@lambda2 lambda2 commented May 18, 2016

Purpose

Adding documentation on conditional attributes for the attribute method.

Changes

Adding documentation and short example (from this pull request) on conditional attributes for the attribute method.

Related GitHub issues

#1403

@@ -34,6 +34,18 @@ Serialization of the resource `title`
| `attribute :title { 'A Different Title'}` | `{ title: 'A Different Title' } `
| `attribute :title`<br>`def title 'A Different Title' end` | `{ title: 'A Different Title' }`

An `if` or `unless` parameter can make an attribute conditional. It take a symbol of a method name on the serializer.
Copy link
Contributor

Choose a reason for hiding this comment

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

s/take/takes/

@beauby
Copy link
Contributor

beauby commented May 18, 2016

Thanks for this PR! I think we merged the PR allowing lambdas as values for the if/unless options. Would you mind adding an example using those?

object.id == current_user.id
end
```

[PR please for conditional attributes:)](https://github.com/rails-api/active_model_serializers/pull/1403)
Copy link
Contributor

Choose a reason for hiding this comment

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

is this breadcrumb taken care of then?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yup, seems so. Good catch @cgmckeever. @lambda2 could you remove the reminder?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup !

@bf4
Copy link
Member

bf4 commented May 18, 2016

@lambda2 great! would you like to add yourself to the changelog under misc or rebase? If not, we can handle it.

@lambda2
Copy link
Contributor Author

lambda2 commented May 18, 2016

@bf4 Yes, I added it under misc :)

@remear
Copy link
Member

remear commented May 19, 2016

@lambda2 Thanks very much for this!

Could you do the following:

  • Rebase to get the latest from master
  • Move your CHANGELOG entry under master (unreleased)
  • Squash your commits

Once those items are taken care of we can get this merged in.

@lambda2
Copy link
Contributor Author

lambda2 commented May 19, 2016

@remear yeah, of course. Seems good now !

@@ -34,7 +34,18 @@ Serialization of the resource `title`
| `attribute :title { 'A Different Title'}` | `{ title: 'A Different Title' } `
| `attribute :title`<br>`def title 'A Different Title' end` | `{ title: 'A Different Title' }`

[PR please for conditional attributes:)](https://github.com/rails-api/active_model_serializers/pull/1403)
An `if` or `unless` parameter can make an attribute conditional. It takes a symbol of a method name on the serializer, or a lambda literal.
Copy link
Member

Choose a reason for hiding this comment

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

I'd rather use option instead of parameter here.

@remear
Copy link
Member

remear commented May 19, 2016

@lambda2, @groyoh makes a good point. Could you address his comment and squash one more time so we can wrap this one up.

Adding documentation and short example ([from this pull request](rails-api#1403)) on conditional attributes.

Adding lambda literal notation and example.

Adding lambda literal notation and example, and fixing typo.

Removing PR reminder

Adding Changelog entry

Moving CHANGELOG entry under master (unreleased)

Use option instead of parameter
@lambda2
Copy link
Contributor Author

lambda2 commented May 19, 2016

👌

@remear remear merged commit 835aad3 into rails-api:master May 20, 2016
@lambda2 lambda2 deleted the patch-1 branch May 20, 2016 13:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants