From: Tom Hughes Date: Wed, 13 Nov 2024 18:40:02 +0000 (+0000) Subject: Merge remote-tracking branch 'upstream/pull/5319' X-Git-Tag: live~571 X-Git-Url: https://git.openstreetmap.org/rails.git/commitdiff_plain/49ba5ab6d2e98221246635327eb15dc71c97e67d?hp=42224790b56acea5c653f3085c374f28d56e590d Merge remote-tracking branch 'upstream/pull/5319' --- diff --git a/.github/workflows/danger.yml b/.github/workflows/danger.yml index 8999fca5b..67a676d87 100644 --- a/.github/workflows/danger.yml +++ b/.github/workflows/danger.yml @@ -22,6 +22,12 @@ jobs: ruby-version: 3.1 rubygems: 3.4.10 bundler-cache: true + - name: Create base branch + run: | + git fetch ${{ github.event.pull_request.base.repo.clone_url }} ${{ github.event.pull_request.base.ref }}:danger_base + - name: Create head branch + run: | + git fetch ${{ github.event.pull_request.head.repo.clone_url }} ${{ github.event.pull_request.head.ref }}:danger_head - name: Danger env: DANGER_GITHUB_BEARER_TOKEN: ${{ secrets.GITHUB_TOKEN }} diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 539aaa5cf..e298c944f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -3,6 +3,13 @@ * https://www.ruby-lang.org/ - The homepage of Ruby which has more links and some great tutorials. * https://rubyonrails.org/ - The homepage of Rails, also has links and tutorials. +## Assigning Issues + +We don't assign issues to individual contributors. You are welcome to work on any +issue, and there's no need to ask first. + +For more details see [our FAQ](FAQ.md)] + ## Coding style We use [Rubocop](https://github.com/rubocop-hq/rubocop) (for ruby files) diff --git a/FAQ.md b/FAQ.md index 4ab043338..e53c8dddb 100644 --- a/FAQ.md +++ b/FAQ.md @@ -25,3 +25,13 @@ drive. This is a great way to reach a lot of people! See [PR #1296](https://github.com/openstreetmap/openstreetmap-website/pull/1296) as an example. + +## Why don't you assign issues? + +We don't assign issues to volunteers for several reasons. The main reasons are that it discourages other volunteers from working on the issue, and the process turns into an unproductive administrative overhead for our team. + +There's no need to ask for an issue to be assigned before anyone starts working on it. Everyone is welcome to work on any issue at any time. + +In our experience, most people who ask for an issue to be assigned to them never create a pull request. So we would need to keep track of the assigned issues, and remember to unassign them a week or two into the future, when it is likely that they will not be making a PR. Assigned developers might feel bad if they perceive that we're unhappy with their progress, further discouraging them from contributing. Or we will get drawn into discussions about needing more time, or re-assigning them again, or so on. So it is best not to assign in the first place. + +The risk that two people are both genuinely working on the same task in the same hour or two is vanishingly remote, and doesn't outweigh the downsides described above. A better approach is to encourage people to simply work on the task and create a pull request, at which point everyone knows that they are actually working on the issue and not just planning/hoping/wishing to do so. diff --git a/Gemfile b/Gemfile index b83011542..277346b83 100644 --- a/Gemfile +++ b/Gemfile @@ -148,7 +148,7 @@ gem "zeitwerk", "< 2.7" group :development do gem "better_errors" gem "binding_of_caller" - gem "danger", :github => "tomhughes/danger", :ref => "pull-request-target" + gem "danger" gem "danger-auto_label" gem "debug_inspector" gem "i18n-tasks" diff --git a/Gemfile.lock b/Gemfile.lock index 86a69b9f5..5d15e550c 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,23 +1,3 @@ -GIT - remote: https://github.com/tomhughes/danger.git - revision: a265cf74d2f464a25796b48d95697f5eed553454 - ref: pull-request-target - specs: - danger (9.5.1) - base64 (~> 0.2) - claide (~> 1.0) - claide-plugins (>= 0.9.2) - colored2 (~> 3.1) - cork (~> 0.1) - faraday (>= 0.9.0, < 3.0) - faraday-http-cache (~> 2.0) - git (~> 1.13) - kramdown (~> 2.3) - kramdown-parser-gfm (~> 1.0) - octokit (>= 4.0) - pstore (~> 0.1) - terminal-table (>= 1, < 4) - GEM remote: https://rubygems.org/ specs: @@ -190,6 +170,20 @@ GEM rexml crass (1.0.6) dalli (3.2.8) + danger (9.5.1) + base64 (~> 0.2) + claide (~> 1.0) + claide-plugins (>= 0.9.2) + colored2 (~> 3.1) + cork (~> 0.1) + faraday (>= 0.9.0, < 3.0) + faraday-http-cache (~> 2.0) + git (~> 1.13) + kramdown (~> 2.3) + kramdown-parser-gfm (~> 1.0) + octokit (>= 4.0) + pstore (~> 0.1) + terminal-table (>= 1, < 4) danger-auto_label (1.3.1) danger-plugin-api (~> 1.0) danger-plugin-api (1.0.0) @@ -690,7 +684,7 @@ DEPENDENCIES config connection_pool dalli - danger! + danger danger-auto_label dartsass-sprockets debug diff --git a/Vagrantfile b/Vagrantfile index c2869cd5f..617bd7b4d 100644 --- a/Vagrantfile +++ b/Vagrantfile @@ -2,9 +2,11 @@ # vi: set ft=ruby : Vagrant.configure("2") do |config| - # use official ubuntu image for virtualbox + # use official debian image + config.vm.box = "debian/bookworm64" + + # configure virtualbox provider config.vm.provider "virtualbox" do |vb, override| - override.vm.box = "ubuntu/noble64" override.vm.synced_folder ".", "/srv/openstreetmap-website" vb.customize ["modifyvm", :id, "--memory", "4096"] vb.customize ["modifyvm", :id, "--cpus", "2"] @@ -14,16 +16,16 @@ Vagrant.configure("2") do |config| # Use sshfs sharing if available, otherwise NFS sharing sharing_type = Vagrant.has_plugin?("vagrant-sshfs") ? "sshfs" : "nfs" - # use third party image and sshfs or NFS sharing for lxc + # configure lxc provider config.vm.provider "lxc" do |_, override| - override.vm.box = "generic/ubuntu2404" override.vm.synced_folder ".", "/srv/openstreetmap-website", :type => sharing_type end - # use third party image and sshfs or NFS sharing for libvirt - config.vm.provider "libvirt" do |_, override| - override.vm.box = "generic/ubuntu2404" + # configure libvirt provider + config.vm.provider "libvirt" do |libvirt, override| override.vm.synced_folder ".", "/srv/openstreetmap-website", :type => sharing_type + libvirt.memory = 4096 + libvirt.cpus = 2 end # configure shared package cache if possible diff --git a/app/assets/javascripts/index.js b/app/assets/javascripts/index.js index 2b29992e9..4dfc849fe 100644 --- a/app/assets/javascripts/index.js +++ b/app/assets/javascripts/index.js @@ -25,8 +25,6 @@ //= require qs/dist/qs $(document).ready(function () { - var loaderTimeout; - var map = new L.OSM.Map("map", { zoomControl: false, layerControl: false, @@ -39,11 +37,7 @@ $(document).ready(function () { map.setSidebarOverlaid(false); - clearTimeout(loaderTimeout); - - loaderTimeout = setTimeout(function () { - $("#sidebar_loader").show(); - }, 200); + $("#sidebar_loader").show().addClass("delayed-fade-in"); // IE<10 doesn't respect Vary: X-Requested-With header, so // prevent caching the XHR response as a full-page URL. @@ -60,9 +54,8 @@ $(document).ready(function () { url: content_path, dataType: "html", complete: function (xhr) { - clearTimeout(loaderTimeout); $("#flash").empty(); - $("#sidebar_loader").hide(); + $("#sidebar_loader").removeClass("delayed-fade-in").hide(); var content = $(xhr.responseText); diff --git a/app/assets/javascripts/index/new_note.js b/app/assets/javascripts/index/new_note.js index 59fbeeb1d..887ba043b 100644 --- a/app/assets/javascripts/index/new_note.js +++ b/app/assets/javascripts/index/new_note.js @@ -139,8 +139,6 @@ OSM.NewNote = function (map) { newNote.on("remove", function () { addNoteButton.removeClass("active"); - }).on("dragstart", function () { - $(newNote).stopTime("removenote"); }).on("dragend", function () { content.find("textarea").focus(); }); diff --git a/app/assets/javascripts/richtext.js b/app/assets/javascripts/richtext.js index 0c0a69923..bd00d937e 100644 --- a/app/assets/javascripts/richtext.js +++ b/app/assets/javascripts/richtext.js @@ -8,7 +8,7 @@ var container = $(this).closest(".richtext_container"); var preview = container.find(".tab-pane[id$='_preview']"); - preview.children(".richtext_placeholder").attr("hidden", true); + preview.children(".richtext_placeholder").attr("hidden", true).removeClass("delayed-fade-in"); preview.children(".richtext").empty(); }); @@ -34,13 +34,10 @@ var preview = container.find(".tab-pane[id$='_preview']"); if (preview.children(".richtext").contents().length === 0) { - preview.oneTime(200, "loading", function () { - preview.children(".richtext_placeholder").removeAttr("hidden"); - }); + preview.children(".richtext_placeholder").removeAttr("hidden").addClass("delayed-fade-in"); preview.children(".richtext").load(editor.data("previewUrl"), { text: editor.val() }, function () { - preview.stopTime("loading"); - preview.children(".richtext_placeholder").attr("hidden", true); + preview.children(".richtext_placeholder").attr("hidden", true).removeClass("delayed-fade-in"); }); } }); diff --git a/app/assets/stylesheets/common.scss b/app/assets/stylesheets/common.scss index 8dba773d8..73f8521d7 100644 --- a/app/assets/stylesheets/common.scss +++ b/app/assets/stylesheets/common.scss @@ -70,6 +70,18 @@ time[title] { } } +/* Utility for delayed loading spinner */ + +.delayed-fade-in { + animation: 300ms linear forwards delayed-fade-in; +} + +@keyframes delayed-fade-in { + 0% { opacity: 0 } + 66% { opacity: 0 } + 100% { opacity: 1 } +} + /* Rules for the header */ #menu-icon { diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index a3e6f42f0..abbaf5e92 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -20,7 +20,7 @@ class SessionsController < ApplicationController end def create - session[:remember_me] ||= params[:remember_me] + session[:remember_me] = params[:remember_me] == "yes" referer = safe_referer(params[:referer]) if params[:referer] diff --git a/app/controllers/user_blocks_controller.rb b/app/controllers/user_blocks_controller.rb index c42c2659d..d427e5fa5 100644 --- a/app/controllers/user_blocks_controller.rb +++ b/app/controllers/user_blocks_controller.rb @@ -29,7 +29,7 @@ class UserBlocksController < ApplicationController end def show - if current_user && current_user == @user_block.user + if current_user && current_user == @user_block.user && !@user_block.deactivates_at @user_block.needs_view = false @user_block.deactivates_at = [@user_block.ends_at, Time.now.utc].max @user_block.save! diff --git a/app/views/sessions/new.html.erb b/app/views/sessions/new.html.erb index 9d05d4af8..c2d96b63c 100644 --- a/app/views/sessions/new.html.erb +++ b/app/views/sessions/new.html.erb @@ -40,7 +40,7 @@ <%= f.password_field :password, :autocomplete => "on", :tabindex => 2, :value => "", :skip_label => true %> <%= f.form_group do %> - <%= f.check_box :remember_me, { :label => t(".remember"), :tabindex => 3, :checked => (params[:remember_me] == "yes") }, "yes" %> + <%= f.check_box :remember_me, { :label => t(".remember"), :tabindex => 3, :checked => (params[:remember_me] == "true") }, "yes" %> <% end %>
diff --git a/script/vagrant/setup/provision.sh b/script/vagrant/setup/provision.sh index f19234a8e..f6ecd4ed5 100644 --- a/script/vagrant/setup/provision.sh +++ b/script/vagrant/setup/provision.sh @@ -3,12 +3,6 @@ # abort on error set -e -# set locale to UTF-8 compatible. apologies to non-english speakers... -locale-gen en_GB.utf8 -update-locale LANG=en_GB.utf8 LC_ALL=en_GB.utf8 -export LANG=en_GB.utf8 -export LC_ALL=en_GB.utf8 - # make sure we have up-to-date packages apt-get update @@ -18,7 +12,7 @@ apt-get upgrade -y # install packages as explained in INSTALL.md apt-get install -y ruby ruby-dev ruby-bundler \ libxml2-dev libxslt1-dev nodejs npm \ - build-essential git-core \ + build-essential git-core firefox-esr \ postgresql postgresql-contrib libpq-dev libvips-dev libyaml-dev \ libsasl2-dev libffi-dev libgd-dev libarchive-dev libbz2-dev npm install --global yarn diff --git a/test/controllers/sessions_controller_test.rb b/test/controllers/sessions_controller_test.rb index 914a4ab56..f490b748c 100644 --- a/test/controllers/sessions_controller_test.rb +++ b/test/controllers/sessions_controller_test.rb @@ -54,6 +54,24 @@ class SessionsControllerTest < ActionDispatch::IntegrationTest assert_redirected_to root_path end + def test_login_remembered + user = create(:user) + + post login_path, :params => { :username => user.display_name, :password => "test", :remember_me => "yes" } + assert_redirected_to root_path + + assert_equal 28 * 86400, session[:_remember_for] + end + + def test_login_not_remembered + user = create(:user) + + post login_path, :params => { :username => user.display_name, :password => "test", :remember_me => "0" } + assert_redirected_to root_path + + assert_nil session[:_remember_for] + end + def test_logout_without_referer post logout_path assert_redirected_to root_path diff --git a/test/controllers/user_blocks_controller_test.rb b/test/controllers/user_blocks_controller_test.rb index 1d89476ec..2ab90364e 100644 --- a/test/controllers/user_blocks_controller_test.rb +++ b/test/controllers/user_blocks_controller_test.rb @@ -147,14 +147,72 @@ class UserBlocksControllerTest < ActionDispatch::IntegrationTest assert_select "h1 a[href='#{user_path active_block.user}']", :text => active_block.user.display_name assert_select "h1 a[href='#{user_path active_block.creator}']", :text => active_block.creator.display_name assert UserBlock.find(active_block.id).needs_view + end - # Login as the blocked user - session_for(active_block.user) + ## + # test clearing needs_view by showing a zero-hour block to the blocked user + def test_show_sets_deactivates_at_for_zero_hour_block + user = create(:user) + session_for(user) - # Now viewing it should mark it as seen - get user_block_path(:id => active_block) - assert_response :success - assert_not UserBlock.find(active_block.id).needs_view + freeze_time do + block = create(:user_block, :needs_view, :zero_hour, :user => user) + assert block.needs_view + assert_nil block.deactivates_at + + travel 1.hour + + get user_block_path(block) + assert_response :success + block.reload + assert_not block.needs_view + assert_equal Time.now.utc, block.deactivates_at + + travel 1.hour + + get user_block_path(block) + assert_response :success + block.reload + assert_not block.needs_view + assert_equal Time.now.utc - 1.hour, block.deactivates_at + end + end + + ## + # test clearing needs_view by showing a timed block to the blocked user + def test_show_sets_deactivates_at_for_timed_block + user = create(:user) + session_for(user) + + freeze_time do + block = create(:user_block, :needs_view, :created_at => Time.now.utc, :ends_at => Time.now.utc + 24.hours, :user => user) + assert block.needs_view + assert_nil block.deactivates_at + + travel 1.hour + + get user_block_path(block) + assert_response :success + block.reload + assert_not block.needs_view + assert_equal Time.now.utc + 23.hours, block.deactivates_at + + travel 1.hour + + get user_block_path(block) + assert_response :success + block.reload + assert_not block.needs_view + assert_equal Time.now.utc + 22.hours, block.deactivates_at + + travel 24.hours + + get user_block_path(block) + assert_response :success + block.reload + assert_not block.needs_view + assert_equal Time.now.utc - 2.hours, block.deactivates_at + end end ##