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

Add helper methods for configuring Views #202

Merged
merged 6 commits into from Dec 13, 2014
Merged

Conversation

shanitang
Copy link
Contributor

@steved555 @zendesk/archer
Write helper methods for View configuration, which includes assigning and updating "columns" and "conditions" arguments. Minitest for those methods are also written in view_spec.

@ggrossman ggrossman changed the title WIP [WIP] Add helper methods for configuring Views Dec 3, 2014
@@ -15,7 +15,7 @@ if defined?(RSpec)

desc "Run live specs"
RSpec::Core::RakeTask.new("spec:live") do |t|
t.pattern = "spec/live/**/*_spec.rb"
t.pattern = "spec/live/**/view_spec.rb"
Copy link
Contributor

Choose a reason for hiding this comment

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

Should change this back to *_spec.rb

Copy link
Contributor

Choose a reason for hiding this comment

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

You can always run a specific test like rspec spec/live/view_spec.rb

write tests for #add_all_condition and #add_any_condition
write tests for #all_conditions= and #any_conditions=
write a test for #add_column
update .gitignore
@shanitang shanitang changed the title [WIP] Add helper methods for configuring Views Add helper methods for configuring Views Dec 5, 2014
@shanitang
Copy link
Contributor Author

@steved555 @zendesk/archer
Could you take a look at this, and see if everything is well revised?

@@ -473,20 +506,39 @@ class View < Rule
has :execution, :class => RuleExecution
has ViewCount, :path => "count"

def add_column(column)
columns = execution.columns.map {|c| c[:id]}
Copy link
Member

Choose a reason for hiding this comment

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

columns = execution.columns.map(&:id) is slightly shorter

Copy link
Contributor

Choose a reason for hiding this comment

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

👍 nice

Copy link
Contributor

Choose a reason for hiding this comment

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

Hashie...

@shanitang
Copy link
Contributor Author

@steved555
If it's ready to merge, can you give me a plus one?

@@ -20,4 +20,78 @@ def valid_attributes
"all" => [{ "field" => "status", "operator" => "is", "value" => "pending" }]
}
it_should_be_deletable

subject do
described_class.new(double, {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you use valid_attributes for this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, since live specs aren't run under travis it'd be really cool if we added a new spec/resources or add these under spec/core

@shanitang shanitang force-pushed the ytang/rule-helper-methods branch 3 times, most recently from a04fa37 to 5ce06e0 Compare December 12, 2014 23:39
@shanitang
Copy link
Contributor Author

@steved555
Could you take a look again to see if it's good to merge?

@vkmita
Copy link
Contributor

vkmita commented Dec 13, 2014

👍 from me

@steved
Copy link
Contributor

steved commented Dec 13, 2014

👍 Thanks for sticking with it

shanitang added a commit that referenced this pull request Dec 13, 2014
Add helper methods for configuring Views
@shanitang shanitang merged commit f5406fc into master Dec 13, 2014
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

4 participants