Skip to content

Allow attribute to be used, but not to show up in the schema.#157

Merged
richmolj merged 1 commit into
graphiti-api:masterfrom
zeisler:attribute_disable_schema
Jun 13, 2019
Merged

Allow attribute to be used, but not to show up in the schema.#157
richmolj merged 1 commit into
graphiti-api:masterfrom
zeisler:attribute_disable_schema

Conversation

@zeisler

@zeisler zeisler commented Jun 11, 2019

Copy link
Copy Markdown
Contributor

Using attribute option schema: false.
This option is default true and is not effected by only and
except options.

@zeisler

zeisler commented Jun 11, 2019

Copy link
Copy Markdown
Contributor Author

Let me know if you want a PR for docs?

@richmolj richmolj 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.

Great work @zeisler and thanks for this PR ❤️

This seems mergeable as-is, but want to note the scenario where we hide based on functionality rather than attribute. Something like:

attribute :foo, :string, schema: false
sort :foo, schema: true

Not strictly necessary, but something to think about. Up to you.

Something for the docs would be great - ideally there is a separate "Schema" section of the docs where we could put this, but I haven't had time to write that yet. Where would this have been most helpful for you?

Comment thread lib/graphiti/schema.rb
def sorts(resource)
{}.tap do |s|
resource.sorts.each_pair do |name, sort|
next unless resource.attributes[name][:schema]

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.

This seems fine to me, unless you can envision a scenario where we allow something like the sort in the schema but not filtering/reading/etc.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

My only use case is that the whole attribute with it's sorting and filtering be hidden. My current project has a table with dynamic columns that changes with customer and year to year. So they are not relevant or available in different contexts. The place where this is hurting is in tests where I create test data that gets added to the schema, which never makes sense in production.

Comment thread spec/schema_spec.rb

attribute :first_name, :string, description: "The employee's first name"

attribute :hidden_attribute, :string, schema: false

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 know we have the diff spec, but should probably be one here too that flips the boolean and asserts it's now present.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added spec

Comment thread spec/schema_diff_spec.rb
it { is_expected.to eq([]) }
end

context "when attribute goes schema true to false" 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.

👍

@zeisler zeisler force-pushed the attribute_disable_schema branch from c909d2b to d78dec7 Compare June 12, 2019 18:26
zeisler added a commit to zeisler/graphiti-api.github.io that referenced this pull request Jun 12, 2019
zeisler added a commit to zeisler/graphiti-api.github.io that referenced this pull request Jun 12, 2019
zeisler added a commit to zeisler/graphiti-api.github.io that referenced this pull request Jun 12, 2019
@zeisler

zeisler commented Jun 12, 2019

Copy link
Copy Markdown
Contributor Author

A schema section does seem helpful in general, but I would go looking for this flag in the resource concepts section. Here what I came up with graphiti-api/graphiti-api.github.io#12

@richmolj

Copy link
Copy Markdown
Contributor

I think this looks great! @zeisler if you could please add to CHANGELOG I'll go ahead and merge. Thanks again!

Using attribute option `schema: false`.
This option is default true and is not effected by `only` and
`except` options.
@zeisler zeisler force-pushed the attribute_disable_schema branch from d78dec7 to c42e4ba Compare June 12, 2019 20:24
@zeisler

zeisler commented Jun 13, 2019

Copy link
Copy Markdown
Contributor Author

@richmolj changelog added.

@richmolj

Copy link
Copy Markdown
Contributor

Awesome, well done @zeisler

@richmolj richmolj merged commit 9550fec into graphiti-api:master Jun 13, 2019
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