B 21966 add rejected admin int #14694

KonstanceH · 2025-01-30T06:46:15Z

B-21966

Summary

This PR adds the Rejected Office users view on the admin page.
This also adds rejected_on and deleted_on columns to the office users table.

Verification Steps for Reviewers

These are to be checked by a reviewer.

Has the branch been pulled in and checked out?
Have the BL acceptance criteria been met for this change?
Was the CircleCI build successful?
Has the code been reviewed from a standards and best practices point of view?

How to test

Go to Office Users login page.*
Click "Request Account" fill out necessary info and click submit.
Go to Admins Page http://adminlocal:3000/system.
Create new admin user if none available.
Find requested account.
Give it a rejection reason and reject.
Go to "Rejected Users" tab and confirm that you see these fields:
“email, first name, last name, Transportation Office, status, reason for rejection, rejected date, roles requested”
Confirm that you can sort by the header's columns

*Can use RejectedOfficeUser and RequestedOfficeUser testharness to make needed users

Frontend

There are no aXe warnings for UI.
This works in Supported Browsers and their phone views (Chrome, Firefox, Edge).
There are no new console errors in the browser devtools.
There are no new console errors in the test output.
If this PR adds a new component to Storybook, it ensures the component is fully responsive, OR if it is intentionally not, a wrapping div using the officeApp class or custom min-width styling is used to hide any states the would not be visible to the user.
This change meets the standards for Section 508 compliance.

Backend

Code follows the guidelines for Logging.
The requirements listed in Querying the Database Safely have been satisfied.

Database

Any new migrations/schema changes:

Follows our guidelines for Zero-Downtime Deploys.
Have been communicated to #g-database.
Secure migrations have been tested following the instructions in our docs.

Screenshots

Rejected users Page with partial columns

Rejected users Page with remaining columns

…arness and add ui.

robot-mymove · 2025-01-30T06:59:24Z

	Warnings
⚠️	Files located in legacy directories (src/shared or src/scenes) have been edited. Are you sure you don’t want to also relocate them to the new file structure? View the frontend file org ADR for more information

Generated by 🚫 dangerJS against 88a3faf

danieljordan-caci

So I just finished some work for the requested office users and searching by transpo office (which is a requirement for this feature as well I see) and ran into searching nested object values through FetchMany not being possible. Had to refactor.

Could you look at this PR and see how I did the list function and copy that over (except for change the status from REQUESTED to REJECTED)? I just don't want the next dev to have to refactor it when it could be tackled in this one.

Also, I don't think you need the deleted_on column. We are hard deleting users, not soft.

Thoughts? Def welcome to hear them!

JamesHawks224

testing: success
code: looks good, but have comments

Makefile

.circleci/config.yml

.gitlab-ci.yml

KonstanceH · 2025-01-30T16:17:26Z

So I just finished some work for the requested office users and searching by transpo office (which is a requirement for this feature as well I see) and ran into searching nested object values through FetchMany not being possible. Had to refactor.

Could you look at this PR and see how I did the list function and copy that over (except for change the status from REQUESTED to REJECTED)? I just don't want the next dev to have to refactor it when it could be tackled in this one.

Also, I don't think you need the deleted_on column. We are hard deleting users, not soft.

Thoughts? Def welcome to hear them!

Ahh okay I'll get rid of the deleted_on column. But for the transportationOffice search we took that off the feature AC because Paul was gonna add it to a new feature(E-06352) to be handled. I want to say I can just add your changes to my PR here but we had a whole meeting about it so before I say yes let me just get some guidance from my scrum master. I think we can either add a new story to handle it on this feature or just add it on to this story's AC.

danieljordan-caci · 2025-01-30T16:23:21Z

So I just finished some work for the requested office users and searching by transpo office (which is a requirement for this feature as well I see) and ran into searching nested object values through FetchMany not being possible. Had to refactor.
Could you look at this PR and see how I did the list function and copy that over (except for change the status from REQUESTED to REJECTED)? I just don't want the next dev to have to refactor it when it could be tackled in this one.
Also, I don't think you need the deleted_on column. We are hard deleting users, not soft.
Thoughts? Def welcome to hear them!

Ahh okay I'll get rid of the deleted_on column. But for the transportationOffice search we took that off the feature AC because Paul was gonna add it to a new feature(E-06352) to be handled. I want to say I can just add your changes to my PR here but we had a whole meeting about it so before I say yes let me just get some guidance from my scrum master. I think we can either add a new story to handle it on this feature or just add it on to this story's AC.

Hm. Okay. Well still, if that work is coming you can still roll forward with the refactor, just forego all the stuff needed for the transpo office. If that makes sense? Let me know if you're still confused haha

KonstanceH · 2025-01-30T16:26:56Z

So I just finished some work for the requested office users and searching by transpo office (which is a requirement for this feature as well I see) and ran into searching nested object values through FetchMany not being possible. Had to refactor.
Could you look at this PR and see how I did the list function and copy that over (except for change the status from REQUESTED to REJECTED)? I just don't want the next dev to have to refactor it when it could be tackled in this one.
Also, I don't think you need the deleted_on column. We are hard deleting users, not soft.
Thoughts? Def welcome to hear them!

Ahh okay I'll get rid of the deleted_on column. But for the transportationOffice search we took that off the feature AC because Paul was gonna add it to a new feature(E-06352) to be handled. I want to say I can just add your changes to my PR here but we had a whole meeting about it so before I say yes let me just get some guidance from my scrum master. I think we can either add a new story to handle it on this feature or just add it on to this story's AC.

Hm. Okay. Well still, if that work is coming you can still roll forward with the refactor, just forego all the stuff needed for the transpo office. If that makes sense? Let me know if you're still confused haha

Yea I can do that! Sorry I'm in this agile training and I'm terrible at multitasking(trying to understand multiple things at once lol). Once this is over if I have any questions I'll message you in slack.

danieljordan-caci · 2025-01-30T16:28:09Z

Ahh okay I'll get rid of the deleted_on column. But for the transportationOffice search we took that off the feature AC because Paul was gonna add it to a new feature(E-06352) to be handled. I want to say I can just add your changes to my PR here but we had a whole meeting about it so before I say yes let me just get some guidance from my scrum master. I think we can either add a new story to handle it on this feature or just add it on to this story's AC.

Hm. Okay. Well still, if that work is coming you can still roll forward with the refactor, just forego all the stuff needed for the transpo office. If that makes sense? Let me know if you're still confused haha

Yea I can do that! Sorry I'm in this agile training and I'm terrible at multitasking(trying to understand multiple things at once lol). Once this is over if I have any questions I'll message you in slack.

Sounds good! Yeah I'm in here too.. Not the most entertaining material haha

danieljordan-caci · 2025-01-31T17:47:39Z

So I just finished some work for the requested office users and searching by transpo office (which is a requirement for this feature as well I see) and ran into searching nested object values through FetchMany not being possible. Had to refactor.
Could you look at this PR and see how I did the list function and copy that over (except for change the status from REQUESTED to REJECTED)? I just don't want the next dev to have to refactor it when it could be tackled in this one.
Also, I don't think you need the deleted_on column. We are hard deleting users, not soft.
Thoughts? Def welcome to hear them!

Ahh okay I'll get rid of the deleted_on column. But for the transportationOffice search we took that off the feature AC because Paul was gonna add it to a new feature(E-06352) to be handled. I want to say I can just add your changes to my PR here but we had a whole meeting about it so before I say yes let me just get some guidance from my scrum master. I think we can either add a new story to handle it on this feature or just add it on to this story's AC.

Hm. Okay. Well still, if that work is coming you can still roll forward with the refactor, just forego all the stuff needed for the transpo office. If that makes sense? Let me know if you're still confused haha

Yea I can do that! Sorry I'm in this agile training and I'm terrible at multitasking(trying to understand multiple things at once lol). Once this is over if I have any questions I'll message you in slack.

Hey disregard these comments - lot of this work was done in B-21961 and I'm just going to have that roll forward and close my PR. Turns out when it comes to sorting AND filtering nested objects, Pop/Buffalo is not a very flexible choice because we need the DISTINCT keyword which it doesn't support natively. Just FYI - re-requesting review and I'll take a look at this as-is.

KonstanceH · 2025-01-31T20:40:56Z

So I just finished some work for the requested office users and searching by transpo office (which is a requirement for this feature as well I see) and ran into searching nested object values through FetchMany not being possible. Had to refactor.
Could you look at this PR and see how I did the list function and copy that over (except for change the status from REQUESTED to REJECTED)? I just don't want the next dev to have to refactor it when it could be tackled in this one.
Also, I don't think you need the deleted_on column. We are hard deleting users, not soft.
Thoughts? Def welcome to hear them!

Ahh okay I'll get rid of the deleted_on column. But for the transportationOffice search we took that off the feature AC because Paul was gonna add it to a new feature(E-06352) to be handled. I want to say I can just add your changes to my PR here but we had a whole meeting about it so before I say yes let me just get some guidance from my scrum master. I think we can either add a new story to handle it on this feature or just add it on to this story's AC.

Hm. Okay. Well still, if that work is coming you can still roll forward with the refactor, just forego all the stuff needed for the transpo office. If that makes sense? Let me know if you're still confused haha

Yea I can do that! Sorry I'm in this agile training and I'm terrible at multitasking(trying to understand multiple things at once lol). Once this is over if I have any questions I'll message you in slack.

Hey disregard these comments - lot of this work was done in B-21961 and I'm just going to have that roll forward and close my PR. Turns out when it comes to sorting AND filtering nested objects, Pop/Buffalo is not a very flexible choice because we need the DISTINCT keyword which it doesn't support natively. Just FYI - re-requesting review and I'll take a look at this as-is.

oh okay. I'll still pull out that delete column in this next push and then request a rereview.

brooklyn-welsh

Code LGTM. Been testing the changes alone and while working on dependent stories and haven't been able to run into any errors ✅

danieljordan-caci

Couple of things:

the font styling is different for the roles column, this is likely due to you using <p> which has styling overrides in the app, I would try and copy what the requested office user page is doing with react-admin's ArrayField component to match styling
I think this SHOULD be fixed once you make the above change, but I'd like to try and get rid of the side scrolling behavior if possible. It seems the font being bigger/different is causing some sizing differences when it should be similar to the other office user pages

Screen.Recording.2025-02-04.at.9.46.45.AM.mov

Let me know if any of that doesn't make sense. Re-request when you got it all fixed up! Thanks!

danieljordan-caci · 2025-02-04T15:54:44Z

pkg/services/rejected_office_users.go

+// RejectedOfficeUserFetcherPop is the exported interface for fetching a single office user
+//
+//go:generate mockery --name RejectedOfficeUserFetcherPop
+type RejectedOfficeUserFetcherPop interface {
+	FetchRejectedOfficeUserByID(appCtx appcontext.AppContext, id uuid.UUID) (models.OfficeUser, error)
+}


Is this being used? I don't see it being used anywhere in your code - I remember I removed some unused funcs from the requested side so if this is copied from that then I'm 99% sure this function isn't being used.

danieljordan-caci · 2025-02-04T15:55:12Z

pkg/services/rejected_office_users/rejected_office_user_fetcher.go

+// FetchRejectedOfficeUser fetches an office user given a slice of filters
+func (o *rejectedOfficeUserFetcher) FetchRejectedOfficeUser(appCtx appcontext.AppContext, filters []services.QueryFilter) (models.OfficeUser, error) {
+	var rejectedOfficeUser models.OfficeUser
+	err := o.builder.FetchOne(appCtx, &rejectedOfficeUser, filters)
+	return rejectedOfficeUser, err
+}
+
+// NewAdminUserFetcher return an implementation of the AdminUserFetcher interface
+func NewRejectedOfficeUserFetcher(builder rejectedOfficeUserQueryBuilder) services.RejectedOfficeUserFetcher {
+	return &rejectedOfficeUserFetcher{builder}
+}
+
+type rejectedOfficeUserFetcherPop struct {
+}
+
+// FetchOfficeUserByID fetches an office user given an ID
+func (o *rejectedOfficeUserFetcherPop) FetchRejectedOfficeUserByID(appCtx appcontext.AppContext, id uuid.UUID) (models.OfficeUser, error) {
+	var officeUser models.OfficeUser
+	err := appCtx.DB().Eager("TransportationOffice").Find(&officeUser, id)
+	if err != nil {
+		switch err {
+		case sql.ErrNoRows:
+			return models.OfficeUser{}, apperror.NewNotFoundError(id, "looking for OfficeUser")
+		default:
+			return models.OfficeUser{}, apperror.NewQueryError("OfficeUser", err, "")
+		}
+	}
+
+	return officeUser, err
+}
+
+// NewOfficeUserFetcherPop return an implementation of the OfficeUserFetcherPop interface
+func NewRejectedOfficeUserFetcherPop() services.RejectedOfficeUserFetcherPop {
+	return &rejectedOfficeUserFetcherPop{}
+}


If this isn't being used anywhere, then I'd remove

I'm using this for the testharness piece but before I remove all of that would it make sense to keep it in case the users ever want to search one rejected/requested user?

I think if it isn't being used in the code for business logic, then it's just tech debt to me. But.. That's just my opinion.

yea that's definitely fair I'll pull it out

KonstanceH · 2025-02-04T19:46:49Z

Couple of things:

the font styling is different for the roles column, this is likely due to you using <p> which has styling overrides in the app, I would try and copy what the requested office user page is doing with react-admin's ArrayField component to match styling

I think this SHOULD be fixed once you make the above change, but I'd like to try and get rid of the side scrolling behavior if possible. It seems the font being bigger/different is causing some sizing differences when it should be similar to the other office user pages

Screen.Recording.2025-02-04.at.9.46.45.AM.mov
Let me know if any of that doesn't make sense. Re-request when you got it all fixed up! Thanks!

I'm working on getting the font the same. But there's 2 added columns in the rejected table vs the requested so I think that's why it's scrolling.

danieljordan-caci · 2025-02-04T19:50:49Z

Couple of things:

the font styling is different for the roles column, this is likely due to you using <p> which has styling overrides in the app, I would try and copy what the requested office user page is doing with react-admin's ArrayField component to match styling

I think this SHOULD be fixed once you make the above change, but I'd like to try and get rid of the side scrolling behavior if possible. It seems the font being bigger/different is causing some sizing differences when it should be similar to the other office user pages

Screen.Recording.2025-02-04.at.9.46.45.AM.mov
Let me know if any of that doesn't make sense. Re-request when you got it all fixed up! Thanks!

I'm working on getting the font the same. But there's 2 added columns in the rejected table vs the requested so I think that's why it's scrolling.

That's fair! I'm hoping that it'll fix when you adjust that font because I believe it should condense it down a lot. But... We'll see.

KonstanceH and others added 14 commits January 24, 2025 15:41

added cols to table/swagger and generated files.

ba3cfee

merging in latest main and fixing conflicts

f2119a3

added testharness fir requested/rejected office users

8aff514

adding services/tests/api. working on how rejected is added in test h…

6cb024b

…arness and add ui.

pause on frontend

3d188de

can see listed from ui. Need to fix data column and index test

e307356

Merge branch 'main' into B-21966-Add-Rejected-Admin-MAIN

6ab7fd3

Merge branch 'main' into B-21966-Add-Rejected-Admin-MAIN

75970c6

fixed index test

dbb9c0d

fixed pull in also dont need roles on main page just show page

c5b624e

update show page added roles sep by ,

bfecade

Merge branch 'main' into B-21966-Add-Rejected-Admin-MAIN

66074ca

trying to get test coverage up

2818a10

merge in from main and fix conflicts

7f87328

KonstanceH self-assigned this Jan 30, 2025

KonstanceH added ByteSize M&Ms Team ByteSized M&Ms INTEGRATION Slated for Integration Testing labels Jan 30, 2025

KonstanceH added 2 commits January 30, 2025 07:36

fix rejected testharness

a69d87b

fix rejected testharness

e0c7d0e

KonstanceH marked this pull request as ready for review January 30, 2025 14:26

KonstanceH requested review from a team as code owners January 30, 2025 14:26

KonstanceH requested review from JamesHawks224 and WeatherfordAaron January 30, 2025 14:27

danieljordan-caci requested changes Jan 30, 2025

View reviewed changes

JamesHawks224 requested changes Jan 30, 2025

View reviewed changes

Makefile Outdated Show resolved Hide resolved

.circleci/config.yml Outdated Show resolved Hide resolved

.gitlab-ci.yml Outdated Show resolved Hide resolved

JamesHawks224 previously approved these changes Jan 30, 2025

View reviewed changes

danieljordan-caci self-requested a review January 31, 2025 17:45

KonstanceH added 3 commits January 31, 2025 20:50

merge in latest main

5ec11ca

removing deleted columns and adding new migration

b79b241

removing deleted columns and adding new migration

6131213

KonstanceH dismissed JamesHawks224’s stale review via 6131213 January 31, 2025 21:17

KonstanceH and others added 4 commits January 31, 2025 21:21

merge in latest integratino and fix conflict

89ca553

Merge branch 'integrationTesting' into B-21966-Add-Rejected-Admin-INT

96d4b70

Merge branch 'integrationTesting' into B-21966-Add-Rejected-Admin-INT

4c75f0f

Merge branch 'integrationTesting' into B-21966-Add-Rejected-Admin-INT

0b34be2

KonstanceH requested review from taeJungCaci and brooklyn-welsh February 3, 2025 15:58

KonstanceH added 2 commits February 3, 2025 12:11

Merge branch 'integrationTesting' into B-21966-Add-Rejected-Admin-INT

fab4e21

Merge branch 'integrationTesting' into B-21966-Add-Rejected-Admin-INT

88a3faf

brooklyn-welsh previously approved these changes Feb 3, 2025

View reviewed changes

Merge branch 'integrationTesting' into B-21966-Add-Rejected-Admin-INT

418c4b5

danieljordan-caci requested changes Feb 4, 2025

View reviewed changes

danieljordan-caci reviewed Feb 4, 2025

View reviewed changes

Merge branch 'integrationTesting' into B-21966-Add-Rejected-Admin-INT

dfc6a9a

KonstanceH dismissed brooklyn-welsh’s stale review via 03f822b February 4, 2025 20:52

KonstanceH force-pushed the B-21966-Add-Rejected-Admin-INT branch 2 times, most recently from 03f822b to dfc6a9a Compare February 5, 2025 20:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

B 21966 add rejected admin int #14694

B 21966 add rejected admin int #14694

KonstanceH commented Jan 30, 2025 •

edited

Loading

robot-mymove commented Jan 30, 2025 •

edited

Loading

danieljordan-caci left a comment

JamesHawks224 left a comment

KonstanceH commented Jan 30, 2025

danieljordan-caci commented Jan 30, 2025

KonstanceH commented Jan 30, 2025

danieljordan-caci commented Jan 30, 2025

danieljordan-caci commented Jan 31, 2025

KonstanceH commented Jan 31, 2025

brooklyn-welsh left a comment

danieljordan-caci left a comment

danieljordan-caci Feb 4, 2025

danieljordan-caci Feb 4, 2025

KonstanceH Feb 4, 2025

danieljordan-caci Feb 4, 2025

KonstanceH Feb 4, 2025

KonstanceH commented Feb 4, 2025

danieljordan-caci commented Feb 4, 2025

B 21966 add rejected admin int #14694

Are you sure you want to change the base?

B 21966 add rejected admin int #14694

Conversation

KonstanceH commented Jan 30, 2025 • edited Loading

B-21966

Summary

Verification Steps for Reviewers

How to test

Frontend

Backend

Database

Any new migrations/schema changes:

Screenshots

robot-mymove commented Jan 30, 2025 • edited Loading

danieljordan-caci left a comment

Choose a reason for hiding this comment

JamesHawks224 left a comment

Choose a reason for hiding this comment

KonstanceH commented Jan 30, 2025

danieljordan-caci commented Jan 30, 2025

KonstanceH commented Jan 30, 2025

danieljordan-caci commented Jan 30, 2025

danieljordan-caci commented Jan 31, 2025

KonstanceH commented Jan 31, 2025

brooklyn-welsh left a comment

Choose a reason for hiding this comment

danieljordan-caci left a comment

Choose a reason for hiding this comment

danieljordan-caci Feb 4, 2025

Choose a reason for hiding this comment

danieljordan-caci Feb 4, 2025

Choose a reason for hiding this comment

KonstanceH Feb 4, 2025

Choose a reason for hiding this comment

danieljordan-caci Feb 4, 2025

Choose a reason for hiding this comment

KonstanceH Feb 4, 2025

Choose a reason for hiding this comment

KonstanceH commented Feb 4, 2025

danieljordan-caci commented Feb 4, 2025

KonstanceH commented Jan 30, 2025 •

edited

Loading

robot-mymove commented Jan 30, 2025 •

edited

Loading