From 1ecd422f7feebd6c23db5cce32222a006a0f2d00 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Thu, 11 Aug 2022 15:28:51 +0200
Subject: [PATCH 1/6] Extract component to render verification info

We're also adding tests showing the current behavior, which we're about
to change.
---
 .../stylesheets/account/verify-account.scss   | 15 +++++++
 app/assets/stylesheets/layout.scss            | 29 -------------
 .../account/verify_account_component.html.erb | 22 ++++++++++
 .../account/verify_account_component.rb       |  7 ++++
 app/views/account/show.html.erb               | 22 +---------
 .../account/verify_account_component_spec.rb  | 41 +++++++++++++++++++
 6 files changed, 86 insertions(+), 50 deletions(-)
 create mode 100644 app/assets/stylesheets/account/verify-account.scss
 create mode 100644 app/components/account/verify_account_component.html.erb
 create mode 100644 app/components/account/verify_account_component.rb
 create mode 100644 spec/components/account/verify_account_component_spec.rb

diff --git a/app/assets/stylesheets/account/verify-account.scss b/app/assets/stylesheets/account/verify-account.scss
new file mode 100644
index 000000000..990e7381d
--- /dev/null
+++ b/app/assets/stylesheets/account/verify-account.scss
@@ -0,0 +1,15 @@
+.verify-account {
+  padding-right: $line-height / 2;
+
+  .already-verified {
+    color: $check;
+    line-height: $line-height * 2;
+
+    .icon-check {
+      color: $text-medium;
+      font-size: rem-calc(24);
+      line-height: rem-calc(45);
+      vertical-align: middle;
+    }
+  }
+}
diff --git a/app/assets/stylesheets/layout.scss b/app/assets/stylesheets/layout.scss
index 0197288f4..d0ef59c55 100644
--- a/app/assets/stylesheets/layout.scss
+++ b/app/assets/stylesheets/layout.scss
@@ -1228,10 +1228,6 @@ form {
     margin-right: $line-height / 2;
   }
 
-  .verify-account {
-    padding-right: $line-height / 2;
-  }
-
   .final-votes-info {
     background: $warning-bg;
     border: 1px solid $warning-border;
@@ -1270,16 +1266,6 @@ form {
   vertical-align: top;
 }
 
-.user-permissions {
-
-  p {
-    span {
-      color: $text-medium;
-      font-size: rem-calc(12);
-    }
-  }
-}
-
 .notifications-list {
   position: relative;
 
@@ -1666,21 +1652,6 @@ table {
   }
 }
 
-.verify-account {
-  padding-right: $line-height / 2;
-
-  .already-verified {
-    color: $check;
-    line-height: $line-height * 2;
-
-    .icon-check {
-      font-size: rem-calc(24);
-      line-height: rem-calc(45);
-      vertical-align: middle;
-    }
-  }
-}
-
 .verify {
   margin-bottom: $line-height * 2;
   margin-top: $line-height;
diff --git a/app/components/account/verify_account_component.html.erb b/app/components/account/verify_account_component.html.erb
new file mode 100644
index 000000000..3de7bee3d
--- /dev/null
+++ b/app/components/account/verify_account_component.html.erb
@@ -0,0 +1,22 @@
+<div class="verification-info">
+  <p>
+    <%= t("account.show.user_permission_verify") %>
+  </p>
+
+  <% unless account.organization? %>
+    <div>
+      <span class="verify-account">
+        <% if account.level_three_verified? %>
+          <p class="already-verified">
+            <span class="icon-check"></span>
+            <%= t("account.show.verified_account") %>
+          </p>
+        <% elsif account.level_two_verified? %>
+          <%= link_to t("account.show.finish_verification"), verification_path, class: "button success" %>
+        <% else %>
+          <%= link_to t("account.show.verify_my_account"), verification_path, class: "button success" %>
+        <% end %>
+      </span>
+    </div>
+  <% end %>
+</div>
diff --git a/app/components/account/verify_account_component.rb b/app/components/account/verify_account_component.rb
new file mode 100644
index 000000000..93a2bb7f1
--- /dev/null
+++ b/app/components/account/verify_account_component.rb
@@ -0,0 +1,7 @@
+class Account::VerifyAccountComponent < ApplicationComponent
+  attr_reader :account
+
+  def initialize(account)
+    @account = account
+  end
+end
diff --git a/app/views/account/show.html.erb b/app/views/account/show.html.erb
index 34c9d104c..77e22a6a8 100644
--- a/app/views/account/show.html.erb
+++ b/app/views/account/show.html.erb
@@ -94,27 +94,7 @@
           <p><%= t("account.show.user_permission_info") %></p>
 
           <%= render Account::PermissionsListComponent.new(current_user) %>
-
-          <p>
-            <%= t("account.show.user_permission_verify") %>
-          </p>
-
-          <% unless @account.organization? %>
-            <div>
-              <span class="verify-account">
-                <% if current_user.level_three_verified? %>
-                  <p class="already-verified">
-                    <span class="icon-check"></span>
-                    <%= t("account.show.verified_account") %>
-                  </p>
-                <% elsif current_user.level_two_verified? %>
-                  <%= link_to t("account.show.finish_verification"), verification_path, class: "button success" %>
-                <% else %>
-                  <%= link_to t("account.show.verify_my_account"), verification_path, class: "button success" %>
-                <% end %>
-              </span>
-            </div>
-          <% end %>
+          <%= render Account::VerifyAccountComponent.new(@account) %>
         </div>
       </div>
     <% end %>
diff --git a/spec/components/account/verify_account_component_spec.rb b/spec/components/account/verify_account_component_spec.rb
new file mode 100644
index 000000000..bb34e506a
--- /dev/null
+++ b/spec/components/account/verify_account_component_spec.rb
@@ -0,0 +1,41 @@
+require "rails_helper"
+
+describe Account::VerifyAccountComponent do
+  it "shows a link to verify account to unverified users" do
+    account = User.new
+
+    render_inline Account::VerifyAccountComponent.new(account)
+
+    expect(page).to have_content "To perform all the actions verify your account."
+    expect(page).to have_link "Verify my account"
+  end
+
+  it "shows a link to complete verification to level two verified users" do
+    account = User.new(level_two_verified_at: Time.current)
+
+    render_inline Account::VerifyAccountComponent.new(account)
+
+    expect(page).to have_content "To perform all the actions verify your account."
+    expect(page).to have_link "Complete verification"
+  end
+
+  it "shows information about a verified account to level three verified users" do
+    account = User.new(verified_at: Time.current)
+
+    render_inline Account::VerifyAccountComponent.new(account)
+
+    expect(page).to have_content "To perform all the actions verify your account."
+    expect(page).to have_content "Account verified"
+  end
+
+  it "does not show verification info to organizations" do
+    account = User.new(organization: Organization.new)
+
+    render_inline Account::VerifyAccountComponent.new(account)
+
+    expect(page).to have_content "To perform all the actions verify your account."
+    expect(page).not_to have_link "Verify my account"
+    expect(page).not_to have_link "Complete verification"
+    expect(page).not_to have_content "Account verified"
+  end
+end

From 3f664e4ccd6638f840e50b72ab9a56f8375d5544 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Thu, 11 Aug 2022 15:35:50 +0200
Subject: [PATCH 2/6] Don't ask verified users to verify their account

It was strange to tell users they need to verify their account in order
to perform all available actions when they've already verified their
account.
---
 .../account/verify_account_component.html.erb | 34 ++++++++-----------
 .../account/verify_account_component.rb       |  4 +++
 .../account/verify_account_component_spec.rb  |  7 ++--
 3 files changed, 21 insertions(+), 24 deletions(-)

diff --git a/app/components/account/verify_account_component.html.erb b/app/components/account/verify_account_component.html.erb
index 3de7bee3d..bcd6c1880 100644
--- a/app/components/account/verify_account_component.html.erb
+++ b/app/components/account/verify_account_component.html.erb
@@ -1,22 +1,18 @@
-<div class="verification-info">
-  <p>
-    <%= t("account.show.user_permission_verify") %>
-  </p>
+<div class="verify-account">
+  <% if account.level_three_verified? %>
+    <p class="already-verified">
+      <span class="icon-check"></span>
+      <%= t("account.show.verified_account") %>
+    </p>
+  <% else %>
+    <p>
+      <%= t("account.show.user_permission_verify") %>
+    </p>
 
-  <% unless account.organization? %>
-    <div>
-      <span class="verify-account">
-        <% if account.level_three_verified? %>
-          <p class="already-verified">
-            <span class="icon-check"></span>
-            <%= t("account.show.verified_account") %>
-          </p>
-        <% elsif account.level_two_verified? %>
-          <%= link_to t("account.show.finish_verification"), verification_path, class: "button success" %>
-        <% else %>
-          <%= link_to t("account.show.verify_my_account"), verification_path, class: "button success" %>
-        <% end %>
-      </span>
-    </div>
+    <% if account.level_two_verified? %>
+      <%= link_to t("account.show.finish_verification"), verification_path, class: "button success" %>
+    <% else %>
+      <%= link_to t("account.show.verify_my_account"), verification_path, class: "button success" %>
+    <% end %>
   <% end %>
 </div>
diff --git a/app/components/account/verify_account_component.rb b/app/components/account/verify_account_component.rb
index 93a2bb7f1..c1aa09c26 100644
--- a/app/components/account/verify_account_component.rb
+++ b/app/components/account/verify_account_component.rb
@@ -4,4 +4,8 @@ class Account::VerifyAccountComponent < ApplicationComponent
   def initialize(account)
     @account = account
   end
+
+  def render?
+    !account.organization?
+  end
 end
diff --git a/spec/components/account/verify_account_component_spec.rb b/spec/components/account/verify_account_component_spec.rb
index bb34e506a..a7921f716 100644
--- a/spec/components/account/verify_account_component_spec.rb
+++ b/spec/components/account/verify_account_component_spec.rb
@@ -24,7 +24,7 @@ describe Account::VerifyAccountComponent do
 
     render_inline Account::VerifyAccountComponent.new(account)
 
-    expect(page).to have_content "To perform all the actions verify your account."
+    expect(page).not_to have_content "To perform all the actions verify your account."
     expect(page).to have_content "Account verified"
   end
 
@@ -33,9 +33,6 @@ describe Account::VerifyAccountComponent do
 
     render_inline Account::VerifyAccountComponent.new(account)
 
-    expect(page).to have_content "To perform all the actions verify your account."
-    expect(page).not_to have_link "Verify my account"
-    expect(page).not_to have_link "Complete verification"
-    expect(page).not_to have_content "Account verified"
+    expect(page).not_to be_rendered
   end
 end

From d59d02ba83ec2c141438033c206464cfece52951 Mon Sep 17 00:00:00 2001
From: decabeza <alberto@decabeza.es>
Date: Thu, 21 Jul 2022 20:37:28 +0200
Subject: [PATCH 3/6] Show verification info only if verification is enabled

---
 .../account/verify_account_component.rb       |  2 +-
 .../account/verify_account_component_spec.rb  | 68 ++++++++++++-------
 2 files changed, 43 insertions(+), 27 deletions(-)

diff --git a/app/components/account/verify_account_component.rb b/app/components/account/verify_account_component.rb
index c1aa09c26..42f2a5cd6 100644
--- a/app/components/account/verify_account_component.rb
+++ b/app/components/account/verify_account_component.rb
@@ -6,6 +6,6 @@ class Account::VerifyAccountComponent < ApplicationComponent
   end
 
   def render?
-    !account.organization?
+    Setting["feature.user.skip_verification"].blank? && !account.organization?
   end
 end
diff --git a/spec/components/account/verify_account_component_spec.rb b/spec/components/account/verify_account_component_spec.rb
index a7921f716..1b425ec82 100644
--- a/spec/components/account/verify_account_component_spec.rb
+++ b/spec/components/account/verify_account_component_spec.rb
@@ -1,38 +1,54 @@
 require "rails_helper"
 
 describe Account::VerifyAccountComponent do
-  it "shows a link to verify account to unverified users" do
-    account = User.new
+  context "verification is enabled" do
+    before { Setting["feature.user.skip_verification"] = false }
 
-    render_inline Account::VerifyAccountComponent.new(account)
+    it "shows a link to verify account to unverified users" do
+      account = User.new
 
-    expect(page).to have_content "To perform all the actions verify your account."
-    expect(page).to have_link "Verify my account"
+      render_inline Account::VerifyAccountComponent.new(account)
+
+      expect(page).to have_content "To perform all the actions verify your account."
+      expect(page).to have_link "Verify my account"
+    end
+
+    it "shows a link to complete verification to level two verified users" do
+      account = User.new(level_two_verified_at: Time.current)
+
+      render_inline Account::VerifyAccountComponent.new(account)
+
+      expect(page).to have_content "To perform all the actions verify your account."
+      expect(page).to have_link "Complete verification"
+    end
+
+    it "shows information about a verified account to level three verified users" do
+      account = User.new(verified_at: Time.current)
+
+      render_inline Account::VerifyAccountComponent.new(account)
+
+      expect(page).not_to have_content "To perform all the actions verify your account."
+      expect(page).to have_content "Account verified"
+    end
+
+    it "does not show verification info to organizations" do
+      account = User.new(organization: Organization.new)
+
+      render_inline Account::VerifyAccountComponent.new(account)
+
+      expect(page).not_to be_rendered
+    end
   end
 
-  it "shows a link to complete verification to level two verified users" do
-    account = User.new(level_two_verified_at: Time.current)
+  context "verification is disabled" do
+    before { Setting["feature.user.skip_verification"] = true }
 
-    render_inline Account::VerifyAccountComponent.new(account)
+    it "does not show verification info to anyone" do
+      account = User.new
 
-    expect(page).to have_content "To perform all the actions verify your account."
-    expect(page).to have_link "Complete verification"
-  end
+      render_inline Account::VerifyAccountComponent.new(account)
 
-  it "shows information about a verified account to level three verified users" do
-    account = User.new(verified_at: Time.current)
-
-    render_inline Account::VerifyAccountComponent.new(account)
-
-    expect(page).not_to have_content "To perform all the actions verify your account."
-    expect(page).to have_content "Account verified"
-  end
-
-  it "does not show verification info to organizations" do
-    account = User.new(organization: Organization.new)
-
-    render_inline Account::VerifyAccountComponent.new(account)
-
-    expect(page).not_to be_rendered
+      expect(page).not_to be_rendered
+    end
   end
 end

From 3938f4ff95bd7a4b3f703a7b0cf6d7262f1e0caf Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Thu, 11 Aug 2022 20:26:36 +0200
Subject: [PATCH 4/6] Render account verified icon using CSS

As mentioned in commit 925f04e3f, icon classes make screen readers
announce strange symbols and aren't properly displayed for people who
have changed their preferred font family.
---
 app/assets/stylesheets/account/verify-account.scss       | 7 +++----
 app/components/account/verify_account_component.html.erb | 1 -
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/app/assets/stylesheets/account/verify-account.scss b/app/assets/stylesheets/account/verify-account.scss
index 990e7381d..79b56e6ef 100644
--- a/app/assets/stylesheets/account/verify-account.scss
+++ b/app/assets/stylesheets/account/verify-account.scss
@@ -2,14 +2,13 @@
   padding-right: $line-height / 2;
 
   .already-verified {
+    @include has-fa-icon(check, solid);
     color: $check;
     line-height: $line-height * 2;
 
-    .icon-check {
+    &::before {
       color: $text-medium;
-      font-size: rem-calc(24);
-      line-height: rem-calc(45);
-      vertical-align: middle;
+      font-size: 1.4em;
     }
   }
 }
diff --git a/app/components/account/verify_account_component.html.erb b/app/components/account/verify_account_component.html.erb
index bcd6c1880..af9538c35 100644
--- a/app/components/account/verify_account_component.html.erb
+++ b/app/components/account/verify_account_component.html.erb
@@ -1,7 +1,6 @@
 <div class="verify-account">
   <% if account.level_three_verified? %>
     <p class="already-verified">
-      <span class="icon-check"></span>
       <%= t("account.show.verified_account") %>
     </p>
   <% else %>

From 6cc2003e1f2fffe3382aacdceabf5cf943846a6e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Thu, 11 Aug 2022 20:31:18 +0200
Subject: [PATCH 5/6] Fix contrast in account verified text

The contrast value was 1.75, which makes the text hard to read and it
isn't even near to the minimum accessibility requirements.

We're using the `$color-success` variable since the `$check` color is
green and this one is green too.
---
 app/assets/stylesheets/account/verify-account.scss | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/assets/stylesheets/account/verify-account.scss b/app/assets/stylesheets/account/verify-account.scss
index 79b56e6ef..c95b27a38 100644
--- a/app/assets/stylesheets/account/verify-account.scss
+++ b/app/assets/stylesheets/account/verify-account.scss
@@ -3,11 +3,11 @@
 
   .already-verified {
     @include has-fa-icon(check, solid);
-    color: $check;
+    color: $color-success;
     line-height: $line-height * 2;
 
     &::before {
-      color: $text-medium;
+      color: $check;
       font-size: 1.4em;
     }
   }

From f1387092e8e58270ecbceb724b8e5c23539bbe9c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Thu, 11 Aug 2022 22:22:50 +0200
Subject: [PATCH 6/6] Don't use line-height to set verified text margin

Using line-height is confusing and has unexpected results when texts
span over multiple lines, as might be the case in some language and
screen resolution combinations.
---
 app/assets/stylesheets/account/verify-account.scss | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/assets/stylesheets/account/verify-account.scss b/app/assets/stylesheets/account/verify-account.scss
index c95b27a38..2ba30afe7 100644
--- a/app/assets/stylesheets/account/verify-account.scss
+++ b/app/assets/stylesheets/account/verify-account.scss
@@ -4,7 +4,7 @@
   .already-verified {
     @include has-fa-icon(check, solid);
     color: $color-success;
-    line-height: $line-height * 2;
+    margin-top: $line-height;
 
     &::before {
       color: $check;