Merge remote-tracking branch 'upstream/pull/3232'

diff --git a/app/assets/stylesheets/common.scss b/app/assets/stylesheets/common.scss
index 1ec9e08e08c3be457c40934c5d40b44393cf3c0a..736125bedde96c163161c0c7da7fe90bcee8276e 100644 (file)
--- a/app/assets/stylesheets/common.scss
+++ b/app/assets/stylesheets/common.scss
@@ -1404,25 +1404,6 @@ tr.turn:hover {
   &.notice {
     background-color: #CBEEA7;
   }
-
-  div.message {
-    display: inline-block;
-    margin-left: $lineheight / 2;
-    vertical-align: middle;
-
-    p {
-      margin-top: $lineheight * 0.5;
-      margin-bottom: $lineheight * 0.5;
-
-      &:first-child {
-        margin-top: 0px;
-      }
-
-      &:last-child {
-        margin-bottom: 0px;
-      }
-    }
-  }
 }
 
 /* Rules for highlighting fields with rails validation errors */

diff --git a/app/assets/stylesheets/small.scss b/app/assets/stylesheets/small.scss
index 51501758d77cf6b0d5d438793c422921d3c5611d..2247b4bbef940956db7ddf5097da17803ed15141 100644 (file)
--- a/app/assets/stylesheets/small.scss
+++ b/app/assets/stylesheets/small.scss
@@ -20,10 +20,6 @@ body.small-nav {
     min-height: $headerHeight;
     background: #fff;
 
-    h1 {
-      padding-bottom: 15px;
-    }
-
     &.closed nav {
       display: none;
     }

diff --git a/app/controllers/confirmations_controller.rb b/app/controllers/confirmations_controller.rb
index f9de54df0aa20559ada7d2e3b30bbe0be2bebe26..372ac2a70c0dc2277902fc43f71d3bee25a94fdc 100644 (file)
--- a/app/controllers/confirmations_controller.rb
+++ b/app/controllers/confirmations_controller.rb
@@ -66,7 +66,7 @@ class ConfirmationsController < ApplicationController
       flash[:error] = t "confirmations.confirm_resend.failure", :name => params[:display_name]
     else
       UserMailer.signup_confirm(user, user.tokens.create).deliver_later
-      flash[:notice] = t "confirmations.confirm_resend.success_html", :email => user.email, :sender => Settings.email_from
+      flash[:notice] = { :partial => "confirmations/resend_success_flash", :locals => { :email => user.email, :sender => Settings.email_from } }
     end
 
     redirect_to login_path

diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index 43933041cab0543a6653a014b7bb0f569962e5e2..00d427ac944680bdf21273b871f656fa3f557614 100644 (file)
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -42,7 +42,7 @@ class UsersController < ApplicationController
       if current_user
         current_user.terms_seen = true
 
-        flash[:notice] = t("users.new.terms declined", :url => t("users.new.terms declined url")).html_safe if current_user.save
+        flash[:notice] = { :partial => "users/terms_declined_flash" } if current_user.save
 
         if params[:referer]
           redirect_to safe_referer(params[:referer])

diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index ae90d980e81bc3de61da28000617f6103af3551d..faf538fa926917dc429136d1a960eebd4ba7e13e 100644 (file)
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -68,4 +68,14 @@ module ApplicationHelper
 
     data
   end
+
+  # If the flash is a hash, then it will be a partial with a hash of locals, so we can call `render` on that
+  # This allows us to render html into a flash message in a safe manner.
+  def render_flash(flash)
+    if flash.is_a?(Hash)
+      render flash.with_indifferent_access
+    else
+      flash
+    end
+  end
 end

diff --git a/app/views/confirmations/_resend_success_flash.html.erb b/app/views/confirmations/_resend_success_flash.html.erb
new file mode 100644 (file)
index 0000000..e9763e2
--- /dev/null
+++ b/app/views/confirmations/_resend_success_flash.html.erb
@@ -0,0 +1,2 @@
+<p><%= t ".confirmation_sent", :email => email %></p>
+<p><%= t ".whitelist", :sender => sender %></p>

diff --git a/app/views/layouts/_flash.html.erb b/app/views/layouts/_flash.html.erb
index 0f39c4a47f3f7d0df5f415422e4fc749ff5d1615..f8eabaeb32a051daaeb9c140b8f0a3b4e95f688f 100644 (file)
--- a/app/views/layouts/_flash.html.erb
+++ b/app/views/layouts/_flash.html.erb
@@ -1,29 +1,35 @@
 <% if flash[:error] %>
-  <div class="flash error">
-    <picture>
-      <source srcset="<%= image_path "notice.svg" %>" type="image/svg+xml" />
-      <%= image_tag("notice.png", :srcset => image_path("notice.svg"), :class => "small_icon", :border => 0) %>
-    </picture>
-    <div class="message"><%= flash[:error] %></div>
+  <div class="flash error row align-items-center">
+    <div class="col-auto">
+      <picture>
+        <source srcset="<%= image_path "notice.svg" %>" type="image/svg+xml" />
+        <%= image_tag("notice.png", :srcset => image_path("notice.svg")) %>
+      </picture>
+    </div>
+    <div class="col"><%= render_flash(flash[:error]) %></div>
   </div>
 <% end %>
 
 <% if flash[:warning] %>
-  <div class="flash warning">
-    <picture>
-      <source srcset="<%= image_path "notice.svg" %>" type="image/svg+xml"></source>
-      <%= image_tag("notice.png", :srcset => image_path("notice.svg"), :class => "small_icon", :border => 0) %>
-    </picture>
-    <div class="message"><%= flash[:warning] %></div>
+  <div class="flash warning row align-items-center">
+    <div class="col-auto">
+      <picture>
+        <source srcset="<%= image_path "notice.svg" %>" type="image/svg+xml"></source>
+        <%= image_tag("notice.png", :srcset => image_path("notice.svg")) %>
+      </picture>
+    </div>
+    <div class="col"><%= render_flash(flash[:warning]) %></div>
   </div>
 <% end %>
 
 <% if flash[:notice] %>
-  <div class="flash notice">
-    <picture>
-      <source srcset="<%= image_path "notice.svg" %>" type="image/svg+xml"></source>
-      <%= image_tag("notice.png", :srcset => image_path("notice.svg"), :class => "small_icon", :border => 0) %>
-    </picture>
-    <div class="message"><%= flash[:notice] %></div>
+  <div class="flash notice row align-items-center">
+    <div class="col-auto">
+      <picture>
+        <source srcset="<%= image_path "notice.svg" %>" type="image/svg+xml"></source>
+        <%= image_tag("notice.png", :srcset => image_path("notice.svg")) %>
+      </picture>
+    </div>
+    <div class="col"><%= render_flash(flash[:notice]) %></div>
   </div>
 <% end %>

diff --git a/app/views/layouts/_header.html.erb b/app/views/layouts/_header.html.erb
index 06823e18f81f1debcd658cbb2e0c61f8a55159f6..fe5bf23c5b4eb0327d0ea7b498b6add8940062ec 100644 (file)
--- a/app/views/layouts/_header.html.erb
+++ b/app/views/layouts/_header.html.erb
@@ -1,4 +1,4 @@
-<header class="closed">
+<header class="closed clearfix">
   <h1>
     <a href="<%= root_path %>" class="geolink">
       <picture>

diff --git a/app/views/users/_terms_declined_flash.html.erb b/app/views/users/_terms_declined_flash.html.erb
new file mode 100644 (file)
index 0000000..9c9d793
--- /dev/null
+++ b/app/views/users/_terms_declined_flash.html.erb
@@ -0,0 +1 @@
+<%= t ".terms_declined_html", :terms_declined_link => link_to(t(".terms_declined_link"), t(".terms_declined_url")) %>

diff --git a/config/locales/en.yml b/config/locales/en.yml
index 25464bdcfc4e767838f4304df129e35439d88f7a..094a6e9c1c40687d90d313d776ae2d9303362903 100644 (file)
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -1535,7 +1535,6 @@ en:
       unknown token: "That confirmation code has expired or does not exist."
       reconfirm_html: "If you need us to resend the confirmation email, <a href=\"%{reconfirm}\">click here</a>."
     confirm_resend:
-      success_html: "We've sent a new confirmation note to %{email} and as soon as you confirm your account you'll be able to get mapping.<br /><br />If you use an antispam system which sends confirmation requests then please make sure you whitelist %{sender} as we are unable to reply to any confirmation requests."
       failure: "User %{name} not found."
     confirm_email:
       heading: Confirm a change of email address
@@ -1544,6 +1543,9 @@ en:
       success: "Confirmed your change of email address!"
       failure: "An email address has already been confirmed with this token."
       unknown_token: "That confirmation code has expired or does not exist."
+    resend_success_flash:
+      confirmation_sent: We've sent a new confirmation note to %{email} and as soon as you confirm your account you'll be able to get mapping.
+      whitelist: If you use an antispam system which sends confirmation requests then please make sure you whitelist %{sender} as we are unable to reply to any confirmation requests.
   messages:
     inbox:
       title: "Inbox"
@@ -2332,8 +2334,6 @@ en:
       auth no password: "With third party authentication a password is not required, but some extra tools or server may still need one."
       continue: Sign Up
       terms accepted: "Thanks for accepting the new contributor terms!"
-      terms declined: "We are sorry that you have decided to not accept the new Contributor Terms. For more information, please see <a href=\"%{url}\">this wiki page</a>."
-      terms declined url: https://wiki.openstreetmap.org/wiki/Contributor_Terms_Declined
     terms:
       title: "Terms"
       heading: "Terms"
@@ -2356,6 +2356,10 @@ en:
         france: "France"
         italy: "Italy"
         rest_of_world: "Rest of the world"
+    terms_declined_flash:
+      terms_declined_html: We are sorry that you have decided to not accept the new Contributor Terms. For more information, please see %{terms_declined_link}.
+      terms_declined_link: this wiki page
+      terms_declined_url: https://wiki.openstreetmap.org/wiki/Contributor_Terms_Declined
     no_such_user:
       title: "No such user"
       heading: "The user %{user} does not exist"

diff --git a/test/controllers/confirmations_controller_test.rb b/test/controllers/confirmations_controller_test.rb
index bae0158cdfddb9e2ed968d1a894686a0f38955c7..f583e5e39ca819e0218a98847d39c2b4c442a085 100644 (file)
--- a/test/controllers/confirmations_controller_test.rb
+++ b/test/controllers/confirmations_controller_test.rb
@@ -211,7 +211,8 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
 
     assert_response :redirect
     assert_redirected_to login_path
-    assert_match(/sent a new confirmation/, flash[:notice])
+    assert_equal("confirmations/resend_success_flash", flash[:notice][:partial])
+    assert_equal({ :email => user.email, :sender => Settings.email_from }, flash[:notice][:locals])
 
     email = ActionMailer::Base.deliveries.last
 

diff --git a/test/system/confirmation_resend.rb b/test/system/confirmation_resend.rb
new file mode 100644 (file)
index 0000000..6bbde09
--- /dev/null
+++ b/test/system/confirmation_resend.rb
@@ -0,0 +1,26 @@
+require "application_system_test_case"
+
+class ConfirmationResendSystemTest < ApplicationSystemTestCase
+  def setup
+    @user = build(:user)
+    visit user_new_path
+
+    fill_in "Email", :with => @user.email
+    fill_in "Email Confirmation", :with => @user.email
+    fill_in "Display Name", :with => @user.display_name
+    fill_in "Password", :with => "testtest"
+    fill_in "Confirm Password", :with => "testtest"
+    click_button "Sign Up"
+
+    check "I have read and agree to the above contributor terms"
+    check "I have read and agree to the Terms of Use"
+    click_button "Continue"
+  end
+
+  test "flash message should not contain raw html" do
+    visit user_confirm_resend_path(@user)
+
+    assert page.has_content?("sent a new confirmation")
+    assert_not page.has_content?("<p>")
+  end
+end