Skip to content

Commit

Permalink
Merge pull request #199 from appfolio/cpFixCollectionItemXpathBug
Browse files Browse the repository at this point in the history 
Fix bug in collection item xpath generation
  • Loading branch information
@choruk
choruk authored Jul 31, 2017
2 parents 2c1d131 + cb6980d commit 2d427c8
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 28 deletions.
2 changes: 1 addition & 1 deletion lib/ae_page_objects/elements/collection.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ def item_xpath
end

def item_locator_at(index)
[:xpath, "#{item_xpath}[#{index + 1}]", options]
[:xpath, "(#{item_xpath})[#{index + 1}]", options]
end

def default_item_locator
Expand Down
2 changes: 1 addition & 1 deletion test/test_apps/shared/test/page_objects/authors/new_page.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ class NewPage < AePageObjects::Document
collection :books,
:name => "books_attributes",
:locator => "#author_books",
:item_locator => ".some-books-fool .row" do
:item_locator => ".a-book-fool" do
element :title
end
end
Expand Down
12 changes: 6 additions & 6 deletions test/test_apps/shared/views/authors/_form.html.erb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
<a class="show_star">Show Star</a>
<a class="add_star">Add Star</a>
<a class="remove_star">Remove Star</a>

<span class="star">*</span>
</div>

Expand All @@ -41,17 +41,17 @@
<%= f.text_field :last_name %>

<div id="author_books">
<div class="some-books-fool">
<%= f.fields_for :books do |book_form| %>
<div class="row">
<%= f.fields_for :books do |book_form| %>
<div class="a-book-fool-parent">
<div class="a-book-fool">
<%= book_form.label :title %>
<%= book_form.text_field :title %>
<%= book_form.fields_for :index do |index_form| %>
<%= index_form.text_field :pages %>
<% end %>
</div>
<% end %>
</div>
</div>
<% end %>
</div>

<%= f.submit %>
Expand Down
16 changes: 8 additions & 8 deletions test/unit/collection_test.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ def test_locator_options__blank

bullet1_stub = mock(:allow_reload!)
magazine_node.expects(:all).with(:xpath, 'item_xpath', {}).returns([bullet1_stub])
magazine_node.expects(:first).with(:xpath, 'item_xpath[1]', {}).returns(bullet1_stub)
magazine_node.expects(:first).with(:xpath, '(item_xpath)[1]', {}).returns(bullet1_stub)
assert_equal bullet1_stub, magazine.at(0).node
end

Expand All @@ -116,7 +116,7 @@ def test_locator_options__present

bullet1_stub = mock(:allow_reload!)
magazine_node.expects(:all).with(:xpath, 'item_xpath', { :capybara => 'options' }).returns([bullet1_stub])
magazine_node.expects(:first).with(:xpath, "item_xpath[1]", { :capybara => 'options' }).returns(bullet1_stub)
magazine_node.expects(:first).with(:xpath, "(item_xpath)[1]", { :capybara => 'options' }).returns(bullet1_stub)
assert_equal bullet1_stub, magazine.at(0).node
end

Expand Down Expand Up @@ -173,25 +173,25 @@ def test_non_empty

assert_equal 2, magazine.size

magazine_node.expects(:first).with(:xpath, "item_xpath[1]", {}).returns(bullet1_stub)
magazine_node.expects(:first).with(:xpath, "item_xpath[2]", {}).returns(bullet2_stub)
magazine_node.expects(:first).with(:xpath, "(item_xpath)[1]", {}).returns(bullet1_stub)
magazine_node.expects(:first).with(:xpath, "(item_xpath)[2]", {}).returns(bullet2_stub)
each_block_call_count = 0
magazine.each do |bullet|
bullet.name
each_block_call_count += 1
end
assert_equal 2, each_block_call_count

magazine_node.expects(:first).with(:xpath, "item_xpath[1]", {}).times(2).returns(bullet1_stub)
magazine_node.expects(:first).with(:xpath, "(item_xpath)[1]", {}).times(2).returns(bullet1_stub)
assert_equal bullet1_stub, magazine.at(0).node
assert_equal bullet1_stub, magazine.first.node

magazine_node.expects(:first).with(:xpath, "item_xpath[2]", {}).times(2).returns(bullet2_stub)
magazine_node.expects(:first).with(:xpath, "(item_xpath)[2]", {}).times(2).returns(bullet2_stub)
assert_equal bullet2_stub, magazine.at(1).node
assert_equal bullet2_stub, magazine.last.node

magazine_node.expects(:first).with(:xpath, "item_xpath[1]", {}).returns(bullet1_stub)
magazine_node.expects(:first).with(:xpath, "item_xpath[2]", {}).returns(bullet2_stub)
magazine_node.expects(:first).with(:xpath, "(item_xpath)[1]", {}).returns(bullet1_stub)
magazine_node.expects(:first).with(:xpath, "(item_xpath)[2]", {}).returns(bullet2_stub)
assert_equal [bullet1_stub, bullet2_stub], magazine.map(&:node)

assert_equal nil, magazine.at(1000)
Expand Down
24 changes: 12 additions & 12 deletions test/unit/dsl/collection_test.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ def test_collection__no_is__no_contains__block

first_owner_page_object = stub(:allow_reload!)
previous_owners_page_object.expects(:all).with(:xpath, ".//*", {}).returns([first_owner_page_object])
previous_owners_page_object.expects(:first).with(:xpath, ".//*[1]", {}).returns(first_owner_page_object)
previous_owners_page_object.expects(:first).with(:xpath, "(.//*)[1]", {}).returns(first_owner_page_object)
first_owner = verify_item_element_with_intermediary_class(previous_owners, 0, AePageObjects::Element, first_owner_page_object)

owner_name_page_object = stub(:allow_reload!)
Expand Down Expand Up @@ -62,7 +62,7 @@ def test_collection__is__no_contains__block
first_owner_page_object = stub(:allow_reload!)

previous_owners_page_object.expects(:all).with(:xpath, ".//*", {}).returns([first_owner_page_object])
previous_owners_page_object.expects(:first).with(:xpath, ".//*[1]", {}).returns(first_owner_page_object)
previous_owners_page_object.expects(:first).with(:xpath, "(.//*)[1]", {}).returns(first_owner_page_object)
first_owner = verify_item_element_with_intermediary_class(previous_owners, 0, previous_owners_class.item_class, first_owner_page_object)

owner_name_page_object = stub(:allow_reload!)
Expand Down Expand Up @@ -98,7 +98,7 @@ def test_collection__is__no_contains__block__no_item_class
first_owner_page_object = stub(:allow_reload!)

previous_owners_page_object.expects(:all).with(:xpath, ".//*", {}).returns([first_owner_page_object])
previous_owners_page_object.expects(:first).with(:xpath, ".//*[1]", {}).returns(first_owner_page_object)
previous_owners_page_object.expects(:first).with(:xpath, "(.//*)[1]", {}).returns(first_owner_page_object)
first_owner = verify_item_element_with_intermediary_class(previous_owners, 0, previous_owners_class.item_class, first_owner_page_object)

owner_name_page_object = stub(:allow_reload!)
Expand Down Expand Up @@ -136,7 +136,7 @@ def test_collection__is__no_contains__no_block

first_owner_page_object = stub(:allow_reload!)
previous_owners_page_object.expects(:all).with(:xpath, ".//*", {}).returns([first_owner_page_object])
previous_owners_page_object.expects(:first).with(:xpath, ".//*[1]", {}).returns(first_owner_page_object)
previous_owners_page_object.expects(:first).with(:xpath, "(.//*)[1]", {}).returns(first_owner_page_object)
first_owner = verify_item_element(previous_owners, 0, previous_owner_class, first_owner_page_object)

owner_name_page_object = stub(:allow_reload!)
Expand Down Expand Up @@ -174,7 +174,7 @@ def test_collection__is__contains__no_block__same_item_class

first_owner_page_object = stub(:allow_reload!)
previous_owners_page_object.expects(:all).with(:xpath, ".//*", {}).returns([first_owner_page_object])
previous_owners_page_object.expects(:first).with(:xpath, ".//*[1]", {}).returns(first_owner_page_object)
previous_owners_page_object.expects(:first).with(:xpath, "(.//*)[1]", {}).returns(first_owner_page_object)
first_owner = verify_item_element(previous_owners, 0, previous_owner_class, first_owner_page_object)

owner_name_page_object = stub(:allow_reload!)
Expand Down Expand Up @@ -210,7 +210,7 @@ def test_collection__is__contains__no_block__different_item_class

first_owner_page_object = stub(:allow_reload!)
previous_owners_page_object.expects(:all).with(:xpath, ".//*", {}).returns([first_owner_page_object])
previous_owners_page_object.expects(:first).with(:xpath, ".//*[1]", {}).returns(first_owner_page_object)
previous_owners_page_object.expects(:first).with(:xpath, "(.//*)[1]", {}).returns(first_owner_page_object)
first_owner = verify_item_element(previous_owners, 0, previous_owner_class, first_owner_page_object)

owner_name_page_object = stub(:allow_reload!)
Expand Down Expand Up @@ -244,7 +244,7 @@ def test_collection__no_is__contains__no_block

first_owner_page_object = stub(:allow_reload!)
previous_owners_page_object.expects(:all).with(:xpath, ".//*", {}).returns([first_owner_page_object])
previous_owners_page_object.expects(:first).with(:xpath, ".//*[1]", {}).returns(first_owner_page_object)
previous_owners_page_object.expects(:first).with(:xpath, "(.//*)[1]", {}).returns(first_owner_page_object)
first_owner = verify_item_element(previous_owners, 0, previous_owner_class, first_owner_page_object)

owner_name_page_object = stub(:allow_reload!)
Expand Down Expand Up @@ -280,7 +280,7 @@ def test_collection__no_is__contains__block

first_owner_page_object = stub(:allow_reload!)
previous_owners_page_object.expects(:all).with(:xpath, ".//*", {}).returns([first_owner_page_object])
previous_owners_page_object.expects(:first).with(:xpath, ".//*[1]", {}).returns(first_owner_page_object)
previous_owners_page_object.expects(:first).with(:xpath, "(.//*)[1]", {}).returns(first_owner_page_object)
first_owner = verify_item_element_with_intermediary_class(previous_owners, 0, previous_owner_class, first_owner_page_object)

owner_name_page_object = stub(:allow_reload!)
Expand Down Expand Up @@ -323,7 +323,7 @@ def test_collection__is__contains__block

first_owner_page_object = stub(:allow_reload!)
previous_owners_page_object.expects(:all).with(:xpath, ".//*", {}).returns([first_owner_page_object])
previous_owners_page_object.expects(:first).with(:xpath, ".//*[1]", {}).returns(first_owner_page_object)
previous_owners_page_object.expects(:first).with(:xpath, "(.//*)[1]", {}).returns(first_owner_page_object)
first_owner = verify_item_element_with_intermediary_class(previous_owners, 0, previous_owner_class, first_owner_page_object)

owner_name_page_object = stub(:allow_reload!)
Expand Down Expand Up @@ -351,7 +351,7 @@ def test_collection__no_is__no_contains__no_block

first_owner_page_object = stub(:allow_reload!)
previous_owners_page_object.expects(:all).with(:xpath, ".//*", {}).returns([first_owner_page_object])
previous_owners_page_object.expects(:first).with(:xpath, ".//*[1]", {}).returns(first_owner_page_object)
previous_owners_page_object.expects(:first).with(:xpath, "(.//*)[1]", {}).returns(first_owner_page_object)
first_owner = verify_item_element(previous_owners, 0, AePageObjects::Element, first_owner_page_object)
end

Expand All @@ -376,7 +376,7 @@ def test_collection__locator

first_owner_page_object = stub(:allow_reload!)
previous_owners_page_object.expects(:all).with(:xpath, ".//*", {}).returns([first_owner_page_object])
previous_owners_page_object.expects(:first).with(:xpath, ".//*[1]", {}).returns(first_owner_page_object)
previous_owners_page_object.expects(:first).with(:xpath, "(.//*)[1]", {}).returns(first_owner_page_object)
first_owner = verify_item_element_with_intermediary_class(previous_owners, 0, AePageObjects::Element, first_owner_page_object)

owner_name_page_object = stub(:allow_reload!)
Expand Down Expand Up @@ -411,7 +411,7 @@ def test_nested_element__locator__proc

first_owner_page_object = stub(:allow_reload!)
previous_owners_page_object.expects(:all).with(:xpath, ".//*", {}).returns([first_owner_page_object])
previous_owners_page_object.expects(:first).with(:xpath, ".//*[1]", {}).returns(first_owner_page_object)
previous_owners_page_object.expects(:first).with(:xpath, "(.//*)[1]", {}).returns(first_owner_page_object)
first_owner = verify_item_element_with_intermediary_class(previous_owners, 0, AePageObjects::Element, first_owner_page_object)

owner_name_page_object = stub(:allow_reload!)
Expand Down

0 comments on commit 2d427c8

Please sign in to comment.