Fix existing user names in partner invites by w3lld1 · Pull Request #5640 · rubyforgood/human-essentials

w3lld1 · 2026-07-24T21:27:07Z

Resolves #5612

Description

When a bank user leaves the email field, the form now checks whether that account already exists and explains how the submitted name will be handled. Existing accounts keep their current name, while an account without a name receives the submitted one when the partner role is added.

The lookup and invitation paths also match email addresses case-insensitively, consistent with the user email uniqueness rule.

Type of change

Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

bundle exec rspec spec/services/user_invite_service_spec.rb spec/requests/partner_users_requests_spec.rb (30 examples)
Focused JavaScript system scenarios for existing users with and without names (2 examples)
bin/lint (RuboCop and ERB lint)
bundle exec brakeman --no-pager --quiet (0 warnings)
node --check app/javascript/controllers/existing_user_controller.js
git diff --check origin/main...HEAD

The full RSpec suite is deferred to CI.

Copilot

Pull request overview

This PR improves the partner user invite flow by detecting existing users when an email is entered, clarifying how the submitted name will be applied, and aligning user lookup/invitation behavior with case-insensitive email matching.

Changes:

Add a partner-users lookup endpoint plus a Stimulus controller to check for existing users on email-field blur and show guidance in the invite form.
Update UserInviteService to find existing users case-insensitively and to apply the submitted name only when the existing user has no name.
Add request/service/system specs covering the lookup behavior, case-insensitive matching, and name handling for existing users.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
`spec/system/partner_system_spec.rb`	Adds JS system coverage for the “existing user detected on blur” UX.
`spec/services/user_invite_service_spec.rb`	Adds coverage for case-insensitive lookup and name-setting behavior for existing users.
`spec/requests/partner_users_requests_spec.rb`	Adds request coverage for the new `GET #lookup` endpoint.
`config/routes.rb`	Adds `GET /partners/:partner_id/users/lookup` collection route.
`app/views/partner_users/_form.html.erb`	Wires up the Stimulus controller/targets and adds an inline message live region.
`app/services/user_invite_service.rb`	Implements case-insensitive lookup and conditional name update for existing users.
`app/javascript/controllers/existing_user_controller.js`	New Stimulus controller to call lookup endpoint and update the form message.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

 class PartnerUsersController < ApplicationController
  before_action :authorize_admin
-  before_action :set_partner, only: %i[index create destroy resend_invitation]
+  before_action :set_partner, only: %i[index lookup create destroy resend_invitation]


+  def lookup
+    user = User.find_by("LOWER(email) = ?", params[:email].to_s.downcase)
+    render json: {exists: user.present?}
+  end


+              action: "blur->existing-user#lookup"
+            }
+          } %>
+        <small class="form-text text-muted" data-existing-user-target="message" aria-live="polite"></small>


    end

-    user = User.find_by(email: email)
+    user = User.find_by("LOWER(email) = ?", email.to_s.downcase)


+    it "should find an existing user case-insensitively" do
+      uppercase_email = user.email.upcase
+      target_partner = partner
+      expect(User.find_by("LOWER(email) = ?", uppercase_email.downcase)).to eq(user)
+
+      expect {
+        described_class.invite(
+          email: uppercase_email,
+          roles: [Role::PARTNER],
+          resource: target_partner
+        )
+      }.not_to change(User, :count)
+
+      expect(user.reload).to have_role(Role::PARTNER, target_partner)
+    end


dorner · 2026-07-31T19:09:45Z

+      const response = await fetch(`${this.lookupUrlValue}?email=${encodeURIComponent(email)}`, {
+        headers: { "Accept": "application/json" }
+      })
+      if (!response.ok || this.emailTarget.value.trim() !== email) return


This is for debouncing purposes? Wouldn't it be better to cancel the existing fetch if we detect that the name has changed when a fetch is in progress?

Do we still need this check now that we're aborting?

dorner · 2026-07-31T19:10:34Z

+        this.resetNameField()
+      } else {
+        this.nameTarget.disabled = false
+        this.messageTarget.textContent = "This user already exists. The submitted name will only be used if their profile has no name yet."


Probably need more information here... does this imply that we can't change the user's name once it's set? Or we have to change it somewhere else? (In which case, where?)

Looks like the issue didn't specify what should happen in this case... @ruestitch do we want to prevent name changes, or allow name changes that actually work?

dorner · 2026-07-31T19:11:01Z

+
+  resetNameField() {
+    this.nameTarget.disabled = false
+    this.messageTarget.textContent = ""


Do we want to clear out the name field when an error happens? Why?

dorner · 2026-07-31T19:12:35Z

+          input_html: {
+            data: {
+              existing_user_target: "email",
+              action: "blur->existing-user#lookup"


Should this be on blur or on change?

dorner · 2026-07-31T19:12:59Z

+      )
+
+      expect(response).to have_http_status(:ok)
+      expect(response.parsed_body).to eq("exists" => true)


No test for the false path.

w3lld1 · 2026-08-01T19:31:22Z

Thanks — addressed the review in f74d5649:

an in-flight lookup is now aborted when the email changes or the controller disconnects;
lookup errors no longer reset form state;
the message now explicitly says an existing profile name is retained and the submitted name is used only when no profile name exists;
added request coverage for the exists: false path.

I kept lookup on blur so tabbing/clicking away performs one request after email entry rather than firing during each edit. node --check and git diff --check pass. The focused request spec could not run here because Ruby/Bundler is unavailable.

dorner

The Copilot comments also still need to be addressed.

dorner · 2026-08-07T19:52:36Z

      } else {
        this.nameTarget.disabled = false
-        this.messageTarget.textContent = "This user already exists. The submitted name will only be used if their profile has no name yet."
+        this.messageTarget.textContent = "This user already exists. Their current profile name will be kept; the submitted name is used only if their profile has no name yet."


Shouldn't we know whether the user has a name or not? Can we add this to the lookup endpoint?

dorner · 2026-08-07T19:53:04Z

+      const response = await fetch(`${this.lookupUrlValue}?email=${encodeURIComponent(email)}`, {
+        headers: { "Accept": "application/json" }
+      })
+      if (!response.ok || this.emailTarget.value.trim() !== email) return


Do we still need this check now that we're aborting?

dorner · 2026-08-07T19:53:33Z

Tests are failing as well.

w3lld1 · 2026-08-07T20:36:41Z

Thanks — addressed the new review in ed93353ba:

the lookup response now reports whether the existing user has a profile name, and the form shows the matching message;
partner lookup is scoped to the current organization;
the live message has an explicit status role;
request and system expectations cover both named and unnamed existing users, including the failing CI assertions.

I kept the email equality guard because changing the field after a blur does not start a new lookup until the next blur; the guard prevents that already-started response from updating the form with stale data.

Validation: node --check app/javascript/controllers/existing_user_controller.js and git diff HEAD^ --check. I could not rerun the focused Ruby specs here because bundle is unavailable in this environment; CI is running on the pushed head.

fix: handle existing names in partner invites

cf4a8be

dorner requested a review from Copilot July 31, 2026 19:06

Copilot started reviewing on behalf of dorner July 31, 2026 19:07 View session

Copilot AI reviewed Jul 31, 2026

View reviewed changes

dorner requested changes Jul 31, 2026

View reviewed changes

fix: address existing-user lookup review

f74d564

dorner requested changes Aug 7, 2026

View reviewed changes

fix: clarify existing user name handling

ed93353

Uh oh!

Uh oh!

Conversation

w3lld1 commented Jul 24, 2026

Description

Type of change

How Has This Been Tested?

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

w3lld1 commented Aug 1, 2026

Uh oh!

dorner left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dorner commented Aug 7, 2026

Uh oh!

w3lld1 commented Aug 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants