From c3322be9ce3b49643e3d938c0243665d7a2961fd Mon Sep 17 00:00:00 2001
From: Andy Zhao <andyzhao.zhao@gmail.com>
Date: Thu, 11 Oct 2018 13:31:37 -0400
Subject: [PATCH] Improve Sign In/Up Error Handling (#832)

* Add registration view to sign_in page

* Add better sad path for failed sign ins/ups

* Ensure email is downcased for authorization

* Remove .downcase for emails

* Use partial for styles
---
 .../omniauth_callbacks_controller.rb          |  4 +-
 .../registrations/_registration_form.html.erb |  2 +-
 app/views/devise/registrations/new.html.erb   |  4 +-
 app/views/devise/sessions/new.html.erb        |  7 +-
 .../shared/_authorization_error.html.erb      | 25 +++++++
 .../feedback_messages/_style.html.erb         | 67 ++++++++++++++++++
 .../internal/feedback_messages/index.html.erb | 68 +------------------
 .../internal/feedback_messages/show.html.erb  |  2 +
 app/views/layouts/_nav_menu.html.erb          |  2 +-
 9 files changed, 106 insertions(+), 75 deletions(-)
 create mode 100644 app/views/devise/shared/_authorization_error.html.erb
 create mode 100644 app/views/internal/feedback_messages/_style.html.erb

diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb
index a7e4d4bdf..e17309514 100644
--- a/app/controllers/omniauth_callbacks_controller.rb
+++ b/app/controllers/omniauth_callbacks_controller.rb
@@ -11,10 +11,12 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
           remember_me(@user)
           sign_in_and_redirect @user, event: :authentication
           set_flash_message(:notice, :success, kind: "#{provider}".capitalize) if is_navigational_format?
-        elsif @user.persisted? && @user.errors&.full_messages&.join.include?("username has already been taken")
+        elsif @user.persisted? && @user.errors.full_messages.join(", ").include?("username has already been taken")
           redirect_to "/settings?state=previous-registration"
         else
           session["devise.#{provider}_data"] = request.env["omniauth.auth"]
+          user_errors = @user.errors.full_messages
+          flash[:alert] = user_errors
           redirect_to new_user_registration_url
         end
       end
diff --git a/app/views/devise/registrations/_registration_form.html.erb b/app/views/devise/registrations/_registration_form.html.erb
index a863c105f..583c29864 100644
--- a/app/views/devise/registrations/_registration_form.html.erb
+++ b/app/views/devise/registrations/_registration_form.html.erb
@@ -10,4 +10,4 @@
   </div>
   <p>The DEV Community is for amazing humans who code</p>
   <div style="height:200px"/>
-</div>
\ No newline at end of file
+</div>
diff --git a/app/views/devise/registrations/new.html.erb b/app/views/devise/registrations/new.html.erb
index 53ffdad27..a80f70703 100644
--- a/app/views/devise/registrations/new.html.erb
+++ b/app/views/devise/registrations/new.html.erb
@@ -1 +1,3 @@
-<%= render "registration_form" %>
\ No newline at end of file
+<%= render "devise/shared/authorization_error" %>
+
+<%= render "devise/registrations/registration_form" %>
diff --git a/app/views/devise/sessions/new.html.erb b/app/views/devise/sessions/new.html.erb
index 6c0ac5558..a80f70703 100644
--- a/app/views/devise/sessions/new.html.erb
+++ b/app/views/devise/sessions/new.html.erb
@@ -1,4 +1,3 @@
-<div class="devise-container">
-  <a href="/users/auth/twitter" class="sign-up-link" data-no-instant><%= icon("twitter") %> Twitter</a>
-  <a href="/users/auth/github" class="sign-up-link" data-no-instant><%= icon("github") %> Github</a>
-</div>
+<%= render "devise/shared/authorization_error" %>
+
+<%= render "devise/registrations/registration_form" %>
diff --git a/app/views/devise/shared/_authorization_error.html.erb b/app/views/devise/shared/_authorization_error.html.erb
new file mode 100644
index 000000000..98a58f6ac
--- /dev/null
+++ b/app/views/devise/shared/_authorization_error.html.erb
@@ -0,0 +1,25 @@
+<style>
+  .error-notice{
+    background: rgba(255, 80, 80, 0.7);
+    color: white;
+    padding: 40px 0px 40px;
+    text-align: center;
+    position: relative;
+    top: 60px;
+    left: 0px;
+    right: 0px;
+  }
+</style>
+<% unless flash[:alert].blank? %>
+  <div class="error-notice">
+  Something went wrong:
+    <ul>
+      <% flash[:alert].each do |error| %>
+        <li>
+          <%= error %>
+        </li>
+      <% end %>
+    </ul>
+    Please try again below, or contact <a href="mailto:yo@dev.to">yo@dev.to</a> if this persists.
+  </div>
+<% end %>
diff --git a/app/views/internal/feedback_messages/_style.html.erb b/app/views/internal/feedback_messages/_style.html.erb
new file mode 100644
index 000000000..d622303ef
--- /dev/null
+++ b/app/views/internal/feedback_messages/_style.html.erb
@@ -0,0 +1,67 @@
+<style>
+  .top-nav{
+    line-height: 1.4em;
+  }
+  @media (max-width:991px) and (min-width:768px) {
+    .top-nav{
+      margin-top: 100px;
+    }
+    .alert{
+      margin-top: 100px;
+    }
+  }
+  .top-nav a {
+    display: inline-block;
+    padding: 10px;
+  }
+  .active-state {
+    background-color:#89ffba;
+  }
+  .notefield{
+  width: 100%;
+  resize: none;
+  font-size: 18px;
+  height: 50px;
+  border-radius: 3px;
+  padding: 5px;
+  margin: 10px 0px;
+  }
+  .blankmessage{
+    font-style: italic;
+  }
+  .note-content{
+    border: 1px dashed black;
+    border-radius: 5px;
+    width: 100%;
+    padding: 5px;
+    padding-top: 25px;
+    word-wrap: break-word;
+  }
+  .email__alert{
+    display: none;
+    margin: 0;
+  }
+  .panel-right-elements{
+    float: right;
+  }
+  .email__container{
+    border: 1px solid gray;
+    margin: 10px;
+    padding-left: 50px;
+    padding-top: 5px;
+  }
+  .to__subject{
+    margin: 5px;
+    font-size: 20px;
+    color: black;
+    text-align: center;
+  }
+  .time__badge{
+    float: right;
+    margin-top: 7px;
+    margin-right: 7px;
+  }
+  .single__note{
+    width: 60%;
+  }
+</style>
\ No newline at end of file
diff --git a/app/views/internal/feedback_messages/index.html.erb b/app/views/internal/feedback_messages/index.html.erb
index 92404e6ca..5bb6d99e6 100644
--- a/app/views/internal/feedback_messages/index.html.erb
+++ b/app/views/internal/feedback_messages/index.html.erb
@@ -1,70 +1,4 @@
-<style>
-  .top-nav{
-    line-height: 1.4em;
-  }
-  @media (max-width:991px) and (min-width:768px) {
-    .top-nav{
-      margin-top: 100px;
-    }
-    .alert{
-      margin-top: 100px;
-    }
-  }
-  .top-nav a {
-    display: inline-block;
-    padding: 10px;
-  }
-  .active-state {
-    background-color:#89ffba;
-  }
-  .notefield{
-  width: 100%;
-  resize: none;
-  font-size: 18px;
-  height: 50px;
-  border-radius: 3px;
-  padding: 5px;
-  margin: 10px 0px;
-  }
-  .blankmessage{
-    font-style: italic;
-  }
-  .note-content{
-    border: 1px dashed black;
-    border-radius: 5px;
-    width: 100%;
-    padding: 5px;
-    padding-top: 25px;
-    word-wrap: break-word;
-  }
-  .email__alert{
-    display: none;
-    margin: 0;
-  }
-  .panel-right-elements{
-    float: right;
-  }
-  .email__container{
-    border: 1px solid gray;
-    margin: 10px;
-    padding-left: 50px;
-    padding-top: 5px;
-  }
-  .to__subject{
-    margin: 5px;
-    font-size: 20px;
-    color: black;
-    text-align: center;
-  }
-  .time__badge{
-    float: right;
-    margin-top: 7px;
-    margin-right: 7px;
-  }
-  .single__note{
-    width: 60%;
-  }
-</style>
+<%= render "style" %>
 
 <h3 class="top-nav">
   <a href="/internal/reports?state=abuse-reports&status=Open" class="<%= "active-state" if @feedback_type == "abuse-reports" %>">Abuse Reports</a> |
diff --git a/app/views/internal/feedback_messages/show.html.erb b/app/views/internal/feedback_messages/show.html.erb
index d186cbc4e..897aa3910 100644
--- a/app/views/internal/feedback_messages/show.html.erb
+++ b/app/views/internal/feedback_messages/show.html.erb
@@ -1,3 +1,5 @@
+<%= render "style" %>
+
 <div class="blank_space" style="height: 50px;">
 </div>
 <div class="blank_space">
diff --git a/app/views/layouts/_nav_menu.html.erb b/app/views/layouts/_nav_menu.html.erb
index 253e6e116..6936761a3 100644
--- a/app/views/layouts/_nav_menu.html.erb
+++ b/app/views/layouts/_nav_menu.html.erb
@@ -50,7 +50,7 @@
     </a>
     <a href="/users/auth/github?state=navbar_basic" data-no-instant>
       <div class="option">
-        Via Github
+        Via GitHub
       </div>
     </a>
     <a href="/p/information">