Skip to content

Conversation

@michaelchadwick
Copy link
Contributor

@michaelchadwick michaelchadwick commented Aug 28, 2025

Fixes ilios/ilios#4693

Added [id] to search results for easier reference.

Added campusId as a title= attribute to the button so it appears as a tooltip.

@netlify
Copy link

netlify bot commented Aug 28, 2025

Deploy Preview for ilios-frontend ready!

Name Link
🔨 Latest commit e805f18
🔍 Latest deploy log https://app.netlify.com/projects/ilios-frontend/deploys/690d4408e66b1d0008627ba4
😎 Deploy Preview https://deploy-preview-8783--ilios-frontend.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@michaelchadwick michaelchadwick force-pushed the frontend-4693-admin-search-user-id branch from 45063d5 to 0d4233e Compare September 2, 2025 15:51
Copy link
Member

@stopfstedt stopfstedt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@stopfstedt stopfstedt removed the request for review from jrjohnson September 3, 2025 23:47
Copy link
Member

@dartajax dartajax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

works for me - I'll get the tests going - merge tomorrow maybe

@dartajax dartajax added the run ui tests Run the expensive UI tests label Sep 3, 2025
@dartajax
Copy link
Member

dartajax commented Sep 4, 2025

I'm going to wait on this one and merge it tomorrow after our meeting - just want to make sure there are no issues with appending this information to search. It seems it would be helpful. I know that because I filed the ticket while experiencing confusion around this exact issue.

@dartajax
Copy link
Member

dartajax commented Sep 4, 2025

I am not going to merge this but will keep it open for discussion - assigned to me. This ticket was created with a purpose at hand but this is not the correct solution. I apologize for that. The need does arise for admin users to obtain more information about users during a user search. This occurs when multiple users have the same email address, which has happened. Maybe we need to provide a callout, mouse-over, or other report for these users but the inclusion of the Ilios ID on the same line in the interface has been decided not to be the solution. Keep this open please.

@michaelchadwick michaelchadwick marked this pull request as draft October 1, 2025 19:58
@michaelchadwick
Copy link
Contributor Author

@dartajax I think we decided as a team to go through with this one but only if it gets added as a tooltip instead of inline with the name, so I'll update this to that effect.

@michaelchadwick michaelchadwick force-pushed the frontend-4693-admin-search-user-id branch from 43289a0 to dfac501 Compare November 6, 2025 22:47
@michaelchadwick michaelchadwick marked this pull request as ready for review November 6, 2025 22:47
Copy link
Member

@dartajax dartajax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure about this one but it seems useful although users may wonder what the context of that number popping up is - maybe we should add "Ilios User ID: " to it?

@dartajax
Copy link
Member

dartajax commented Nov 6, 2025

Maybe @saschaben might want to check this one out.

@dartajax
Copy link
Member

dartajax commented Nov 7, 2025

I slapped a DO NOT MERGE on here - wanted @saschaben to check it out first.

@saschaben
Copy link
Member

This is an incorrect solution: we are pulling the Ilios ID as the tooltip, when we should be pulling the Campus ID (the "02" number). Swap out the data attribute getting called and I think we are back on track.

@dartajax
Copy link
Member

dartajax commented Nov 7, 2025

I am keeping the DO NOT MERGE on there still. Will comment more tomorrow.

@michaelchadwick michaelchadwick marked this pull request as draft November 21, 2025 17:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

DO NOT MERGE run ui tests Run the expensive UI tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Consider providing More Information (ID, attached Courses) etc. to Help with Duplicates

4 participants