From: Anton Khorev Date: Wed, 29 Jan 2025 15:22:21 +0000 (+0300) Subject: Merge branch 'pull/4990' X-Git-Tag: live~376 X-Git-Url: https://git.openstreetmap.org/rails.git/commitdiff_plain/0cd18eb02f853bfa93673f046b793bd53ac5aa68?hp=1a23bfa1ecae74ea47dfb76eadd4e6bc856fed70 Merge branch 'pull/4990' --- diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 25722c1f4..bfeb3d33a 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -65,7 +65,7 @@ jobs: - name: Run javascript tests run: bundle exec teaspoon - name: Report completion to Coveralls - uses: coverallsapp/github-action@v2.3.4 + uses: coverallsapp/github-action@v2.3.6 with: github-token: ${{ secrets.github_token }} flag-name: ubuntu-${{ matrix.ubuntu }}-ruby-${{ matrix.ruby }} @@ -77,7 +77,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Report completion to Coveralls - uses: coverallsapp/github-action@v2.3.4 + uses: coverallsapp/github-action@v2.3.6 with: github-token: ${{ secrets.github_token }} parallel-finished: true diff --git a/.rubocop.yml b/.rubocop.yml index 60d144544..68a7ca003 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -7,6 +7,7 @@ require: - rubocop-performance - rubocop-rails - rubocop-rake + - ./.rubocop/specific_action_names.rb AllCops: TargetRubyVersion: 3.1 @@ -110,3 +111,41 @@ Style/StringLiterals: Style/SymbolArray: EnforcedStyle: brackets + +Rails/SpecificActionNames: + Description: Use only specific action names. + Enabled: true + ActionNames: + - index + - show + - new + - edit + - create + - update + - destroy + Include: + - app/controllers/**/*.rb + Exclude: + # This is a todo list, but is currently too long for `rubocop --auto-gen-config` + - 'app/controllers/api/changeset_comments_controller.rb' + - 'app/controllers/api/changesets_controller.rb' + - 'app/controllers/api/nodes_controller.rb' + - 'app/controllers/api/notes_controller.rb' + - 'app/controllers/api/old_elements_controller.rb' + - 'app/controllers/api/relations_controller.rb' + - 'app/controllers/api/user_preferences_controller.rb' + - 'app/controllers/api/users_controller.rb' + - 'app/controllers/api/ways_controller.rb' + - 'app/controllers/browse_controller.rb' + - 'app/controllers/changesets_controller.rb' + - 'app/controllers/confirmations_controller.rb' + - 'app/controllers/diary_comments_controller.rb' + - 'app/controllers/diary_entries_controller.rb' + - 'app/controllers/directions_controller.rb' + - 'app/controllers/errors_controller.rb' + - 'app/controllers/export_controller.rb' + - 'app/controllers/geocoder_controller.rb' + - 'app/controllers/issues_controller.rb' + - 'app/controllers/site_controller.rb' + - 'app/controllers/traces_controller.rb' + - 'app/controllers/users_controller.rb' diff --git a/.rubocop/specific_action_names.rb b/.rubocop/specific_action_names.rb new file mode 100644 index 000000000..e78790bac --- /dev/null +++ b/.rubocop/specific_action_names.rb @@ -0,0 +1,69 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Rails + # Use only specific action names. + # + # It is good practice to separate controller classes rather than adding more actions as needed. + # By default, the 7 CRUD action names are specified that are generated by the Rails scaffold. + # + # @example + # # bad + # class UsersController < ApplicationController + # def articles + # end + # end + # + # # good + # class UserArticlesController < ApplicationController + # def index + # end + # end + class SpecificActionNames < Base + include VisibilityHelp + + MSG = "Use only specific action names." + + # @param node [RuboCop::AST::DefNode] + # @return [void] + def on_def(node) + return unless bad?(node) + + add_offense( + node.location.name, + :message => format( + "Use only specific action names (%s).", + :action_names => configured_action_names.join(", ") + ) + ) + end + + private + + # @param node [RuboCop::AST::DefNode] + # @return [Boolean] + def action?(node) + node_visibility(node) == :public + end + + # @param node [RuboCop::AST::DefNode] + # @return [Boolean] + def bad?(node) + action?(node) && !configured_action_name?(node) + end + + # @param node [RuboCop::AST::DefNode] + # @return [Boolean] + def configured_action_name?(node) + configured_action_names.include?(node.method_name.to_s) + end + + # @return [Array] + def configured_action_names + cop_config["ActionNames"] + end + end + end + end +end diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index fa15ea90a..3d7cea500 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -106,19 +106,6 @@ Minitest/EmptyLineBeforeAssertionMethods: Minitest/MultipleAssertions: Max: 60 -# Offense count: 10 -# This cop supports unsafe autocorrection (--autocorrect-all). -Rails/ActionControllerFlashBeforeRender: - Exclude: - - 'app/controllers/application_controller.rb' - - 'app/controllers/confirmations_controller.rb' - - 'app/controllers/issue_comments_controller.rb' - - 'app/controllers/messages_controller.rb' - - 'app/controllers/passwords_controller.rb' - - 'app/controllers/traces_controller.rb' - - 'app/controllers/user_blocks_controller.rb' - - 'app/controllers/users_controller.rb' - # Offense count: 2 # Configuration parameters: Include. # Include: app/models/**/*.rb diff --git a/Gemfile b/Gemfile index 60a8fcace..2765b1ae7 100644 --- a/Gemfile +++ b/Gemfile @@ -130,9 +130,6 @@ gem "ffi-libarchive" gem "gd2-ffij", ">= 0.4.0" gem "marcel" -# Used for browser detection -gem "browser", "< 6" # for ruby 3.1 support - # Used for S3 object storage gem "aws-sdk-s3" diff --git a/Gemfile.lock b/Gemfile.lock index 615787927..761beefa3 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -92,8 +92,8 @@ GEM autoprefixer-rails (10.4.19.0) execjs (~> 2) aws-eventstream (1.3.0) - aws-partitions (1.1040.0) - aws-sdk-core (3.216.0) + aws-partitions (1.1043.0) + aws-sdk-core (3.217.0) aws-eventstream (~> 1, >= 1.3.0) aws-partitions (~> 1, >= 1.992.0) aws-sigv4 (~> 1.9) @@ -134,13 +134,12 @@ GEM brakeman (7.0.0) racc brotli (0.6.0) - browser (5.3.1) builder (3.3.0) bzip2-ffi (1.1.1) ffi (~> 1.0) cancancan (3.6.1) - canonical-rails (0.2.16) - actionview (>= 4.1, < 7.3) + canonical-rails (0.2.17) + actionview (>= 4.1, < 8.1) capybara (3.40.0) addressable matrix @@ -291,7 +290,7 @@ GEM rchardet (~> 1.8) globalid (1.2.1) activesupport (>= 6.1) - google-protobuf (3.25.5) + google-protobuf (3.25.6) hashdiff (1.1.2) hashie (5.0.0) highline (3.1.2) @@ -329,7 +328,7 @@ GEM in_threads (1.6.0) iniparse (1.5.0) io-console (0.8.0) - irb (1.15.0) + irb (1.15.1) pp (>= 0.6.0) rdoc (>= 4.0.0) reline (>= 0.4.2) @@ -349,7 +348,7 @@ GEM rexml (>= 3.3.9) kramdown-parser-gfm (1.1.0) kramdown (~> 2.0) - language_server-protocol (3.17.0.3) + language_server-protocol (3.17.0.4) libv8-node (18.19.0.0) libxml-ruby (5.0.3) listen (3.9.0) @@ -452,7 +451,7 @@ GEM open4 (1.3.4) openstreetmap-deadlock_retry (1.3.1) ostruct (0.6.1) - overcommit (0.64.1) + overcommit (0.65.0) childprocess (>= 0.6.3, < 6) iniparse (~> 1.4) rexml (>= 3.3.9) @@ -550,7 +549,7 @@ GEM rouge (4.5.1) rtlcss (0.2.1) mini_racer (>= 0.6.3) - rubocop (1.70.0) + rubocop (1.71.0) json (~> 2.3) language_server-protocol (>= 3.17.0) parallel (~> 1.10) @@ -560,7 +559,7 @@ GEM rubocop-ast (>= 1.36.2, < 2.0) ruby-progressbar (~> 1.7) unicode-display_width (>= 2.4.0, < 4.0) - rubocop-ast (1.37.0) + rubocop-ast (1.38.0) parser (>= 3.3.1.0) rubocop-capybara (2.21.0) rubocop (~> 1.41) @@ -572,7 +571,7 @@ GEM rubocop-performance (1.23.1) rubocop (>= 1.48.1, < 2.0) rubocop-ast (>= 1.31.1, < 2.0) - rubocop-rails (2.29.0) + rubocop-rails (2.29.1) activesupport (>= 4.2.0) rack (>= 1.1) rubocop (>= 1.52.0, < 2.0) @@ -683,7 +682,6 @@ DEPENDENCIES bootstrap (~> 5.3.2) bootstrap_form (~> 5.0) brakeman - browser (< 6) bzip2-ffi cancancan canonical-rails diff --git a/app/abilities/ability.rb b/app/abilities/ability.rb index 3ba2ab707..3116bc5cd 100644 --- a/app/abilities/ability.rb +++ b/app/abilities/ability.rb @@ -32,7 +32,7 @@ class Ability can :read, [:deletion, :account_terms] if Settings.status != "database_offline" - can [:subscribe, :unsubscribe], Changeset + can [:read, :create, :destroy], :changeset_subscription can [:read, :create, :update, :destroy], :oauth2_application can [:read, :destroy], :oauth2_authorized_application can [:read, :create, :destroy], :oauth2_authorization @@ -43,7 +43,7 @@ class Ability can :update, DiaryEntry, :user => user can [:create], DiaryComment can [:show, :create, :destroy], Follow - can [:read, :create, :mark, :unmute, :destroy], Message + can [:read, :create, :destroy], Message can [:close, :reopen], Note can [:read, :update], :preference can :update, :profile diff --git a/app/assets/config/manifest.js b/app/assets/config/manifest.js index 85b5cb860..3b1f63f8b 100644 --- a/app/assets/config/manifest.js +++ b/app/assets/config/manifest.js @@ -17,8 +17,6 @@ //= link_tree ../../../vendor/assets/leaflet .png -//= link_directory ../../../vendor/assets/polyfill .js - //= link leaflet/dist/images/marker-icon.png //= link leaflet/dist/images/marker-icon-2x.png //= link leaflet/dist/images/marker-shadow.png diff --git a/app/assets/javascripts/application.js b/app/assets/javascripts/application.js index ead01a8ff..29563cb4a 100644 --- a/app/assets/javascripts/application.js +++ b/app/assets/javascripts/application.js @@ -16,6 +16,29 @@ //= require richtext //= require qs/dist/qs +{ + const application_data = $("head").data(); + + I18n.default_locale = OSM.DEFAULT_LOCALE; + I18n.locale = application_data.locale; + I18n.fallbacks = true; + + OSM.preferred_editor = application_data.preferredEditor; + OSM.preferred_languages = application_data.preferredLanguages; + + if (application_data.user) { + OSM.user = application_data.user; + + if (application_data.userHome) { + OSM.home = application_data.userHome; + } + } + + if (application_data.location) { + OSM.location = application_data.location; + } +} + /* * Called as the user scrolls/zooms around to manipulate hrefs of the * view tab and various other links @@ -122,27 +145,6 @@ $(document).ready(function () { $("header").toggleClass("closed"); }); - var application_data = $("head").data(); - - I18n.default_locale = OSM.DEFAULT_LOCALE; - I18n.locale = application_data.locale; - I18n.fallbacks = true; - - OSM.preferred_editor = application_data.preferredEditor; - OSM.preferred_languages = application_data.preferredLanguages; - - if (application_data.user) { - OSM.user = application_data.user; - - if (application_data.userHome) { - OSM.home = application_data.userHome; - } - } - - if (application_data.location) { - OSM.location = application_data.location; - } - $("#edit_tab") .attr("title", I18n.t("javascripts.site.edit_disabled_tooltip")); }); diff --git a/app/assets/javascripts/edit/id.js.erb b/app/assets/javascripts/edit/id.js.erb index 90318788c..08e04dcf3 100644 --- a/app/assets/javascripts/edit/id.js.erb +++ b/app/assets/javascripts/edit/id.js.erb @@ -10,15 +10,15 @@ $(document).ready(function () { var params = {}; if (mapParams.object) { - params.id = mapParams.object.type + '/' + mapParams.object.id; + params.id = mapParams.object.type + "/" + mapParams.object.id; mapParams = OSM.parseHash(location.hash); if (mapParams.center) { - params.map = mapParams.zoom + '/' + mapParams.center.lat + '/' + mapParams.center.lng; + params.map = mapParams.zoom + "/" + mapParams.center.lat + "/" + mapParams.center.lng; } } else if (id.data("lat") && id.data("lon")) { params.map = "16/" + id.data("lat") + "/" + id.data("lon"); } else { - params.map = (mapParams.zoom || 17) + '/' + mapParams.lat + '/' + mapParams.lon; + params.map = (mapParams.zoom || 17) + "/" + mapParams.lat + "/" + mapParams.lon; } if (hashParams.background) params.background = hashParams.background; diff --git a/app/assets/javascripts/embed.js.erb b/app/assets/javascripts/embed.js.erb index b9e9a2eaf..ef6f00670 100644 --- a/app/assets/javascripts/embed.js.erb +++ b/app/assets/javascripts/embed.js.erb @@ -15,13 +15,13 @@ I18n.default_locale = <%= I18n.default_locale.to_json %>; I18n.fallbacks = true; window.onload = function () { - var query = (window.location.search || '?').slice(1), - args = {}; + var query = (window.location.search || "?").slice(1), + args = {}; - var pairs = query.split('&'); + var pairs = query.split("&"); for (var i = 0; i < pairs.length; i++) { - var parts = pairs[i].split('='); - args[parts[0]] = decodeURIComponent(parts[1] || ''); + var parts = pairs[i].split("="); + args[parts[0]] = decodeURIComponent(parts[1] || ""); } var mapnikOptions = { @@ -37,7 +37,7 @@ window.onload = function () { }; var map = L.map("map"); - map.attributionControl.setPrefix(''); + map.attributionControl.setPrefix(""); map.removeControl(map.attributionControl); if (args.layer === "cyclosm") { @@ -53,19 +53,21 @@ window.onload = function () { } if (args.marker) { - L.marker(args.marker.split(','), {icon: L.icon({ + L.marker(args.marker.split(","), { icon: L.icon({ iconUrl: <%= asset_path('leaflet/dist/images/marker-icon.png').to_json %>, iconSize: new L.Point(25, 41), iconAnchor: new L.Point(12, 41), shadowUrl: <%= asset_path('leaflet/dist/images/marker-shadow.png').to_json %>, shadowSize: new L.Point(41, 41) - })}).addTo(map); + }) }).addTo(map); } if (args.bbox) { - var bbox = args.bbox.split(','); - map.fitBounds([L.latLng(bbox[1], bbox[0]), - L.latLng(bbox[3], bbox[2])]); + var bbox = args.bbox.split(","); + map.fitBounds([ + L.latLng(bbox[1], bbox[0]), + L.latLng(bbox[3], bbox[2]) + ]); } else { map.fitWorld(); } @@ -75,14 +77,14 @@ window.onload = function () { L.Control.OSMReportAProblem = L.Control.Attribution.extend({ options: { - position: 'bottomright', - prefix: ''+I18n.t('javascripts.embed.report_problem')+'' + position: "bottomright", + prefix: "" + I18n.t("javascripts.embed.report_problem") + "" }, onAdd: function (map) { var container = L.Control.Attribution.prototype.onAdd.call(this, map); - map.on('moveend', this._update, this); + map.on("moveend", this._update, this); return container; }, @@ -92,8 +94,8 @@ L.Control.OSMReportAProblem = L.Control.Attribution.extend({ this._container.innerHTML = this._container.innerHTML - .replace('{x}', this._map.getCenter().lat) - .replace('{y}', this._map.getCenter().lng) - .replace('{z}', this._map.getZoom()); + .replace("{x}", this._map.getCenter().lat) + .replace("{y}", this._map.getCenter().lng) + .replace("{z}", this._map.getZoom()); } }); diff --git a/app/assets/javascripts/index.js b/app/assets/javascripts/index.js index 56495b31a..d61191fb4 100644 --- a/app/assets/javascripts/index.js +++ b/app/assets/javascripts/index.js @@ -173,7 +173,7 @@ $(document).ready(function () { var expiry = new Date(); expiry.setYear(expiry.getFullYear() + 10); - map.on("moveend layeradd layerremove", function () { + map.on("moveend baselayerchange overlayadd overlayremove", function () { updateLinks( map.getCenter().wrap(), map.getZoom(), @@ -205,7 +205,7 @@ $(document).ready(function () { }); if (OSM.MATOMO) { - map.on("layeradd", function (e) { + map.on("baselayerchange overlayadd", function (e) { if (e.layer.options) { var goal = OSM.MATOMO.goals[e.layer.options.layerId]; diff --git a/app/assets/javascripts/index/layers/data.js b/app/assets/javascripts/index/layers/data.js index 0a996f0c7..e1246ca6c 100644 --- a/app/assets/javascripts/index/layers/data.js +++ b/app/assets/javascripts/index/layers/data.js @@ -25,18 +25,16 @@ OSM.initializeDataLayer = function (map) { onSelect(e.layer); }); - map.on("layeradd", function (e) { - if (e.layer === dataLayer) { - map.on("moveend", updateData); - updateData(); - } + dataLayer.on("add", function () { + map.fire("overlayadd", { layer: this }); + map.on("moveend", updateData); + updateData(); }); - map.on("layerremove", function (e) { - if (e.layer === dataLayer) { - map.off("moveend", updateData); - $("#browse_status").empty(); - } + dataLayer.on("remove", function () { + map.off("moveend", updateData); + $("#browse_status").empty(); + map.fire("overlayremove", { layer: this }); }); function updateData() { @@ -46,7 +44,7 @@ OSM.initializeDataLayer = function (map) { } } - function displayFeatureWarning(count, limit, add, cancel) { + function displayFeatureWarning(num_features, add, cancel) { $("#browse_status").html( $("
").append( $("
").append( @@ -57,12 +55,28 @@ OSM.initializeDataLayer = function (map) { .attr("aria-label", I18n.t("javascripts.close")) .click(cancel))), $("

") - .text(I18n.t("browse.start_rjs.feature_warning", { num_features: count, max_features: limit })), + .text(I18n.t("browse.start_rjs.feature_warning", { num_features })), $("") .val(I18n.t("browse.start_rjs.load_data")) .click(add))); } + function displayLoadError(message, close) { + $("#browse_status").html( + $("

").append( + $("
").append( + $("

") + .text(I18n.t("browse.start_rjs.load_data")), + $("
").append( + $("