From de51ec959644154e7b1564fd96b4ddbb120a1209 Mon Sep 17 00:00:00 2001 From: mmd-osm Date: Fri, 15 Aug 2025 17:29:43 +0200 Subject: [PATCH] Lazy loading relation members --- app/abilities/ability.rb | 1 + .../old_relation_members_controller.rb | 14 +++++ app/controllers/old_relations_controller.rb | 2 +- .../relation_members_controller.rb | 11 ++++ app/controllers/relations_controller.rb | 2 +- app/views/browse/_relation.html.erb | 23 ++++++-- app/views/browse/_relation_member.html.erb | 2 +- .../browse/_relation_member_frame.html.erb | 5 ++ .../_not_found_message.html.erb | 7 +++ .../old_relation_members/timeout.html.erb | 5 ++ .../_not_found_message.html.erb | 3 ++ app/views/relation_members/timeout.html.erb | 5 ++ config/locales/en.yml | 10 ++++ config/routes.rb | 4 ++ .../old_relation_members_controller_test.rb | 54 +++++++++++++++++++ .../relation_members_controller_test.rb | 34 ++++++++++++ 16 files changed, 176 insertions(+), 6 deletions(-) create mode 100644 app/controllers/old_relation_members_controller.rb create mode 100644 app/controllers/relation_members_controller.rb create mode 100644 app/views/browse/_relation_member_frame.html.erb create mode 100644 app/views/old_relation_members/_not_found_message.html.erb create mode 100644 app/views/old_relation_members/timeout.html.erb create mode 100644 app/views/relation_members/_not_found_message.html.erb create mode 100644 app/views/relation_members/timeout.html.erb create mode 100644 test/controllers/old_relation_members_controller_test.rb create mode 100644 test/controllers/relation_members_controller_test.rb diff --git a/app/abilities/ability.rb b/app/abilities/ability.rb index b3b861bb2..5a54c5586 100644 --- a/app/abilities/ability.rb +++ b/app/abilities/ability.rb @@ -6,6 +6,7 @@ class Ability def initialize(user) can :read, [:feature_query, :layers_pane, :legend_pane, :share_pane] can :read, [Node, Way, Relation, OldNode, OldWay, OldRelation] + can :read, [RelationMember, OldRelationMember] can [:show, :create], Note can :read, :directions can [:index, :permalink, :edit, :help, :fixthemap, :offline, :export, :about, :communities, :preview, :copyright, :id], :site diff --git a/app/controllers/old_relation_members_controller.rb b/app/controllers/old_relation_members_controller.rb new file mode 100644 index 000000000..76b3d8f65 --- /dev/null +++ b/app/controllers/old_relation_members_controller.rb @@ -0,0 +1,14 @@ +class OldRelationMembersController < OldElementsController + def show + @type = "relation" + @current_feature = Relation.find(params[:id]) + @feature = OldRelation.preload(:old_members => :member).find([params[:id], params[:version]]) + @frame_id = "member_relation_#{@feature.id}" + + return deny_access(nil) if @feature.redacted? && !params[:show_redactions] + + render :partial => "browse/relation_member_frame", :locals => { :relation => @feature, :frame_id => @frame_id } + rescue ActiveRecord::RecordNotFound + render "browse/not_found", :status => :not_found + end +end diff --git a/app/controllers/old_relations_controller.rb b/app/controllers/old_relations_controller.rb index 174a52fd9..f470a1a87 100644 --- a/app/controllers/old_relations_controller.rb +++ b/app/controllers/old_relations_controller.rb @@ -14,7 +14,7 @@ class OldRelationsController < OldElementsController def show @type = "relation" @current_feature = Relation.find(params[:id]) - @feature = OldRelation.preload(:old_tags, :changeset => [:changeset_tags, :user], :old_members => :member).find([params[:id], params[:version]]) + @feature = OldRelation.preload(:old_tags, :changeset => [:changeset_tags, :user]).find([params[:id], params[:version]]) rescue ActiveRecord::RecordNotFound render "browse/not_found", :status => :not_found end diff --git a/app/controllers/relation_members_controller.rb b/app/controllers/relation_members_controller.rb new file mode 100644 index 000000000..5d4525c34 --- /dev/null +++ b/app/controllers/relation_members_controller.rb @@ -0,0 +1,11 @@ +class RelationMembersController < ElementsController + def show + @type = "relation" + @feature = Relation.preload(:relation_members => :member).find(params[:id]) + @frame_id = "member_relation_#{@feature.id}" + + render :partial => "browse/relation_member_frame", :locals => { :relation => @feature, :frame_id => @frame_id } + rescue ActiveRecord::RecordNotFound + render "browse/not_found", :status => :not_found + end +end diff --git a/app/controllers/relations_controller.rb b/app/controllers/relations_controller.rb index 9199c9e4a..5fc8806e8 100644 --- a/app/controllers/relations_controller.rb +++ b/app/controllers/relations_controller.rb @@ -1,7 +1,7 @@ class RelationsController < ElementsController def show @type = "relation" - @feature = Relation.preload(:relation_tags, :containing_relation_members, :changeset => [:changeset_tags, :user], :relation_members => :member).find(params[:id]) + @feature = Relation.preload(:relation_tags, :containing_relation_members, :changeset => [:changeset_tags, :user]).find(params[:id]) rescue ActiveRecord::RecordNotFound render "browse/not_found", :status => :not_found end diff --git a/app/views/browse/_relation.html.erb b/app/views/browse/_relation.html.erb index d8ca59053..aef3c8048 100644 --- a/app/views/browse/_relation.html.erb +++ b/app/views/browse/_relation.html.erb @@ -23,9 +23,26 @@

<%= t ".members" %>

> <%= t ".members_count", :count => relation.relation_members.count %> - + <% if relation.relation_members.count >= 10 %> + + <% path_params = [relation.relation_id, relation.version] %> + <% path_params << { :show_redactions => params[:show_redactions] } if params[:show_redactions] %> + src="<%= old_relation_members_path(*path_params) %>"> + <% else %> + src="<%= relation_members_path(relation) %>"> + <% end %> +
+ +
+ + <% else %> + + <% end %>
<% end %> <% end %> diff --git a/app/views/browse/_relation_member.html.erb b/app/views/browse/_relation_member.html.erb index c00396fbd..3c1e9cb59 100644 --- a/app/views/browse/_relation_member.html.erb +++ b/app/views/browse/_relation_member.html.erb @@ -1,4 +1,4 @@ -<% linked_name = link_to printable_element_name(relation_member.member), relation_member.member, { :rel => link_follow(relation_member.member) } +<% linked_name = link_to printable_element_name(relation_member.member), relation_member.member, { :rel => link_follow(relation_member.member), :data => { :turbo_prefetch => false } } type_str = t ".type.#{relation_member.member_type.downcase}" %> <%= element_list_item_with_strikethrough relation_member.member_type.downcase, relation_member.member do %> <%= if relation_member.member_role.blank? diff --git a/app/views/browse/_relation_member_frame.html.erb b/app/views/browse/_relation_member_frame.html.erb new file mode 100644 index 000000000..97531f17a --- /dev/null +++ b/app/views/browse/_relation_member_frame.html.erb @@ -0,0 +1,5 @@ + + + diff --git a/app/views/old_relation_members/_not_found_message.html.erb b/app/views/old_relation_members/_not_found_message.html.erb new file mode 100644 index 000000000..a4598eb8f --- /dev/null +++ b/app/views/old_relation_members/_not_found_message.html.erb @@ -0,0 +1,7 @@ +
+ <% if params[:version] %> +

<%= t ".sorry", :id => params[:id], :version => params[:version] %>

+ <% else %> +

<%= t "relations.not_found_message.sorry", :id => params[:id] %>

+ <% end %> +
diff --git a/app/views/old_relation_members/timeout.html.erb b/app/views/old_relation_members/timeout.html.erb new file mode 100644 index 000000000..61415c650 --- /dev/null +++ b/app/views/old_relation_members/timeout.html.erb @@ -0,0 +1,5 @@ +<% set_title(t("browse.timeout.title")) %> + +<%= render "sidebar_header", :title => t("browse.timeout.title") %> + +

<%= t ".sorry", :id => params[:id] %>

diff --git a/app/views/relation_members/_not_found_message.html.erb b/app/views/relation_members/_not_found_message.html.erb new file mode 100644 index 000000000..9297f3b9a --- /dev/null +++ b/app/views/relation_members/_not_found_message.html.erb @@ -0,0 +1,3 @@ +
+

<%= t ".sorry", :id => params[:id] %>

+
diff --git a/app/views/relation_members/timeout.html.erb b/app/views/relation_members/timeout.html.erb new file mode 100644 index 000000000..61415c650 --- /dev/null +++ b/app/views/relation_members/timeout.html.erb @@ -0,0 +1,5 @@ +<% set_title(t("browse.timeout.title")) %> + +<%= render "sidebar_header", :title => t("browse.timeout.title") %> + +

<%= t ".sorry", :id => params[:id] %>

diff --git a/config/locales/en.yml b/config/locales/en.yml index 43392c22d..d00e5eb44 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -483,11 +483,21 @@ en: sorry: "Sorry, relation #%{id} could not be found." timeout: sorry: "Sorry, the data for the relation with the id %{id} took too long to retrieve." + relation_members: + not_found_message: + sorry: "Sorry, relation #%{id} could not be found." + timeout: + sorry: "Sorry, the data for the relation with the id %{id} took too long to retrieve." old_relations: not_found_message: sorry: "Sorry, relation #%{id} version %{version} could not be found." timeout: sorry: "Sorry, the history of the relation with the id %{id} took too long to retrieve." + old_relation_members: + not_found_message: + sorry: "Sorry, relation #%{id} version %{version} could not be found." + timeout: + sorry: "Sorry, the members of the relation with the id %{id} took too long to retrieve." changeset_comments: feeds: comment: diff --git a/config/routes.rb b/config/routes.rb index e84e77d31..bfbc1477b 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -142,9 +142,13 @@ OpenStreetMap::Application.routes.draw do get "/node/:id" => "nodes#show", :id => /\d+/, :as => :node get "/node/:id/history" => "old_nodes#index", :id => /\d+/, :as => :node_history resources :old_nodes, :path => "/node/:id/history", :id => /\d+/, :version => /\d+/, :param => :version, :only => :show + get "/relation/:id" => "relations#show", :id => /\d+/, :as => :relation + get "/relation/:id/members" => "relation_members#show", :id => /\d+/, :as => :relation_members + get "/relation/:id/history" => "old_relations#index", :id => /\d+/, :as => :relation_history resources :old_relations, :path => "/relation/:id/history", :id => /\d+/, :version => /\d+/, :param => :version, :only => :show + get "/relation/:id/history/:version/members" => "old_relation_members#show", :id => /\d+/, :version => /\d+/, :as => :old_relation_members resources :changesets, :path => "changeset", :id => /\d+/, :only => :show do resource :subscription, :controller => :changeset_subscriptions, :only => [:show, :create, :destroy] diff --git a/test/controllers/old_relation_members_controller_test.rb b/test/controllers/old_relation_members_controller_test.rb new file mode 100644 index 000000000..0ffed06e4 --- /dev/null +++ b/test/controllers/old_relation_members_controller_test.rb @@ -0,0 +1,54 @@ +require "test_helper" + +class OldRelationMembersControllerTest < ActionDispatch::IntegrationTest + def test_routes + assert_routing( + { :path => "/relation/1/history/2/members", :method => :get }, + { :controller => "old_relation_members", :action => "show", :id => "1", :version => "2" } + ) + end + + def test_show_with_members + relation = create(:relation, :with_history) + create(:old_relation_member, :old_relation => relation.old_relations.first) + + get old_relation_members_path(relation, 1) + + assert_response :success + end + + def test_show_redacted_to_unauthorized_users + relation = create(:relation, :with_history, :version => 2) + relation.old_relations.find_by(:version => 1).redact!(create(:redaction)) + + get old_relation_members_path(relation, 1, :params => { :show_redactions => true }) + + assert_response :redirect + end + + def test_show_redacted_to_regular_users + relation = create(:relation, :with_history, :version => 2) + relation.old_relations.find_by(:version => 1).redact!(create(:redaction)) + + session_for(create(:user)) + get old_relation_members_path(relation, 1, :params => { :show_redactions => true }) + + assert_response :redirect + end + + def test_show_not_found + get old_relation_members_path(0, 0) + + assert_response :not_found + end + + def test_show_timeout + relation = create(:relation, :with_history) + + with_settings(:web_timeout => -1) do + get old_relation_members_path(relation, 1) + end + + assert_response :error + end +end diff --git a/test/controllers/relation_members_controller_test.rb b/test/controllers/relation_members_controller_test.rb new file mode 100644 index 000000000..236f3af19 --- /dev/null +++ b/test/controllers/relation_members_controller_test.rb @@ -0,0 +1,34 @@ +require "test_helper" + +class RelationMembersControllerTest < ActionDispatch::IntegrationTest + ## + # test all routes which lead to this controller + def test_routes + assert_routing( + { :path => "/relation/1/members", :method => :get }, + { :controller => "relation_members", :action => "show", :id => "1" } + ) + end + + def test_show_with_members + relation = create(:relation) + get relation_members_path(relation) + assert_response :success + end + + def test_show_not_found + get relation_members_path(0) + + assert_response :not_found + end + + def test_show_timeout + relation = create(:relation) + + with_settings(:web_timeout => -1) do + get relation_members_path(relation, 1) + end + + assert_response :error + end +end -- 2.39.5