[3260] Add feature to manage partner users for a Partner as a Organization/Bank #3266

edwinthinks · 2022-11-23T20:44:21Z

Resolves #3260

Description

This PR adds a page to manage the users for a partner as an organization. This uses the roles!

A few additional things to change in an upcoming PR:

Modify the user management flow to use this code as a partner. Right now there is a Partner::UsersController which we can deprecate in favor of using common code.
In the update above, I would work in the ability to set the partner users as admins so they have the ability to add/remove and promote/demote users.

Type of change

New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Tested locally. Still need to add specs.

Screenshots

See for quick recording - https://jam.dev/c/9c61fa7a-ba15-415e-8568-aabd3e4171e3

app/controllers/partner_users_controller.rb

dorner · 2023-02-08T00:52:16Z

@edwinthinks pushed the change - this is really just using plain old Rails forms. Everything seems to work fine. Not sure why the revoking logic doesn't work right now but this has to be rebased/merged off main anyway.

Conflicts: Gemfile.lock app/controllers/partners_controller.rb app/views/partners/show.html.erb

awwaiid · 2023-02-24T00:12:41Z

I merged in main and fixed conflicts.

…r-management Conflicts: db/seeds.rb

dorner · 2024-05-24T20:43:44Z

@cielf fixed this up, it should be good to test out!

all good

cielf · 2024-05-24T21:15:42Z

Finally! I'm not going to try to get it in for this weekend -- it's already pretty meaty -- but I will kick the tires.

cielf

Not a complete review -- a couple of phrasing suggestions , then -- I ran into a 500 error...
How I got that:
bin/setup
bin/start
sign in as org_admin1
Partner Agencies | All Partners
choose Pawnee Parent Service
Users
In "invite user" fill in Name: User 1, email [email protected]
(that's the user with org_user status in the seed)
I got an Argument Error "nil can't be converted to a Time value" at line 45 in app/views/partner_users/_users.html.erb

It does send the email -- it's just failing on displaying?

I also want to check with the stakeholder circle on whether org_user level folk should have access to this . I know that's what it says on the issue -- but I'm not sure the question was asked.

cielf · 2024-05-25T17:46:17Z

spec/requests/partner_users_requests_spec.rb

+      }.to change { partner_user.roles.count }.from(1).to(0)
+
+      expect(response).to redirect_to(root_path)
+      expect(flash[:notice]).to eq("Access to #{partner_user.name} has been revoked.")


Wording change, This should be more like "#{partner_user.name}'s access has been revoked."

That form handles 99% of the cases for possessives, and may be "good enough". There is a possessive gem -- it's fairly old, but the rules don't change [I haven't looked into it in detail]

Alternatively, we could say "Access to # {partner.name} has been revoked for #{partner_user.name}"

However -- We should be using the display name in the same manner as we have in the superadmin interface (displaying the email if the name is not given) -- There are a lot of partners whose names we do not have.

cielf · 2024-05-25T18:01:12Z

app/views/partners/show.html.erb

@@ -34,6 +34,12 @@
            <h2 class="card-title">Partner Actions</h2>
          </div>
          <div class="card-body p-3">
+            <%= link_to partner_users_path(@partner) do %>
+              <div class="btn btn-app bg-success">
+                <i class="fas fa-users"></i> Users


Please change this to "Manage Users" to follow the same form for the other actions on the page.

cielf · 2024-06-05T23:10:34Z

@dorner From tonight's stakeholder meeting -- can we please limit access to this to org_admins ? Thanks.

# Conflicts: # config/locales/en.yml

dorner · 2024-06-07T19:58:35Z

The bug you found was because you were inviting a user that already existed in the system - and that user doesn't have an invitation_sent_at. Pretty sure this is a non-case in production.

dorner · 2024-06-07T20:02:02Z

Addressed other feedback!

cielf · 2024-06-07T20:54:16Z

The bug you found was because you were inviting a user that already existed in the system - and that user doesn't have an invitation_sent_at. Pretty sure this is a non-case in production.

Probably -- we should address that in the seed (I'm making a note for a new issue around that), and I'll test the appropriate case with a newly invited user.

Looks good to me now!

cielf · 2024-06-08T19:06:47Z

@awwaiid Can you take a look -- mostly as a formality -- @dorner took this over to get it to the finish line.

cielf

@dorner The manual testing looks pretty good, but it looks like you need to tweak the tests to match.

dorner · 2024-06-14T20:29:03Z

@cielf fixed!

cielf · 2024-06-15T01:15:29Z

The status on this is waiting for @awwaiid to take a look at it then.

Tests got fixed.

awwaiid · 2024-06-19T14:42:32Z

app/controllers/partners_controller.rb

@@ -129,18 +129,6 @@ def invite
    end
  end

-  def invite_partner_user


This is still referenced from config/routes.rb, but nothing submits to it, so not an immediate blocker

I created #4459 to address

awwaiid · 2024-06-19T14:57:10Z

app/controllers/partner_users_controller.rb

+# frozen_String_literal: true
+
+class PartnerUsersController < ApplicationController
+  before_action :set_partner, only: %i[index create destroy resend_invitation]


We only link to this controller from bank admins, but we don't here restrict access to only admins. I'll open a separate ticket to address this minor security flaw.

Created #4458 to address

github-actions · 2024-06-23T14:16:17Z

@edwinthinks: Your PR [3260] Add feature to manage partner users for a Partner as a Organization/Bank is part of today's Human Essentials production release: 2024.06.23.
Thank you very much for your contribution!

edwinthinks added 9 commits November 21, 2022 18:46

WIP

fa7267f

WIP

2bf70e7

Merge branch 'main' into 3260/bank-partner-user-management

7543691

Update with button

76d3559

WIP

c70ed81

WIP

8a97f8c

Update

59705a2

Updated

3ba9e15

Add and remove access to partner account for users

90d7440

edwinthinks added the WIP label Nov 23, 2022

edwinthinks added 7 commits November 23, 2022 16:04

WIP

c7c4718

WIP

85b505c

WIP

f2799fc

Re-styling

2428e22

WIP

7d8bbad

Remove older method for inviting and reseting password for partner

530ef2d

Fix rubocop complaints

1b7945d

dorner reviewed Nov 27, 2022

View reviewed changes

app/controllers/partner_users_controller.rb Outdated Show resolved Hide resolved

dorner reviewed Nov 27, 2022

View reviewed changes

app/controllers/partner_users_controller.rb Outdated Show resolved Hide resolved

edwinthinks and others added 6 commits December 18, 2022 09:11

Merge branch 'main' into 3260/bank-partner-user-management

eb0c08e

Merge branch 'main' into 3260/bank-partner-user-management

98d7183

Update styling on table

a77c123

Update with partner merge changes

cc60914

Use basic flow instead of turbo

4267d36

fix form errors

49eeb4f

Merge branch 'main' into 3260/bank-partner-user-management

d43f14d

Conflicts: Gemfile.lock app/controllers/partners_controller.rb app/views/partners/show.html.erb

awwaiid added 2 commits April 2, 2023 11:37

Merge remote-tracking branch 'origin/main' into 3260/bank-partner-use…

f58b941

…r-management Conflicts: db/seeds.rb

Satiate Rubocop! [#3260]

846db60

dorner added 2 commits May 24, 2024 16:23

Lint + spec fix

b9bdf96

Fix

59cc42a

dorner requested review from cielf and removed request for dorner May 24, 2024 20:43

cielf previously requested changes May 25, 2024

View reviewed changes

Merge branch 'main' into 3260/bank-partner-user-management

e330716

# Conflicts: # config/locales/en.yml

CR fixes

49d936c

dorner requested a review from cielf June 7, 2024 20:02

cielf requested a review from awwaiid June 8, 2024 19:05

cielf previously requested changes Jun 8, 2024

View reviewed changes

Fix spec

4128e4b

awwaiid reviewed Jun 19, 2024

View reviewed changes

awwaiid approved these changes Jun 19, 2024

View reviewed changes

awwaiid merged commit c03df01 into main Jun 19, 2024
38 checks passed

awwaiid deleted the 3260/bank-partner-user-management branch June 19, 2024 15:04

cielf mentioned this pull request Sep 30, 2024

Some users in the seed do not have an 'invited at' . That has caused testing issues. Can we fix it? #4689

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[3260] Add feature to manage partner users for a Partner as a Organization/Bank #3266

[3260] Add feature to manage partner users for a Partner as a Organization/Bank #3266

edwinthinks commented Nov 23, 2022 •

edited

Loading

dorner commented Feb 8, 2023

awwaiid commented Feb 24, 2023

dorner commented May 24, 2024

cielf commented May 24, 2024

cielf left a comment •

edited

Loading

cielf May 25, 2024

cielf May 25, 2024

cielf commented Jun 5, 2024

dorner commented Jun 7, 2024

dorner commented Jun 7, 2024

cielf commented Jun 7, 2024

cielf commented Jun 8, 2024

cielf left a comment

dorner commented Jun 14, 2024

cielf commented Jun 15, 2024

awwaiid Jun 19, 2024

awwaiid Jun 19, 2024

awwaiid Jun 19, 2024

awwaiid Jun 19, 2024

github-actions bot commented Jun 23, 2024

[3260] Add feature to manage partner users for a Partner as a Organization/Bank #3266

[3260] Add feature to manage partner users for a Partner as a Organization/Bank #3266

Conversation

edwinthinks commented Nov 23, 2022 • edited Loading

Description

Type of change

How Has This Been Tested?

Screenshots

dorner commented Feb 8, 2023

awwaiid commented Feb 24, 2023

dorner commented May 24, 2024

cielf commented May 24, 2024

cielf left a comment • edited Loading

Choose a reason for hiding this comment

cielf May 25, 2024

Choose a reason for hiding this comment

cielf May 25, 2024

Choose a reason for hiding this comment

cielf commented Jun 5, 2024

dorner commented Jun 7, 2024

dorner commented Jun 7, 2024

cielf commented Jun 7, 2024

cielf commented Jun 8, 2024

cielf left a comment

Choose a reason for hiding this comment

dorner commented Jun 14, 2024

cielf commented Jun 15, 2024

awwaiid Jun 19, 2024

Choose a reason for hiding this comment

awwaiid Jun 19, 2024

Choose a reason for hiding this comment

awwaiid Jun 19, 2024

Choose a reason for hiding this comment

awwaiid Jun 19, 2024

Choose a reason for hiding this comment

github-actions bot commented Jun 23, 2024

edwinthinks commented Nov 23, 2022 •

edited

Loading

cielf left a comment •

edited

Loading