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.

Sign up for GitHub

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

Group untagged way nodes with same related ways #5393

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Group untagged way nodes with same related ways #5393

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions app/helpers/browse_helper.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,22 @@
module BrowseHelper
def group_way_nodes(way)
groups = []

way.way_nodes.each do |way_node|
related_ways = related_ways_of_way_node(way_node)
if !groups.empty? && way_node.node.tags.empty? && groups.last[:nodes].last.tags.empty? && groups.last[:related_ways] == related_ways
groups.last[:nodes] << way_node.node
else
groups << {
:nodes => [way_node.node],
:related_ways => related_ways
}
end
end

groups
end

def element_icon(type, object)
selected_icon_data = { :filename => "#{type}.svg", :priority => 1 }

Expand Down Expand Up @@ -119,4 +137,8 @@ def sidebar_classic_pagination(pages, page_param)
def name_locales(object)
object.tags.keys.map { |k| Regexp.last_match(1) if k =~ /^name:(.*)$/ }.flatten
end

def related_ways_of_way_node(way_node)
way_node.node.ways.uniq.sort.reject { |related_way| related_way.id == way_node.way_id }
end
end
18 changes: 11 additions & 7 deletions app/views/browse/_way.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,19 @@
<details <%= "open" if way.way_nodes.count < 10 %>>
<summary><%= t ".nodes_count", :count => way.way_nodes.count %></summary>
<ul class="list-unstyled browse-element-list">
<% way.way_nodes.each do |wn| %>
<%= element_list_item "node", wn.node do %>
<%= element_single_current_link "node", wn.node %>
<% related_ways = wn.node.ways.reject { |w| w.id == wn.way_id } %>
<% group_way_nodes(way).each do |node_group| %>
<%= element_list_item "node", node_group[:nodes].first do %>
<% node_group[:nodes].each_with_index do |node, i| %>
<%= " · " if i.positive? %>
<%= element_single_current_link "node", node %>
<% end %>
<% icon_connector = " " %>
<% if related_ways.size > 0 then %>
<% if node_group[:related_ways].size.positive? %>
(<%= t ".also_part_of_html",
:count => related_ways.size,
:related_ways => to_sentence(related_ways.map { |w| element_icon("way", w) + icon_connector + element_single_current_link("way", w) }) %>)
:count => node_group[:related_ways].size,
:related_ways => to_sentence(node_group[:related_ways].map do |w|
element_icon("way", w) + icon_connector + element_single_current_link("way", w)
end) %>)
<% end %>
<% end %>
<% end %>
Expand Down
118 changes: 118 additions & 0 deletions test/helpers/browse_helper_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,124 @@ class BrowseHelperTest < ActionView::TestCase
include ERB::Util
include ApplicationHelper

def test_group_way_nodes_one_node
way = create(:way_with_nodes, :nodes_count => 1)

assert_equal [
{ :nodes => [way.nodes[0]], :related_ways => [] }
], group_way_nodes(way)
end

def test_group_way_nodes_two_untagged_nodes
way = create(:way_with_nodes, :nodes_count => 2)

assert_equal [
{ :nodes => [way.nodes[0], way.nodes[1]], :related_ways => [] }
], group_way_nodes(way)
end

def test_group_way_nodes_two_tagged_nodes
way = create(:way_with_nodes, :nodes_count => 2)
create(:node_tag, :node => way.nodes[0], :k => "name", :v => "Distinct Node 0")
create(:node_tag, :node => way.nodes[1], :k => "name", :v => "Distinct Node 1")

assert_equal [
{ :nodes => [way.nodes[0]], :related_ways => [] },
{ :nodes => [way.nodes[1]], :related_ways => [] }
], group_way_nodes(way)
end

#
# (b)--1--(a)--2--(c)
#
def test_group_way_nodes_shared_with_one_way
way1 = create(:way)
way2 = create(:way)
node_a = create(:node)
node_b = create(:node)
node_c = create(:node)
create(:way_node, :way => way1, :node => node_a, :sequence_id => 1)
create(:way_node, :way => way1, :node => node_b, :sequence_id => 2)
create(:way_node, :way => way2, :node => node_a, :sequence_id => 1)
create(:way_node, :way => way2, :node => node_c, :sequence_id => 2)

assert_equal [
{ :nodes => [node_a], :related_ways => [way2] },
{ :nodes => [node_b], :related_ways => [] }
], group_way_nodes(way1)
assert_equal [
{ :nodes => [node_a], :related_ways => [way1] },
{ :nodes => [node_c], :related_ways => [] }
], group_way_nodes(way2)
end

#
# (b)--1--(a)--2--(c)
# \ /
# \ /
# (d)
#
def test_group_way_nodes_shared_with_one_looped_way
way1 = create(:way)
way2 = create(:way)
node_a = create(:node)
node_b = create(:node)
node_c = create(:node)
node_d = create(:node)
create(:way_node, :way => way1, :node => node_a, :sequence_id => 1)
create(:way_node, :way => way1, :node => node_b, :sequence_id => 2)
create(:way_node, :way => way2, :node => node_a, :sequence_id => 1)
create(:way_node, :way => way2, :node => node_c, :sequence_id => 2)
create(:way_node, :way => way2, :node => node_d, :sequence_id => 3)
create(:way_node, :way => way2, :node => node_a, :sequence_id => 4)

assert_equal [
{ :nodes => [node_a], :related_ways => [way2] },
{ :nodes => [node_b], :related_ways => [] }
], group_way_nodes(way1)
assert_equal [
{ :nodes => [node_a], :related_ways => [way1] },
{ :nodes => [node_c, node_d], :related_ways => [] },
{ :nodes => [node_a], :related_ways => [way1] }
], group_way_nodes(way2)
end

#
# (b)--1--(a)--2--(c)
# \
# 3
# \
# (d)
#
def test_group_way_nodes_shared_with_two_ways
way1 = create(:way)
way2 = create(:way)
way3 = create(:way)
node_a = create(:node)
node_b = create(:node)
node_c = create(:node)
node_d = create(:node)
create(:way_node, :way => way1, :node => node_a, :sequence_id => 1)
create(:way_node, :way => way1, :node => node_b, :sequence_id => 2)
create(:way_node, :way => way2, :node => node_a, :sequence_id => 1)
create(:way_node, :way => way2, :node => node_c, :sequence_id => 2)
create(:way_node, :way => way3, :node => node_a, :sequence_id => 1)
create(:way_node, :way => way3, :node => node_d, :sequence_id => 2)

assert_equal [
{ :nodes => [node_a], :related_ways => [way2, way3] },
{ :nodes => [node_b], :related_ways => [] }
], group_way_nodes(way1)
assert_equal [
{ :nodes => [node_a], :related_ways => [way1, way3] },
{ :nodes => [node_c], :related_ways => [] }
], group_way_nodes(way2)
assert_equal [
{ :nodes => [node_a], :related_ways => [way1, way2] },
{ :nodes => [node_d], :related_ways => [] }
], group_way_nodes(way3)
end

def test_printable_element_name
node = create(:node, :with_history, :version => 2)
node_v1 = node.old_nodes.find_by(:version => 1)
Expand Down
Loading