Auto generate profile image for email signups (#13635)

* Add required tooltips and asterisks to fields * Generate profile image for email signups * Use be over eq Co-authored-by: Michael Kohl <citizen428@dev.to> * Use path helpers over hard-coded paths Co-authored-by: Michael Kohl <citizen428@dev.to> * Add labels to make it more accessible * Stub it all... eek * stub some more stuff * stub stub stub * Move aria label to proper label and fix tests Co-authored-by: Michael Kohl <citizen428@dev.to>
2021-05-05 12:08:24 -04:00 · 2021-05-05 12:08:24 -04:00 · 9970b1c2d5
commit 9970b1c2d5
parent d1bc35d0d2
5 changed files with 64 additions and 18 deletions
--- a/app/controllers/registrations_controller.rb
+++ b/app/controllers/registrations_controller.rb
@ -25,6 +25,7 @@ class RegistrationsController < Devise::RegistrationsController
      resource.registered = true
      resource.registered_at = Time.current
      resource.editor_version = "v2"
+      resource.remote_profile_image_url = Users::ProfileImageGenerator.call if resource.remote_profile_image_url.blank?
      check_allowed_email(resource) if resource.email.present?
      resource.save if resource.email.present?
      yield resource if block_given?
--- a/app/views/shared/authentication/_email_registration_form.html.erb
+++ b/app/views/shared/authentication/_email_registration_form.html.erb
@ -48,31 +48,46 @@
    <% end %>
    <div class="crayons-field mt-3">
      <%= f.label :profile_image, class: "crayons-field__label" %>
-      <%= f.file_field :profile_image, accept: "image/*", class: "crayons-card crayons-card--secondary p-3 flex items-center flex-1 w-100", required: true %>
+      <%= f.file_field :profile_image, accept: "image/*", class: "crayons-card crayons-card--secondary p-3 flex items-center flex-1 w-100" %>
    </div>

    <div class="crayons-field mt-2">
-      <%= f.label :name, class: "crayons-field__label" %>
+      <%= f.label :name, class: "crayons-field__label", aria: { label: "Name" } do %>
+        Name
+        <span class="crayons-field__required crayons-tooltip" data-tooltip="* - required">*</span>
+      <% end %>
      <%= f.text_field :name, class: "crayons-textfield", required: true %>
    </div>

    <div class="crayons-field mt-2">
-      <%= f.label :username, class: "crayons-field__label" %>
+      <%= f.label :username, class: "crayons-field__label", aria: { label: "Username" } do %>
+        Username
+        <span class="crayons-field__required crayons-tooltip" data-tooltip="* - required">*</span>
+      <% end %>
      <%= f.text_field :username, class: "crayons-textfield", required: true %>
    </div>

    <div class="crayons-field mt-2">
-      <%= f.label :email, class: "crayons-field__label" %>
+      <%= f.label :email, class: "crayons-field__label", aria: { label: "Email" } do %>
+        Email
+        <span class="crayons-field__required crayons-tooltip" data-tooltip="* - required">*</span>
+      <% end %>
      <%= f.email_field :email, autocomplete: "email", class: "crayons-textfield", required: true %>
    </div>

    <div class="crayons-field mt-2">
-      <%= f.label :password, class: "crayons-field__label" %>
+      <%= f.label :password, class: "crayons-field__label", aria: { label: "Password" } do %>
+        Password
+        <span class="crayons-field__required crayons-tooltip" data-tooltip="* - required">*</span>
+      <% end %>
      <%= f.password_field :password, autocomplete: "current-password", class: "crayons-textfield", required: true %>
    </div>

    <div class="crayons-field mt-2">
-      <%= f.label :password_confirmation, class: "crayons-field__label" %>
+      <%= f.label :password_confirmation, class: "crayons-field__label", aria: { label: "Password Confirmation" } do %>
+        Password Confirmation
+        <span class="crayons-field__required crayons-tooltip" data-tooltip="* - required">*</span>
+      <% end %>
      <%= f.password_field :password_confirmation, autocomplete: "current-password", class: "crayons-textfield", required: true %>
    </div>

--- a/cypress/integration/registrationFlows/emailRegistration.spec.js
+++ b/cypress/integration/registrationFlows/emailRegistration.spec.js
@ -9,19 +9,19 @@ describe('Sign up with email', () => {
    cy.findByRole('link', { name: /Sign up with Email/ }).click();

    cy.findByLabelText('Profile image').attachFile('images/admin-image.png');
-    cy.findByLabelText('Name').type('Sloan');
-    cy.findByLabelText('Username').type('s');
-    cy.findByLabelText('Email').type('sloan@example.com');
-    cy.findByLabelText('Password').type('password');
-    cy.findByLabelText('Password confirmation').type('password');
+    cy.findByLabelText('Name *').type('Sloan');
+    cy.findByLabelText('Username *').type('s');
+    cy.findByLabelText('Email *').type('sloan@example.com');
+    cy.findByLabelText('Password *').type('password');
+    cy.findByLabelText('Password Confirmation *').type('password');
    cy.findByRole('button', { name: 'Sign up' }).click();

    // The validation failed but the user's data is not lost
    cy.get('li')
      .contains('Username is too short (minimum is 2 characters)')
      .should('be.visible');
-    cy.findByLabelText('Name').should('have.value', 'Sloan');
-    cy.findByLabelText('Username').should('have.value', 's');
-    cy.findByLabelText('Email').should('have.value', 'sloan@example.com');
+    cy.findByLabelText('Name *').should('have.value', 'Sloan');
+    cy.findByLabelText('Username *').should('have.value', 's');
+    cy.findByLabelText('Email *').should('have.value', 'sloan@example.com');
  });
 });
--- a/spec/requests/registrations_spec.rb
+++ b/spec/requests/registrations_spec.rb
@ -62,7 +62,11 @@ RSpec.describe "Registrations", type: :request do

  describe "Create Account" do
    context "when email registration allowed" do
-      before { allow(Settings::Authentication).to receive(:allow_email_password_registration).and_return(true) }
+      before do
+        allow(Settings::Authentication).to receive(:allow_email_password_registration).and_return(true)
+        # rubocop:disable RSpec/AnyInstance
+        allow_any_instance_of(ProfileImageUploader).to receive(:download!)
+      end

      it "shows the sign in page with email option" do
        get sign_up_path, params: { state: "new-user" }
@ -75,6 +79,21 @@ RSpec.describe "Registrations", type: :request do

        expect(response.body).to include("View more sign in options")
      end
+
+      it "creates a user with a random profile image if none was uploaded" do
+        name = "test"
+        post users_path, params: {
+          user: {
+            name: name,
+            username: "username",
+            email: "yo@whatup.com",
+            password: "password",
+            password_confirmation: "password"
+          }
+        }
+
+        expect(User.find_by(name: name).persisted?).to be true
+      end
    end

    context "when email registration not allowed" do
@ -90,6 +109,7 @@ RSpec.describe "Registrations", type: :request do

    context "when email registration allowed and captcha required" do
      before do
+        allow_any_instance_of(ProfileImageUploader).to receive(:download!)
        allow(Settings::Authentication).to receive(:recaptcha_secret_key).and_return("someSecretKey")
        allow(Settings::Authentication).to receive(:recaptcha_site_key).and_return("someSiteKey")
        allow(Settings::Authentication).to receive(:allow_email_password_registration).and_return(true)
@ -153,7 +173,6 @@ RSpec.describe "Registrations", type: :request do

  describe "POST /users" do
    def mock_recaptcha_verification
-      # rubocop:disable RSpec/AnyInstance
      allow_any_instance_of(RegistrationsController).to(
        receive(:recaptcha_verified?).and_return(true),
      )
@ -173,8 +192,9 @@ RSpec.describe "Registrations", type: :request do

    context "when site is configured to accept email registration" do
      before do
-        allow(Settings::Authentication)
-          .to receive(:allow_email_password_registration).and_return(true)
+        allow(Settings::Authentication).to receive(:allow_email_password_registration).and_return(true)
+        # rubocop:disable RSpec/AnyInstance
+        allow_any_instance_of(ProfileImageUploader).to receive(:download!)
      end

      it "does not raise disallowed if community is set to allow email" do
@ -230,6 +250,7 @@ RSpec.describe "Registrations", type: :request do

    context "when email registration allowed and email allow list empty" do
      before do
+        allow_any_instance_of(ProfileImageUploader).to receive(:download!)
        allow(Settings::Authentication).to receive(:allow_email_password_registration).and_return(true)
        allow(Settings::Authentication).to receive(:allowed_registration_email_domains).and_return([])
      end
@ -247,6 +268,7 @@ RSpec.describe "Registrations", type: :request do

    context "when email registration allowed and email allow list present" do
      before do
+        allow_any_instance_of(ProfileImageUploader).to receive(:download!)
        allow(Settings::Authentication).to receive(:allow_email_password_registration).and_return(true)
        allow(Settings::Authentication).to receive(:allowed_registration_email_domains).and_return(["dev.to",
                                                                                                    "forem.com"])
@ -275,6 +297,7 @@ RSpec.describe "Registrations", type: :request do

    context "when site configured to accept email registration AND require captcha" do
      before do
+        allow_any_instance_of(ProfileImageUploader).to receive(:download!)
        allow(Settings::Authentication).to receive(:recaptcha_secret_key).and_return("someSecretKey")
        allow(Settings::Authentication).to receive(:recaptcha_site_key).and_return("someSiteKey")
        allow(Settings::Authentication).to receive(:allow_email_password_registration).and_return(true)
@ -309,6 +332,7 @@ RSpec.describe "Registrations", type: :request do

    context "when site is in waiting_on_first_user state" do
      before do
+        allow_any_instance_of(ProfileImageUploader).to receive(:download!)
        allow(SiteConfig).to receive(:waiting_on_first_user).and_return(true)
        ENV["FOREM_OWNER_SECRET"] = nil
      end
@ -388,7 +412,9 @@ RSpec.describe "Registrations", type: :request do

    context "with the creator_onboarding feature flag" do
      before do
+        allow_any_instance_of(ProfileImageUploader).to receive(:download!)
        allow(FeatureFlag).to receive(:enabled?).with(:creator_onboarding).and_return(true)
+        allow(FeatureFlag).to receive(:enabled?).with(:runtime_banner).and_return(false)
        allow(SiteConfig).to receive(:waiting_on_first_user).and_return(true)
      end

@ -417,3 +443,4 @@ RSpec.describe "Registrations", type: :request do
    end
  end
 end
+# rubocop:enable RSpec/AnyInstance
--- a/spec/system/authentication/user_logs_in_with_email_spec.rb
+++ b/spec/system/authentication/user_logs_in_with_email_spec.rb
@ -7,6 +7,9 @@ RSpec.describe "Authenticating with Email" do
  before do
    allow(Settings::Authentication).to receive(:allow_email_password_registration).and_return(true)
    allow(Settings::Authentication).to receive(:allow_email_password_login).and_return(true)
+    # rubocop:disable RSpec/AnyInstance
+    allow_any_instance_of(ProfileImageUploader).to receive(:download!)
+    # rubocop:enable RSpec/AnyInstance
  end

  context "when a user is new" do