diff --git a/app/controllers/admin_controller.rb b/app/controllers/admin_controller.rb index 36f7e7ab..c2fd8416 100644 --- a/app/controllers/admin_controller.rb +++ b/app/controllers/admin_controller.rb @@ -4,7 +4,7 @@ class AdminController < ApplicationController before_action :require_admin_user def index - @maps = filter_and_sort_maps(Map.includes(:layers, :user)) + @maps = filter_and_sort_maps(Map.includes(:layers, :owners)) respond_to do |format| format.html # full page diff --git a/app/controllers/concerns/map_list_filters.rb b/app/controllers/concerns/map_list_filters.rb index 4b95d709..cb838fc2 100644 --- a/app/controllers/concerns/map_list_filters.rb +++ b/app/controllers/concerns/map_list_filters.rb @@ -11,7 +11,10 @@ def filter_and_sort_maps(maps) @searchuser = User.find(userid) if userid.present? end - maps = maps.where(user: @searchuser) if @searchuser + if @searchuser + # Find maps where user is in owner_ids array + maps = maps.where(owner_ids: @searchuser.id) + end maps = maps.search(@filter) unless @filter.empty? maps = maps.sorted(@sort, @direction) maps.limit(params[:limit] || 300) diff --git a/app/controllers/maps_controller.rb b/app/controllers/maps_controller.rb index 44236f5d..c6912ebf 100644 --- a/app/controllers/maps_controller.rb +++ b/app/controllers/maps_controller.rb @@ -13,12 +13,12 @@ class MapsController < ApplicationController layout "map", only: [ :show, :tutorial ] def index - @maps = filter_and_sort_maps(Map.unscoped.listed.includes(:layers, :user)) + @maps = filter_and_sort_maps(Map.unscoped.listed.includes(:layers, :owners)) end def my @recent_map_ids = @user.recent_map_ids - @my_maps = filter_and_sort_maps(Map.where(user: @user).includes(:layers, :user)) + @my_maps = filter_and_sort_maps(@user.owned_maps.includes(:layers, :owners)) respond_to do |format| format.html # full page @@ -41,7 +41,7 @@ def show gon.map_id = params[:id] gon.user_id = @user.id if @user - gon.edit_id = @map.private_id.to_s if @user&.admin? || (@user && @map.user == @user) + gon.edit_id = @map.private_id.to_s if @user&.admin? || @map.owned_by?(@user) gon.map_mode = @map_mode gon.rails_env = Rails.env gon.csrf_token = form_authenticity_token @@ -70,7 +70,9 @@ def tutorial def create coords = ip_coordinates - @map = Map.create!(user: @user, center: coords) + @map = Map.new(center: coords) + @map.add_owner(@user) + @map.save! redirect_to @map.private_map_path, notice: "Map was successfully created." end @@ -78,7 +80,8 @@ def create def copy require_map_owner if @map.view_permission == "private" cloned_map = @map.clone_with_layers - cloned_map.update(user: @user, name: "Copy of " + @map.name.to_s) + cloned_map.update(name: "Copy of " + @map.name.to_s) + cloned_map.add_owner(@user) redirect_to cloned_map.private_map_path, notice: "Map was successfully copied." end @@ -136,7 +139,7 @@ def ip_coordinates # :nocov: def require_map_owner - if !(@user&.admin? || (@map.user && @map.user == @user)) + if !(@user&.admin? || @map.owned_by?(@user)) Rails.logger.warn "Map view requires owner permissions, but current user isn't." redirect_to maps_path end diff --git a/app/models/map.rb b/app/models/map.rb index a711dd1a..aeb896eb 100644 --- a/app/models/map.rb +++ b/app/models/map.rb @@ -5,7 +5,11 @@ class Map include Turbo::Broadcastable has_many :layers, dependent: :destroy - belongs_to :user, optional: true, counter_cache: true + + # Many-to-many relationship with users (stores owner_ids array) + has_and_belongs_to_many :owners, + class_name: "User", + inverse_of: nil # implicit_order_column is not supported by mongoid default_scope { sorted(:created_at, :asc) } @@ -34,8 +38,8 @@ class Map field :bearing, type: String field :name, type: String field :description, type: String - field :private_id, type: String, default: -> { SecureRandom.hex(6).tap { |i| i[0..1] = "11" } } - field :public_id, type: String, default: -> { SecureRandom.hex(4).tap { |i| i[0..1] = "11" } } + field :private_id, type: String, default: -> { SecureRandom.hex(6) } + field :public_id, type: String, default: -> { SecureRandom.hex(4) } field :viewed_at, type: DateTime field :view_count, type: Integer, default: 0 field :type, type: String @@ -102,6 +106,23 @@ class Map format: { without: /\//, message: "private_id cannot contain a '/'" }, if: :will_save_change_to_private_id? + # Check if a user is an owner + def owned_by?(user) + return false unless user + owner_ids.include?(user.id) + end + + # Add an owner (idempotent) + def add_owner(user) + return if owned_by?(user) + owners << user + end + + # Remove an owner (allows maps with no owners for anonymous/demo maps) + def remove_owner(user) + owners.delete(user) + end + def properties { name: name, description: description, @@ -207,10 +228,11 @@ def self.tutorial_map(user) tutorial_file = Rails.root.join("db/seeds/demo.json") if user&.name - unless (map = Map.tutorial.where(user: user).first) + unless (map = user.owned_maps.tutorial.first) map = Map.create_from_file(tutorial_file) name = user.name.split.first - map.update(user: user, type: "tutorial") + map.update(type: "tutorial") + map.add_owner(user) map.features.where("properties.label" => "Welcome to the Mapforge Tutorial map") .update_all("properties.label" => "Welcome #{name} to the Mapforge Tutorial map") end @@ -299,6 +321,7 @@ def delete_screenshot File.delete(screenshot_file) if File.exist?(screenshot_file) end + # make public id safe for file names def safe_public_id separator = "_" public_id.strip diff --git a/app/models/user.rb b/app/models/user.rb index 2c208cd1..62da8b3e 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -3,12 +3,10 @@ class User include Mongoid::Document include Mongoid::Timestamps - has_many :maps - scope :admin, -> { where(admin: true) } scope :github, -> { where(provider: "github") } scope :google, -> { where(provider: "google_oauth2") } - scope :with_maps, -> { where(:maps_count.gt => 0) } + scope :with_maps, -> { where(:_id.in => Map.distinct(:owner_ids)) } scope :with_images, -> { where(:images_count.gt => 0) } field :uid @@ -17,10 +15,14 @@ class User field :email field :image field :admin - field :maps_count, type: Integer, default: 0 field :images_count, type: Integer, default: 0 field :recent_map_ids, type: Array, default: [] + # Get all maps where this user is an owner + def owned_maps + Map.where(owner_ids: id) + end + # Track a map view and maintain a limited history of recently viewed maps # The list can store private as well as public map ids # @param map [Map] the map being viewed diff --git a/app/views/maps/_map.haml b/app/views/maps/_map.haml index 1a26c23c..1aaa7586 100644 --- a/app/views/maps/_map.haml +++ b/app/views/maps/_map.haml @@ -10,10 +10,10 @@ .map-auth{title: 'This map is private and not linked from the public listing.'} %i.bi.bi-lock-fill - - if map.user && avatar - - if map.user.image - = link_to = link_to url_for(params: { search: "user:#{map.user.id}"}), 'aria-label': "Browse maps by user #{map.user.email}" do - = image_tag avatar_url(map.user.image, 40), class: 'map-avatar', loading: 'lazy', alt: "", crossorigin: "anonymous" + - if map.owners.first && avatar + - if map.owners.first.image + = link_to = link_to url_for(params: { search: "user:#{map.owners.first.id}"}), 'aria-label': "Browse maps by user #{map.owners.first.email}" do + = image_tag avatar_url(map.owners.first.image, 40), class: 'map-avatar', loading: 'lazy', alt: "", crossorigin: "anonymous" - if clone = link_to copy_map_path(id: map.public_id), data: { turbo_method: :post, turbo_confirm: "Do you want to copy this map?" }, title: 'Copy map', class: 'map-copy' do diff --git a/app/views/maps/modals/_share.haml b/app/views/maps/modals/_share.haml index 36b85c8a..b1cf83d1 100644 --- a/app/views/maps/modals/_share.haml +++ b/app/views/maps/modals/_share.haml @@ -8,7 +8,7 @@ .feature-section-card.pt-3 - if @map_mode == 'rw' - - if @map.user + - if @map.owners.any? %p You can access this map under '#{link_to "Your maps", my_path}'. - else @@ -27,7 +27,7 @@ .form-floating.edit-ui %label{ for: "map-edit-permissions"} Edit permissions: %select.form-select#map-edit-permissions{ "name": "map-visibility", "aria-label": "Edit permissions", data: { action: "change->map--share#updateEditPermissions" }} - - if @map.user + - if @map.owners.any? %option{ value: 'private' } Only you %option{ value: 'link' } Everybody with the edit link @@ -41,7 +41,7 @@ .form-floating.edit-ui %label{ for: "map-view-permissions"} View permissions: %select.form-select#map-view-permissions{ "name": "map-visibility", "aria-label": "View permissions", data: { action: "change->map--share#updateViewPermissions" }} - - if @map.user + - if @map.owners.any? %option{ value: 'private' } Only you %option{ value: 'link' } Everybody with the view link %option{ value: 'listed' } Everybody, map is shown on mapforge diff --git a/app/views/shared/_header.html.haml b/app/views/shared/_header.html.haml index 165dcb46..ac8213e3 100644 --- a/app/views/shared/_header.html.haml +++ b/app/views/shared/_header.html.haml @@ -41,7 +41,7 @@ %li.nav-item = button_to my_path, { method: :get, class: 'mb-2 btn btn-green w-100 no-wrap' } do %i.bi.bi-map.me-1 - = "Your maps (#{@user.maps_count})" + = "Your maps (#{@user.owned_maps.count})" - if @user&.admin? %li.nav-item @@ -88,7 +88,7 @@ %li = link_to my_path, { class: 'dropdown-item' } do %i.bi.bi-map - = "Your maps (#{@user.maps_count})" + = "Your maps (#{@user.owned_maps.count})" %li = link_to logout_path, { class: 'dropdown-item', data: { turbo: false } } do %i.bi.bi-box-arrow-in-left diff --git a/db/migrate/20260328003620_convert_to_multi_owner.rb b/db/migrate/20260328003620_convert_to_multi_owner.rb new file mode 100644 index 00000000..27359c0f --- /dev/null +++ b/db/migrate/20260328003620_convert_to_multi_owner.rb @@ -0,0 +1,29 @@ +class ConvertToMultiOwner < Mongoid::Migration + def self.up + # Maps with users: convert user_id to owner_ids array + Map.collection.update_many( + { user_id: { '$ne' => nil } }, + [ { '$set' => { owner_ids: [ '$user_id' ] } } ] + ) + + # Anonymous maps: set empty owner_ids array + Map.collection.update_many( + { user_id: nil }, + { '$set' => { owner_ids: [] } } + ) + + # Remove user_id field + # Map.collection.update_many({}, { '$unset' => { user_id: '' } }) + end + + def self.down + # Restore user_id from first owner + Map.collection.update_many( + { owner_ids: { '$ne' => [] } }, + [ { '$set' => { user_id: { '$first' => '$owner_ids' } } } ] + ) + + # Remove owner_ids field + # Map.collection.update_many({}, { '$unset' => { owner_ids: '' } }) + end +end diff --git a/engines/ulogger/app/controllers/api/ulogger_controller.rb b/engines/ulogger/app/controllers/api/ulogger_controller.rb index 81817266..17176c2e 100644 --- a/engines/ulogger/app/controllers/api/ulogger_controller.rb +++ b/engines/ulogger/app/controllers/api/ulogger_controller.rb @@ -29,10 +29,11 @@ def addtrack @map = Map.find_by(private_id: $1) else session["track_name"] = params[:track] - @map = Map.create!(private_id: random_map_id, name: params[:track], + @map = Map.new(private_id: random_map_id, name: params[:track], view_permission: "link", - edit_permission: "link", - user: @user) + edit_permission: "link") + @map.add_owner(@user) if @user + @map.save! end if @map render json: { error: false, trackid: @map.private_id.to_i } @@ -118,7 +119,7 @@ def save_image(uploaded) filename = "ulogger-#{SecureRandom.hex(4)}.#{ext}" uid = Dragonfly.app.store(uploaded.tempfile, "name" => filename) # name needs to be a string here - Image.create(img_uid: uid, public_id: filename, user: @map.user) + Image.create(img_uid: uid, public_id: filename, user: @map.owners.first) end def image_properties(img) diff --git a/spec/factories/maps.rb b/spec/factories/maps.rb index a64bd9bd..5c74b995 100644 --- a/spec/factories/maps.rb +++ b/spec/factories/maps.rb @@ -7,10 +7,16 @@ transient do features { nil } + owner { nil } end after :create do |map, evaluator| map.layers.first.update(features: evaluator.features) + + # Add owner if provided + if evaluator.owner + map.add_owner(evaluator.owner) + end end end end diff --git a/spec/features/map_permissions_spec.rb b/spec/features/map_permissions_spec.rb index e540c61d..b57c3b9c 100644 --- a/spec/features/map_permissions_spec.rb +++ b/spec/features/map_permissions_spec.rb @@ -9,7 +9,7 @@ end context "private" do - subject(:map) { create(:map, edit_permission: "private", user: user) } + subject(:map) { create(:map, edit_permission: "private", owners: [ user ]) } it "is not accessible via link" do expect(page).to have_current_path(maps_path) @@ -37,7 +37,7 @@ end context "private" do - subject(:map) { create(:map, view_permission: "private", user: user) } + subject(:map) { create(:map, view_permission: "private", owners: [ user ]) } it "is not accessible via link" do expect(page).to have_current_path(maps_path) diff --git a/spec/features/map_share_spec.rb b/spec/features/map_share_spec.rb index 582b7122..80b62ac7 100644 --- a/spec/features/map_share_spec.rb +++ b/spec/features/map_share_spec.rb @@ -1,7 +1,7 @@ require "rails_helper" describe "Map" do - subject(:map) { create(:map, name: "Test Map", user: create(:user)) } + subject(:map) { create(:map, name: "Test Map", owners: [ create(:user) ]) } context "share links" do before do @@ -33,7 +33,7 @@ end context "export" do - subject(:map) { create(:map, user: create(:user), features: features) } + subject(:map) { create(:map, owners: [ create(:user) ], features: features) } let(:features) { create_list(:feature, 2, :line_string) } @@ -49,7 +49,7 @@ end context "export gpx" do - subject(:map) { create(:map, user: create(:user), features: features) } + subject(:map) { create(:map, owners: [ create(:user) ], features: features) } let(:features) { [ create(:feature, :line_string, coordinates: [ [ 11.041, 49.481 ], [ 11.056, 49.463 ] ]), diff --git a/spec/features/map_view_spec.rb b/spec/features/map_view_spec.rb index 0ae7bf12..d0a24ef2 100644 --- a/spec/features/map_view_spec.rb +++ b/spec/features/map_view_spec.rb @@ -173,7 +173,7 @@ end context "as map owner / admin" do - let(:map) { create(:map, user: user) } + let(:map) { create(:map, owners: [ user ]) } let(:user) { create(:user) } before do diff --git a/spec/features/maps_list_spec.rb b/spec/features/maps_list_spec.rb index f1272d7b..49163bd9 100644 --- a/spec/features/maps_list_spec.rb +++ b/spec/features/maps_list_spec.rb @@ -25,7 +25,7 @@ context "filter list" do let(:maps) { [ create(:map, name: "Map1", view_permission: "listed"), - create(:map, name: "Map2", user: user, view_permission: "listed") ] + create(:map, name: "Map2", owners: [ user ], view_permission: "listed") ] } it "searches in map names" do diff --git a/spec/features/maps_my_list_spec.rb b/spec/features/maps_my_list_spec.rb index 27e18b25..02c0b8fd 100644 --- a/spec/features/maps_my_list_spec.rb +++ b/spec/features/maps_my_list_spec.rb @@ -5,28 +5,28 @@ before do allow_any_instance_of(ApplicationController).to receive(:session).and_return({ user_id: user.id }) - create(:map, user: user) + create(:map, owners: [ user ]) visit my_path end it "shows private links to maps" do - expect(page).to have_selector(:xpath, "//a[@href='/m/#{user.maps.first.private_id}']") + expect(page).to have_selector(:xpath, "//a[@href='/m/#{user.owned_maps.first.private_id}']") end it "can delete own map" do - expect(user.maps_count).to eq 1 + expect(user.owned_maps.count).to eq 1 expect(page).to have_css(".map-preview") accept_alert do find(".map-delete", match: :first).click end expect(page).not_to have_css(".map-preview") - expect(user.reload.maps_count).to eq 0 + expect(user.reload.owned_maps.count).to eq 0 end context "filter list" do before { - [ create(:map, user: user, name: "Map1"), - create(:map, user: user, name: "Map2") ] + [ create(:map, owners: [ user ], name: "Map1"), + create(:map, owners: [ user ], name: "Map2") ] } it "searches in map names" do diff --git a/spec/models/map_spec.rb b/spec/models/map_spec.rb index ec247511..a3667187 100644 --- a/spec/models/map_spec.rb +++ b/spec/models/map_spec.rb @@ -121,4 +121,52 @@ expect(clone.features.map(&:id)).not_to match_array(subject.features.map(&:id)) end end + + describe 'multi-owner support' do + let(:user1) { create(:user) } + let(:user2) { create(:user) } + let(:map) { create(:map, owners: [ user1 ]) } + + describe '#owned_by?' do + it 'returns true for owners' do + expect(map.owned_by?(user1)).to be true + end + + it 'returns false for non-owners' do + expect(map.owned_by?(user2)).to be false + end + end + + describe '#add_owner' do + it 'adds a new owner' do + expect { map.add_owner(user2) }.to change { map.owners.count }.by(1) + end + + it 'is idempotent' do + map.add_owner(user2) + expect { map.add_owner(user2) }.not_to change { map.owners.count } + end + end + + describe '#remove_owner' do + it 'removes an owner' do + map.add_owner(user2) + expect { map.remove_owner(user2) }.to change { map.owners.count }.by(-1) + end + + it 'allows removing last owner (for anonymous maps)' do + expect { map.remove_owner(user1) }.to change { map.owners.count }.by(-1) + expect(map.owners).to be_empty + end + end + + describe 'anonymous maps' do + it 'allows maps without owners' do + map = create(:map) + map.owners.clear + expect(map.owners).to be_empty + expect(map).to be_valid + end + end + end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 2a8a2e90..1e9a03ed 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -4,7 +4,7 @@ subject(:user) { create :user } it "with defaults" do - expect(user.maps_count).to be_zero + expect(user.owned_maps.count).to be_zero expect(user.images_count).to be_zero expect(user.recent_map_ids).to eq([]) end