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

HEEDLS-558 Show tracking system to UserAdmin #500

Merged
merged 2 commits into from
Jul 22, 2021

Conversation

livzorn
Copy link
Contributor

@livzorn livzorn commented Jul 22, 2021

A small bug fix - I changed the tracking system access to accept users who are User Admins, Centre Managers, or Centre Admins, rather than assuming that if a user is a Centre Admin they have those other permissions.

@@ -13,7 +13,7 @@ public class ApplicationSelectorController : Controller
public IActionResult Index()
{
var learningPortalAccess = User.GetCustomClaimAsBool(CustomClaimTypes.LearnUserAuthenticated) ?? false;
var trackingSystemAccess = User.GetCustomClaimAsBool(CustomClaimTypes.UserCentreAdmin) ?? false;
var trackingSystemAccess = User.GetCustomClaimAsBool(CustomClaimTypes.UserUserAdmin) | User.GetCustomClaimAsBool(CustomClaimTypes.UserCentreManager) | User.GetCustomClaimAsBool(CustomClaimTypes.UserCentreAdmin) ?? false;
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be able to be replaced with User.HasCentreAdminPermissions(), I think the resulting bool is the same.

If it can't be replaced by that smaller helper method, you need to run the formatter on this to break up the long line. If you don't know how to do that drop me a message.

Copy link
Contributor

@AlexJacksonDS AlexJacksonDS left a comment

Choose a reason for hiding this comment

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

I think the long line can be replaced with a claims helper method, otherwise it needs the formatter running.

Copy link
Contributor

@AlexJacksonDS AlexJacksonDS left a comment

Choose a reason for hiding this comment

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

Looks good.

@livzorn livzorn force-pushed the HEEDLS-558-show-tracking-system-to-useradmin branch from a3f9798 to 182deee Compare July 22, 2021 13:14
Copy link
Contributor

@stellake stellake left a comment

Choose a reason for hiding this comment

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

Looks good! 👍

@stellake stellake merged commit 25507e9 into master Jul 22, 2021
@stellake stellake deleted the HEEDLS-558-show-tracking-system-to-useradmin branch July 22, 2021 13:48
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.

3 participants