From: Tom Hughes Date: Sun, 16 Feb 2025 09:04:17 +0000 (+0000) Subject: Merge remote-tracking branch 'upstream/pull/4359' X-Git-Tag: live~264 X-Git-Url: https://git.openstreetmap.org/rails.git/commitdiff_plain/d90f353e5f30c35fdfc61a462fdb39b3504998ce?hp=2a5051376aac75c95cc57e3f39c3868cd5cf7c9f Merge remote-tracking branch 'upstream/pull/4359' --- diff --git a/app/abilities/ability.rb b/app/abilities/ability.rb index adedce543..dd377a727 100644 --- a/app/abilities/ability.rb +++ b/app/abilities/ability.rb @@ -29,7 +29,7 @@ class Ability if user&.active? can :welcome, :site - can :read, [:deletion, :account_terms, :account_pd_declaration] + can :read, [:deletion, :account_terms, :account_pd_declaration, :account_home] if Settings.status != "database_offline" can [:read, :create, :destroy], :changeset_subscription diff --git a/app/abilities/api_ability.rb b/app/abilities/api_ability.rb index d183aca74..acacec049 100644 --- a/app/abilities/api_ability.rb +++ b/app/abilities/api_ability.rb @@ -44,7 +44,9 @@ class ApiAbility can :destroy, Note if scopes.include?("write_notes") - can :redact, [OldNode, OldWay, OldRelation] if user&.terms_agreed? && scopes.include?("write_redactions") + can :redact, [OldNode, OldWay, OldRelation] if user.terms_agreed? && scopes.include?("write_redactions") + + can :create, UserBlock if scopes.include?("write_blocks") end end end diff --git a/app/assets/javascripts/index.js b/app/assets/javascripts/index.js index c3ee1e3bc..810327e3f 100644 --- a/app/assets/javascripts/index.js +++ b/app/assets/javascripts/index.js @@ -21,6 +21,7 @@ //= require index/directions //= require index/changeset //= require index/query +//= require index/home //= require router $(document).ready(function () { @@ -38,6 +39,14 @@ $(document).ready(function () { $("#sidebar_loader").show().addClass("delayed-fade-in"); + // Prevent caching the XHR response as a full-page URL + // https://github.com/openstreetmap/openstreetmap-website/issues/5663 + if (content_path.indexOf("?") >= 0) { + content_path += "&xhr=1"; + } else { + content_path += "?xhr=1"; + } + $("#sidebar_content") .empty(); @@ -213,16 +222,6 @@ $(document).ready(function () { L.marker([params.mlat, params.mlon]).addTo(map); } - $("#homeanchor").on("click", function (e) { - e.preventDefault(); - - var data = $(this).data(), - center = L.latLng(data.lat, data.lon); - - map.setView(center, data.zoom); - L.marker(center, { icon: OSM.getUserIcon() }).addTo(map); - }); - function remoteEditHandler(bbox, object) { var remoteEditHost = "http://127.0.0.1:8111", osmHost = location.protocol + "//" + location.host, @@ -309,8 +308,9 @@ $(document).ready(function () { }; function addObject(type, id, center) { + var hashParams = OSM.parseHash(window.location.hash); map.addObject({ type: type, id: parseInt(id, 10) }, function (bounds) { - if (!window.location.hash && bounds.isValid() && + if (!hashParams.center && bounds.isValid() && (center || !map.getBounds().contains(bounds))) { OSM.router.withoutMoveListener(function () { map.fitBounds(bounds); @@ -356,7 +356,8 @@ $(document).ready(function () { "/relation/:id(/history)": OSM.Browse(map, "relation"), "/relation/:id/history/:version": OSM.OldBrowse(), "/changeset/:id": OSM.Changeset(map), - "/query": OSM.Query(map) + "/query": OSM.Query(map), + "/account/home": OSM.Home(map) }); if (OSM.preferred_editor === "remote" && document.location.pathname === "/edit") { diff --git a/app/assets/javascripts/index/changeset.js b/app/assets/javascripts/index/changeset.js index 39b4abde1..d3e61270b 100644 --- a/app/assets/javascripts/index/changeset.js +++ b/app/assets/javascripts/index/changeset.js @@ -12,9 +12,10 @@ OSM.Changeset = function (map) { const changesetData = content.find("[data-changeset]").data("changeset"); changesetData.type = "changeset"; + var hashParams = OSM.parseHash(window.location.hash); initialize(); map.addObject(changesetData, function (bounds) { - if (!window.location.hash && bounds.isValid()) { + if (!hashParams.center && bounds.isValid()) { OSM.router.withoutMoveListener(function () { map.fitBounds(bounds); }); diff --git a/app/assets/javascripts/index/home.js b/app/assets/javascripts/index/home.js new file mode 100644 index 000000000..7e297b724 --- /dev/null +++ b/app/assets/javascripts/index/home.js @@ -0,0 +1,38 @@ +OSM.Home = function (map) { + let marker; + + function clearMarker() { + if (marker) map.removeLayer(marker); + marker = null; + } + + const page = {}; + + page.pushstate = page.popstate = page.load = function () { + map.setSidebarOverlaid(true); + clearMarker(); + + if (OSM.home) { + OSM.router.withoutMoveListener(function () { + map.setView(OSM.home, 15, { reset: true }); + }); + marker = L.marker(OSM.home, { + icon: OSM.getUserIcon(), + title: I18n.t("javascripts.home.marker_title") + }).addTo(map); + } else { + $("#browse_status").html( + $("
").text( + I18n.t("javascripts.home.not_set") + ) + ); + } + }; + + page.unload = function () { + clearMarker(); + $("#browse_status").empty(); + }; + + return page; +}; diff --git a/app/assets/javascripts/index/note.js b/app/assets/javascripts/index/note.js index e9c51f9bf..6a0487aaa 100644 --- a/app/assets/javascripts/index/note.js +++ b/app/assets/javascripts/index/note.js @@ -27,13 +27,16 @@ OSM.Note = function (map) { var data = $(".details").data(); if (!data) return; var latLng = L.latLng(data.coordinates.split(",")); - if (!map.getBounds().contains(latLng)) moveToNote(); + if (!map.getBounds().contains(latLng)) { + OSM.router.withoutMoveListener(function () { + map.setView(latLng, 15, { reset: true }); + }); + } }); }; page.load = function (path, id) { initialize(path, id); - moveToNote(); }; function initialize(path, id) { @@ -48,7 +51,6 @@ OSM.Note = function (map) { success: () => { OSM.loadSidebarContent(path, () => { initialize(path, id); - moveToNote(); }); }, error: (xhr) => { @@ -77,11 +79,19 @@ OSM.Note = function (map) { var data = $(".details").data(); if (data) { + var hashParams = OSM.parseHash(window.location.hash); map.addObject({ type: "note", id: parseInt(id, 10), latLng: L.latLng(data.coordinates.split(",")), icon: noteIcons[data.status] + }, function () { + if (!hashParams.center) { + var latLng = L.latLng(data.coordinates.split(",")); + OSM.router.withoutMoveListener(function () { + map.setView(latLng, 15, { reset: true }); + }); + } }); } } @@ -99,18 +109,6 @@ OSM.Note = function (map) { } } - function moveToNote() { - var data = $(".details").data(); - if (!data) return; - var latLng = L.latLng(data.coordinates.split(",")); - - if (!window.location.hash || window.location.hash.match(/^#?c[0-9]+$/)) { - OSM.router.withoutMoveListener(function () { - map.setView(latLng, 15, { reset: true }); - }); - } - } - page.unload = function () { map.removeObject(); }; diff --git a/app/assets/stylesheets/errors.scss b/app/assets/stylesheets/errors.scss index fd1400232..77b440a88 100644 --- a/app/assets/stylesheets/errors.scss +++ b/app/assets/stylesheets/errors.scss @@ -1,8 +1,43 @@ -.logo { - float: left; - margin: 10px; +body { + margin: 1rem; + margin-top: 2rem; + font-family: system-ui; } -.details { - float: left; +main { + display: flex; + flex-direction: column; + align-items: center; + gap: 1rem 2rem; + max-width: 960px; + + .logo { + flex-shrink: 0; + + img { + display: block; + max-width: 100%; + height: auto; + } + } + + .details { + h1 { + margin-top: 0; + } + } +} + +@media (min-width: 640px) { + body { + margin: 2rem; + } + + main { + flex-direction: row; + + .logo { + align-self: start; + } + } } diff --git a/app/controllers/accounts/homes_controller.rb b/app/controllers/accounts/homes_controller.rb new file mode 100644 index 000000000..e31cce746 --- /dev/null +++ b/app/controllers/accounts/homes_controller.rb @@ -0,0 +1,13 @@ +module Accounts + class HomesController < ApplicationController + layout :map_layout + + before_action :authorize_web + before_action :set_locale + before_action :require_oauth + + authorize_resource :class => :account_home + + def show; end + end +end diff --git a/app/controllers/api/user_blocks_controller.rb b/app/controllers/api/user_blocks_controller.rb index 51f0d26d3..e1fb70a65 100644 --- a/app/controllers/api/user_blocks_controller.rb +++ b/app/controllers/api/user_blocks_controller.rb @@ -1,5 +1,8 @@ module Api class UserBlocksController < ApiController + before_action :check_api_writable, :only => :create + before_action :authorize, :only => :create + authorize_resource before_action :set_request_formats @@ -11,5 +14,33 @@ module Api rescue ActiveRecord::RecordNotFound raise OSM::APINotFoundError end + + def create + raise OSM::APIBadUserInput, "No user was given" unless params[:user] + + user = User.visible.find_by(:id => params[:user]) + raise OSM::APINotFoundError unless user + raise OSM::APIBadUserInput, "No reason was given" unless params[:reason] + raise OSM::APIBadUserInput, "No period was given" unless params[:period] + + period = Integer(params[:period], :exception => false) + raise OSM::APIBadUserInput, "Period should be a number of hours" unless period + + max_period = UserBlock::PERIODS.max + raise OSM::APIBadUserInput, "Period must be between 0 and #{max_period}" if period.negative? || period > max_period + raise OSM::APIBadUserInput, "Needs_view must be true if provided" unless params[:needs_view].nil? || params[:needs_view] == "true" + + ends_at = Time.now.utc + period.hours + needs_view = params[:needs_view] == "true" + @user_block = UserBlock.create( + :user => user, + :creator => current_user, + :reason => params[:reason], + :ends_at => ends_at, + :deactivates_at => (ends_at unless needs_view), + :needs_view => needs_view + ) + render :show + end end end diff --git a/app/controllers/users/lists_controller.rb b/app/controllers/users/lists_controller.rb index a5cd7203d..a2f35e9b2 100644 --- a/app/controllers/users/lists_controller.rb +++ b/app/controllers/users/lists_controller.rb @@ -16,8 +16,8 @@ module Users @params = params.permit(:status, :ip, :before, :after) users = User.all - users = users.where(:status => @params[:status]) if @params[:status] - users = users.where(:creation_address => @params[:ip]) if @params[:ip] + users = users.where(:status => @params[:status]) if @params[:status].present? + users = users.where("creation_address <<= ?", @params[:ip]) if @params[:ip].present? @users_count = users.limit(501).count @users_count = I18n.t("count.at_least_pattern", :count => 500) if @users_count > 500 diff --git a/app/views/accounts/edit.html.erb b/app/views/accounts/edit.html.erb index ce7dd97a8..5c626fc9f 100644 --- a/app/views/accounts/edit.html.erb +++ b/app/views/accounts/edit.html.erb @@ -3,7 +3,7 @@ <% end %> <% content_for :heading do %> -

<%= t ".my settings" %>

+

<%= t ".my_account" %>

<% end %> <%= render :partial => "settings_menu" %> diff --git a/app/views/accounts/homes/show.html.erb b/app/views/accounts/homes/show.html.erb new file mode 100644 index 000000000..ea6ee7088 --- /dev/null +++ b/app/views/accounts/homes/show.html.erb @@ -0,0 +1 @@ +<% content_for(:content_class) { "overlay-sidebar" } %> diff --git a/app/views/layouts/_head.html.erb b/app/views/layouts/_head.html.erb index e6d709b27..37d830ef6 100644 --- a/app/views/layouts/_head.html.erb +++ b/app/views/layouts/_head.html.erb @@ -1,6 +1,5 @@ <%= tag.head :data => application_data do %> - - + <%= render :partial => "layouts/meta" %> <%= javascript_include_tag "turbo", :type => "module" %> <%= javascript_include_tag "application" %> <%= javascript_include_tag "i18n/#{I18n.locale}" %> @@ -11,7 +10,6 @@ <% end %> <%= stylesheet_link_tag "print-#{dir}", :media => "print" %> <%= stylesheet_link_tag "leaflet-all", :media => "screen, print" %> - <%= render :partial => "layouts/meta" %> <%= yield :head %> <%= yield :auto_discovery_link_tag %> <%= csrf_meta_tag %> diff --git a/app/views/layouts/_header.html.erb b/app/views/layouts/_header.html.erb index aa5249381..f09812972 100644 --- a/app/views/layouts/_header.html.erb +++ b/app/views/layouts/_header.html.erb @@ -85,10 +85,12 @@ <%= number_with_delimiter(current_user.new_messages.size) %> <% end %> <%= link_to t("users.show.my profile"), current_user, :class => "dropdown-item" %> - <%= link_to t("users.show.my settings"), edit_account_path, :class => "dropdown-item" %> + <%= link_to t("users.show.my_account"), edit_account_path, :class => "dropdown-item" %> <%= link_to t("users.show.my_preferences"), preferences_path, :class => "dropdown-item" %>
- <%= yield :greeting %> + <% if current_user.home_location? %> + <%= link_to t("layouts.home"), account_home_path, :class => "dropdown-item" %> + <% end %> <%= link_to t("layouts.logout"), logout_path(:referer => request.fullpath), :method => "post", :class => "geolink dropdown-item" %>
diff --git a/app/views/layouts/_meta.html.erb b/app/views/layouts/_meta.html.erb index 4c88887f9..48be6e0aa 100644 --- a/app/views/layouts/_meta.html.erb +++ b/app/views/layouts/_meta.html.erb @@ -1,3 +1,5 @@ + + <% [57, 60, 72, 76, 114, 120, 144, 152, 180].each do |size| -%> <%= favicon_link_tag "apple-touch-icon-#{size}x#{size}.png", :rel => "apple-touch-icon", :sizes => "#{size}x#{size}", :type => "image/png" %> <% end -%> diff --git a/app/views/layouts/error.html.erb b/app/views/layouts/error.html.erb index dfcb3cb91..eab764aa3 100644 --- a/app/views/layouts/error.html.erb +++ b/app/views/layouts/error.html.erb @@ -1,5 +1,5 @@ - + OpenStreetMap @@ -7,11 +7,13 @@ <%= render :partial => "layouts/meta" %> - - <%= image_tag "osm_logo.svg", :alt => t("layouts.logo.alt_text"), :class => "logo" %> - -
- <%= yield %> -
+
+ +
+ <%= yield %> +
+
diff --git a/app/views/layouts/map.html.erb b/app/views/layouts/map.html.erb index e17ea4ed8..72f6076b4 100644 --- a/app/views/layouts/map.html.erb +++ b/app/views/layouts/map.html.erb @@ -4,18 +4,6 @@ <% content_for(:body_class) { "map-layout" } %> -<% if current_user&.home_location? %> - <% content_for :greeting do %> - <%= link_to t("layouts.home"), - "#", - :id => "homeanchor", - :class => "set_position dropdown-item", - :data => { :lat => current_user.home_lat, - :lon => current_user.home_lon, - :zoom => 15 } %> - <% end %> -<% end %> - <% content_for :header do %> <%= render :partial => "layouts/search", :locals => { :autofocus => false } %> <% end %> diff --git a/app/views/users/lists/_page.html.erb b/app/views/users/lists/_page.html.erb index d06516e06..2c06f0237 100644 --- a/app/views/users/lists/_page.html.erb +++ b/app/views/users/lists/_page.html.erb @@ -1,48 +1,52 @@ - <%= form_tag @params, :method => :put do %> -
-
- <%= render "shared/pagination", - :translation_scope => "shared.pagination.users", - :newer_id => @newer_users_id, - :older_id => @older_users_id %> -
-
- <%= t ".found_users", :count => @users_count %> -
-
+ <% unless @users.empty? %> + <%= form_tag @params, :method => :put do %> +
+
+ <%= render "shared/pagination", + :translation_scope => "shared.pagination.users", + :newer_id => @newer_users_id, + :older_id => @older_users_id %> +
+
+ <%= t ".found_users", :count => @users_count %> +
+
- <%= hidden_field_tag :status, params[:status] if params[:status] %> - <%= hidden_field_tag :ip, params[:ip] if params[:ip] %> - <%= hidden_field_tag :page, params[:page] if params[:page] %> - - - - - - - - <%= render :partial => "user", :collection => @users %> -
- - <%= check_box_tag "user_all", "1", false %> -
+ <%= hidden_field_tag :status, params[:status] if params[:status] %> + <%= hidden_field_tag :ip, params[:ip] if params[:ip] %> + <%= hidden_field_tag :page, params[:page] if params[:page] %> + + + + + + + + <%= render :partial => "user", :collection => @users %> +
+ + <%= check_box_tag "user_all", "1", false %> +
-
-
- <%= render "shared/pagination", - :translation_scope => "shared.pagination.users", - :newer_id => @newer_users_id, - :older_id => @older_users_id %> -
-
- <%= t ".found_users", :count => @users_count %> -
-
+
+
+ <%= render "shared/pagination", + :translation_scope => "shared.pagination.users", + :newer_id => @newer_users_id, + :older_id => @older_users_id %> +
+
+ <%= t ".found_users", :count => @users_count %> +
+
-
- <%= submit_tag t(".confirm"), :name => "confirm", :class => "btn btn-primary" %> - <%= submit_tag t(".hide"), :name => "hide", :class => "btn btn-primary" %> -
- <% end %> +
+ <%= submit_tag t(".confirm"), :name => "confirm", :class => "btn btn-primary" %> + <%= submit_tag t(".hide"), :name => "hide", :class => "btn btn-primary" %> +
+ <% end %> + <% else -%> +

<%= t ".empty" %>

+ <% end -%> diff --git a/app/views/users/lists/_user.html.erb b/app/views/users/lists/_user.html.erb index 14216a244..a3900cb38 100644 --- a/app/views/users/lists/_user.html.erb +++ b/app/views/users/lists/_user.html.erb @@ -7,7 +7,7 @@ <% if user.creation_address %> <%= t ".summary_html", :name => link_to(user.display_name, user), - :ip_address => link_to(user.creation_address, :ip => user.creation_address), + :ip_address => link_to(user.creation_address, :status => params[:status], :ip => user.creation_address), :date => l(user.created_at, :format => :friendly) %> <% else %> <%= t ".summary_no_ip_html", diff --git a/app/views/users/lists/show.html.erb b/app/views/users/lists/show.html.erb index abf0092ce..dd037c7af 100644 --- a/app/views/users/lists/show.html.erb +++ b/app/views/users/lists/show.html.erb @@ -8,8 +8,26 @@

<%= t(".heading") %>

<% end %> -<% unless @users.empty? %> - <%= render :partial => "page" %> -<% else %> -

<%= t ".empty" %>

-<% end %> +<%= form_tag(users_list_path, :method => :get, :data => { "turbo" => true, "turbo-frame" => "pagination", "turbo-action" => "advance" }) do %> +
+
+ <%= select_tag :status, + options_for_select(User.aasm.states.map(&:name).map { |state| [t(".states.#{state}"), state] }, params[:status]), + :include_blank => t(".select_status"), + :data => { :behavior => "category_dropdown" }, + :class => "form-select" %> +
+
+ <%= text_field_tag :ip, + params[:ip], + :placeholder => t(".ip_address"), + :autocomplete => "on", + :class => "form-control" %> +
+
+ <%= submit_tag t(".search"), :name => nil, :class => "btn btn-primary" %> +
+
+<% end -%> + +<%= render :partial => "page" %> diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb index 8ef7b5644..a32f5fae1 100644 --- a/app/views/users/show.html.erb +++ b/app/views/users/show.html.erb @@ -30,7 +30,7 @@ <%= number_with_delimiter(current_user.diary_comments.size) %>
  • - <%= link_to t(".my settings"), edit_account_path %> + <%= link_to t(".my_account"), edit_account_path %>
    • <% if current_user.blocks.exists? %> diff --git a/config/locales/en.yml b/config/locales/en.yml index eaf0fdf15..42669ca9e 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -246,7 +246,7 @@ en: accounts: edit: title: "Edit account" - my settings: My Settings + my_account: My Account current email address: "Current Email Address" external auth: "External Authentication" openid: @@ -1922,7 +1922,7 @@ en: failure: Couldn't update profile. sessions: new: - tab_title: "Log in" + tab_title: "Log In" login_to_authorize_html: "Log in to OpenStreetMap to access %{client_app_name}." email or username: "Email Address or Username" password: "Password" @@ -2653,8 +2653,8 @@ en: need_to_see_terms: "Your access to the API is temporarily suspended. Please log-in to the web interface to view the Contributor Terms. You do not need to agree, but you must view them." settings_menu: account_settings: Account Settings - oauth2_applications: OAuth 2 applications - oauth2_authorizations: OAuth 2 authorizations + oauth2_applications: OAuth 2 Applications + oauth2_authorizations: OAuth 2 Authorizations muted_users: Muted Users auth_providers: openid_url: "OpenID URL" @@ -2713,6 +2713,7 @@ en: write_gpx: Upload GPS traces write_notes: Modify notes write_redactions: Redact map data + write_blocks: Create and revoke user blocks read_email: Read user email address consume_messages: Read, update status and delete user messages send_messages: Send private messages to other users @@ -2770,7 +2771,7 @@ en: users: new: title: "Sign Up" - tab_title: "Sign up" + tab_title: "Sign Up" signup_to_authorize_html: "Sign up with OpenStreetMap to access %{client_app_name}." no_auto_account_create: "Unfortunately we are not currently able to create an account for you automatically." please_contact_support_html: 'Please contact %{support_link} to arrange for an account to be created - we will try and deal with the request as quickly as possible.' @@ -2809,7 +2810,7 @@ en: my notes: My Notes my messages: My Messages my profile: My Profile - my settings: My Settings + my_account: My Account my comments: My Comments my_preferences: My Preferences my_dashboard: My Dashboard @@ -2889,13 +2890,22 @@ en: show: title: Users heading: Users - empty: No matching users found + select_status: Select Status + states: + pending: Pending + active: Active + confirmed: Confirmed + suspended: Suspended + deleted: Deleted + ip_address: IP Address + search: Search page: found_users: one: "%{count} user found" other: "%{count} users found" confirm: Confirm Selected Users hide: Hide Selected Users + empty: No matching users found user: summary_html: "%{name} created from %{ip_address} on %{date}" summary_no_ip_html: "%{name} created on %{date}" @@ -2903,7 +2913,7 @@ en: index: heading_html: "%{user}'s Comments" changesets: "Changesets" - diary_entries: "Diary entries" + diary_entries: "Diary Entries" no_comments: "No comments" changeset_comments: index: @@ -3316,6 +3326,9 @@ en: show_address: Show address query_features: Query features centre_map: Centre map here + home: + marker_title: My home location + not_set: Home location is not set for your account redactions: edit: heading: "Edit Redaction" diff --git a/config/routes.rb b/config/routes.rb index 0a6ab0356..0ffd0a546 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -123,7 +123,7 @@ OpenStreetMap::Application.routes.draw do resource :subscription, :only => [:create, :destroy], :controller => "note_subscriptions" end - resources :user_blocks, :only => :show, :id => /\d+/, :controller => "user_blocks" + resources :user_blocks, :only => [:show, :create], :id => /\d+/, :controller => "user_blocks" namespace :user_blocks, :path => "user/blocks" do resource :active_list, :path => "active", :only => :show end @@ -302,6 +302,7 @@ OpenStreetMap::Application.routes.draw do resource :terms, :only => [:show, :update] resource :pd_declaration, :only => [:show, :create] resource :deletion, :only => :show + resource :home, :only => :show end end diff --git a/config/settings/test.yml b/config/settings/test.yml index b7cffcc27..3cf8c2836 100644 --- a/config/settings/test.yml +++ b/config/settings/test.yml @@ -53,5 +53,7 @@ doorkeeper_signing_key: | cK1+/2V+OkM/0nXjxPwPj7LiOediUyZNUn48r29uGOL1S83PSUdyST207CP6mZjc K8aJmnGsVEAcWPzbpNh14q/c -----END PRIVATE KEY----- +# Run system tests using headless Firefox +system_test_headless: true # Override Firefox binary used in system tests #system_test_firefox_binary: diff --git a/lib/oauth.rb b/lib/oauth.rb index dfa3a8028..47edba500 100644 --- a/lib/oauth.rb +++ b/lib/oauth.rb @@ -1,11 +1,11 @@ module Oauth SCOPES = %w[ read_prefs write_prefs write_diary - write_api write_changeset_comments read_gpx write_gpx write_notes write_redactions + write_api write_changeset_comments read_gpx write_gpx write_notes write_redactions write_blocks consume_messages send_messages openid ].freeze PRIVILEGED_SCOPES = %w[read_email skip_authorization].freeze - MODERATOR_SCOPES = %w[write_redactions].freeze + MODERATOR_SCOPES = %w[write_redactions write_blocks].freeze class Scope attr_reader :name diff --git a/test/application_system_test_case.rb b/test/application_system_test_case.rb index 496f37c16..f2fcadfd7 100644 --- a/test/application_system_test_case.rb +++ b/test/application_system_test_case.rb @@ -7,7 +7,7 @@ ActiveSupport.on_load(:action_dispatch_system_test_case) do end class ApplicationSystemTestCase < ActionDispatch::SystemTestCase - driven_by :selenium, :using => :headless_firefox do |options| + driven_by :selenium, :using => Settings.system_test_headless ? :headless_firefox : :firefox do |options| options.add_preference("intl.accept_languages", "en") options.binary = Settings.system_test_firefox_binary if Settings.system_test_firefox_binary end @@ -46,4 +46,8 @@ class ApplicationSystemTestCase < ActionDispatch::SystemTestCase def within_content_body(&) within("#content > .content-body", &) end + + def within_content_heading(&) + within("#content > .content-heading", &) + end end diff --git a/test/controllers/api/user_blocks_controller_test.rb b/test/controllers/api/user_blocks_controller_test.rb index 169338811..2705e332d 100644 --- a/test/controllers/api/user_blocks_controller_test.rb +++ b/test/controllers/api/user_blocks_controller_test.rb @@ -3,6 +3,10 @@ require "test_helper" module Api class UserBlocksControllerTest < ActionDispatch::IntegrationTest def test_routes + assert_routing( + { :path => "/api/0.6/user_blocks", :method => :post }, + { :controller => "api/user_blocks", :action => "create" } + ) assert_routing( { :path => "/api/0.6/user_blocks/1", :method => :get }, { :controller => "api/user_blocks", :action => "show", :id => "1" } @@ -14,11 +18,22 @@ module Api end def test_show - block = create(:user_block) + blocked_user = create(:user) + creator_user = create(:moderator_user) + block = create(:user_block, :user => blocked_user, :creator => creator_user, :reason => "because running tests") get api_user_block_path(block) assert_response :success - assert_select "user_block[id='#{block.id}']", 1 + assert_select "osm>user_block", 1 do + assert_select ">@id", block.id.to_s + assert_select ">user", 1 + assert_select ">user>@uid", blocked_user.id.to_s + assert_select ">creator", 1 + assert_select ">creator>@uid", creator_user.id.to_s + assert_select ">revoker", 0 + assert_select ">reason", 1 + assert_select ">reason", "because running tests" + end get api_user_block_path(block, :format => "json") assert_response :success @@ -32,5 +47,165 @@ module Api assert_response :not_found assert_equal "text/plain", @response.media_type end + + def test_create_no_permission + blocked_user = create(:user) + assert_empty blocked_user.blocks + + post api_user_blocks_path(:user => blocked_user.id, :reason => "because", :period => 1) + assert_response :unauthorized + assert_empty blocked_user.blocks + + regular_creator_user = create(:user) + auth_header = bearer_authorization_header(regular_creator_user, :scopes => %w[read_prefs]) + post api_user_blocks_path(:user => blocked_user.id, :reason => "because", :period => 1), :headers => auth_header + assert_response :forbidden + assert_empty blocked_user.blocks + + auth_header = bearer_authorization_header(regular_creator_user, :scopes => %w[read_prefs write_blocks]) + post api_user_blocks_path(:user => blocked_user.id, :reason => "because", :period => 1), :headers => auth_header + assert_response :forbidden + assert_empty blocked_user.blocks + + moderator_creator_user = create(:moderator_user) + auth_header = bearer_authorization_header(moderator_creator_user, :scopes => %w[read_prefs]) + post api_user_blocks_path(:user => blocked_user.id, :reason => "because", :period => 1), :headers => auth_header + assert_response :forbidden + assert_empty blocked_user.blocks + end + + def test_create_invalid_because_no_user + blocked_user = create(:user, :deleted) + assert_empty blocked_user.blocks + + creator_user = create(:moderator_user) + auth_header = bearer_authorization_header(creator_user, :scopes => %w[read_prefs write_blocks]) + post api_user_blocks_path(:reason => "because", :period => 1), :headers => auth_header + assert_response :bad_request + assert_equal "text/plain", @response.media_type + assert_equal "No user was given", @response.body + + assert_empty blocked_user.blocks + end + + def test_create_invalid_because_user_is_unknown + creator_user = create(:moderator_user) + auth_header = bearer_authorization_header(creator_user, :scopes => %w[read_prefs write_blocks]) + post api_user_blocks_path(:user => 0, :reason => "because", :period => 1), :headers => auth_header + assert_response :not_found + assert_equal "text/plain", @response.media_type + end + + def test_create_invalid_because_user_is_deleted + blocked_user = create(:user, :deleted) + assert_empty blocked_user.blocks + + creator_user = create(:moderator_user) + auth_header = bearer_authorization_header(creator_user, :scopes => %w[read_prefs write_blocks]) + post api_user_blocks_path(:user => blocked_user.id, :reason => "because", :period => 1), :headers => auth_header + assert_response :not_found + assert_equal "text/plain", @response.media_type + + assert_empty blocked_user.blocks + end + + def test_create_invalid_because_missing_reason + create_with_params_and_assert_bad_request("No reason was given", :period => "10") + end + + def test_create_invalid_because_missing_period + create_with_params_and_assert_bad_request("No period was given", :reason => "because") + end + + def test_create_invalid_because_non_numeric_period + create_with_params_and_assert_bad_request("Period should be a number of hours", :reason => "because", :period => "one hour") + end + + def test_create_invalid_because_negative_period + create_with_params_and_assert_bad_request("Period must be between 0 and #{UserBlock::PERIODS.max}", :reason => "go away", :period => "-1") + end + + def test_create_invalid_because_excessive_period + create_with_params_and_assert_bad_request("Period must be between 0 and #{UserBlock::PERIODS.max}", :reason => "go away", :period => "10000000") + end + + def test_create_invalid_because_unknown_needs_view + create_with_params_and_assert_bad_request("Needs_view must be true if provided", :reason => "because", :period => "1", :needs_view => "maybe") + end + + def test_create_success + blocked_user = create(:user) + creator_user = create(:moderator_user) + + assert_empty blocked_user.blocks + auth_header = bearer_authorization_header(creator_user, :scopes => %w[read_prefs write_blocks]) + post api_user_blocks_path(:user => blocked_user.id, :reason => "because", :period => 1), :headers => auth_header + assert_response :success + assert_equal 1, blocked_user.blocks.length + + block = blocked_user.blocks.take + assert_predicate block, :active? + assert_equal "because", block.reason + assert_equal creator_user, block.creator + + assert_equal "application/xml", @response.media_type + assert_select "osm>user_block", 1 do + assert_select ">@id", block.id.to_s + assert_select ">@needs_view", "false" + assert_select ">user", 1 + assert_select ">user>@uid", blocked_user.id.to_s + assert_select ">creator", 1 + assert_select ">creator>@uid", creator_user.id.to_s + assert_select ">revoker", 0 + assert_select ">reason", 1 + assert_select ">reason", "because" + end + end + + def test_create_success_with_needs_view + blocked_user = create(:user) + creator_user = create(:moderator_user) + + assert_empty blocked_user.blocks + auth_header = bearer_authorization_header(creator_user, :scopes => %w[read_prefs write_blocks]) + post api_user_blocks_path(:user => blocked_user.id, :reason => "because", :period => "1", :needs_view => "true"), :headers => auth_header + assert_response :success + assert_equal 1, blocked_user.blocks.length + + block = blocked_user.blocks.take + assert_predicate block, :active? + assert_equal "because", block.reason + assert_equal creator_user, block.creator + + assert_equal "application/xml", @response.media_type + assert_select "osm>user_block", 1 do + assert_select ">@id", block.id.to_s + assert_select ">@needs_view", "true" + assert_select ">user", 1 + assert_select ">user>@uid", blocked_user.id.to_s + assert_select ">creator", 1 + assert_select ">creator>@uid", creator_user.id.to_s + assert_select ">revoker", 0 + assert_select ">reason", 1 + assert_select ">reason", "because" + end + end + + private + + def create_with_params_and_assert_bad_request(message, **params) + blocked_user = create(:user) + assert_empty blocked_user.blocks + + moderator_creator_user = create(:moderator_user) + auth_header = bearer_authorization_header(moderator_creator_user, :scopes => %w[read_prefs write_blocks]) + + post api_user_blocks_path({ :user => blocked_user.id }.merge(params)), :headers => auth_header + assert_response :bad_request + assert_equal "text/plain", @response.media_type + assert_equal message, @response.body + + assert_empty blocked_user.blocks + end end end diff --git a/test/system/account_home_test.rb b/test/system/account_home_test.rb new file mode 100644 index 000000000..813c45ec8 --- /dev/null +++ b/test/system/account_home_test.rb @@ -0,0 +1,57 @@ +require "application_system_test_case" + +class AccountHomeTest < ApplicationSystemTestCase + test "Go to Home Location works on map layout pages" do + user = create(:user, :display_name => "test user", :home_lat => 60, :home_lon => 30) + sign_in_as(user) + + visit root_path + assert_no_selector "img.leaflet-marker-icon" + + click_on "test user" + click_on "Go to Home Location" + all "img.leaflet-marker-icon", :count => 1 do |marker| + assert_equal "My home location", marker["title"] + end + + click_on "OpenStreetMap logo" + assert_no_selector "img.leaflet-marker-icon" + end + + test "Go to Home Location works on non-map layout pages" do + user = create(:user, :display_name => "test user", :home_lat => 60, :home_lon => 30) + sign_in_as(user) + + visit about_path + assert_no_selector "img.leaflet-marker-icon" + + click_on "test user" + click_on "Go to Home Location" + all "img.leaflet-marker-icon", :count => 1 do |marker| + assert_equal "My home location", marker["title"] + end + + click_on "OpenStreetMap logo" + assert_no_selector "img.leaflet-marker-icon" + end + + test "Go to Home Location is not available for users without home location" do + user = create(:user, :display_name => "test user") + sign_in_as(user) + + visit root_path + assert_no_selector "img.leaflet-marker-icon" + + click_on "test user" + assert_no_link "Go to Home Location" + end + + test "account home page shows a warning when visited by users without home location" do + user = create(:user, :display_name => "test user") + sign_in_as(user) + + visit account_home_path + assert_no_selector "img.leaflet-marker-icon" + assert_text "Home location is not set" + end +end diff --git a/test/system/issues_test.rb b/test/system/issues_test.rb index e26ae89ac..ae5e114c3 100644 --- a/test/system/issues_test.rb +++ b/test/system/issues_test.rb @@ -5,7 +5,10 @@ class IssuesTest < ApplicationSystemTestCase def test_view_issues_not_logged_in visit issues_path - assert_content "Log in" + + within_content_heading do + assert_content "Log In" + end end def test_view_issues_normal_user diff --git a/test/system/user_signup_test.rb b/test/system/user_signup_test.rb index 2fb90fc3a..2d05447a6 100644 --- a/test/system/user_signup_test.rb +++ b/test/system/user_signup_test.rb @@ -79,7 +79,9 @@ class UserSignupTest < ApplicationSystemTestCase test "Sign up from login page" do visit login_path - click_on "Sign up" + within_content_heading do + click_on "Sign Up" + end within_content_body do assert_content "Confirm Password" diff --git a/test/system/user_suspension_test.rb b/test/system/user_suspension_test.rb index a72512357..d6368b68b 100644 --- a/test/system/user_suspension_test.rb +++ b/test/system/user_suspension_test.rb @@ -5,7 +5,7 @@ class UserSuspensionTest < ApplicationSystemTestCase user = create(:user) sign_in_as(user) visit edit_account_path - assert_content "My Settings" + assert_content "My Account" user.suspend! diff --git a/test/teaspoon_env.rb b/test/teaspoon_env.rb index c14045b1c..a64274c0a 100644 --- a/test/teaspoon_env.rb +++ b/test/teaspoon_env.rb @@ -100,7 +100,8 @@ Teaspoon.configure do |config| # Capybara Webkit: https://github.com/jejacks0n/teaspoon/wiki/Using-Capybara-Webkit require "selenium-webdriver" config.driver = :selenium - firefox_options = Selenium::WebDriver::Firefox::Options.new(:args => ["-headless"]) + firefox_options = Selenium::WebDriver::Firefox::Options.new + firefox_options.args = ["-headless"] if Settings.system_test_headless firefox_options.binary = Settings.system_test_firefox_binary if Settings.system_test_firefox_binary config.driver_options = { :client_driver => :firefox,