Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Addresses most of Brandon's concerns #14

Merged
merged 5 commits into from
Jul 24, 2024
Merged

Conversation

JasonRobertFrancis
Copy link
Contributor

No description provided.

Copy link
Contributor

@bsedwards bsedwards left a comment

Choose a reason for hiding this comment

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

This is a great start. In addition to the comments in the files, I would suggest looking through the Error List tab for the files you've added and make sure Warnings are fixed, and Messages that are not just "style" related are fixed (for example, the unused variable messages).

There are still some things missing, compared to the old directory: phone numbers, the email host, user address, and student affiliations. Maybe the title and department columns can be combined to display this data. Also the Alt Photo link should be shown for users with SVMSecure.CATS.ServiceDesk.

How do you want to handle the "All UCD" mode of the directory, where it searches the campus LDAP first, and then adds our info?

I appreciate the work you've done on this! Once we figure out the permission issues with the IDs I can approve this as a starting point.

web/Areas/Directory/Controllers/DirectoryController.cs Outdated Show resolved Hide resolved
web/Areas/Directory/Controllers/DirectoryController.cs Outdated Show resolved Hide resolved
web/Areas/Directory/Controllers/DirectoryController.cs Outdated Show resolved Hide resolved
web/Areas/Directory/Views/Index.cshtml Outdated Show resolved Hide resolved
web/Areas/Directory/Views/Index.cshtml Outdated Show resolved Hide resolved
web/Areas/Directory/Views/Index.cshtml Outdated Show resolved Hide resolved
web/Areas/Directory/Controllers/DirectoryController.cs Outdated Show resolved Hide resolved
web/Areas/Directory/Controllers/DirectoryController.cs Outdated Show resolved Hide resolved
@JasonRobertFrancis JasonRobertFrancis changed the title Adds first-draft of directory Addresses most of Brandon's concerns Jan 31, 2024
@JasonRobertFrancis
Copy link
Contributor Author

This is a great start. In addition to the comments in the files, I would suggest looking through the Error List tab for the files you've added and make sure Warnings are fixed, and Messages that are not just "style" related are fixed (for example, the unused variable messages).

There are still some things missing, compared to the old directory: phone numbers, the email host, user address, and student affiliations. Maybe the title and department columns can be combined to display this data. Also the Alt Photo link should be shown for users with SVMSecure.CATS.ServiceDesk.

How do you want to handle the "All UCD" mode of the directory, where it searches the campus LDAP first, and then adds our info?

I appreciate the work you've done on this! Once we figure out the permission issues with the IDs I can approve this as a starting point.

Thanks! I will work on a few of the remaining items (email host, address, student affiliations, alt photo). I will also need to think about the All UCD mode. I wanted to at least start a conversation about a few of my changes, so I thought it would be helpful to show you where I am now.

web/Classes/Utilities/LdapService.cs Dismissed Show dismissed Hide dismissed
Updates to the layout, fixes LDAP issues, adds context-specific directory.css
Adds the info missing from directory:
 - Department and/or student level
 - VMACS mobile
 - MIV ID
 - Photo use just the mailid portion of the email
Fixes EmulateUser link
Adds tooltip to buttons
@bsedwards bsedwards self-requested a review July 23, 2024 20:47
@JasonRobertFrancis JasonRobertFrancis merged commit cea462e into main Jul 24, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants