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

added a check for when you are asking from your profile, or your public profile #951

Merged
merged 1 commit into from
Jul 10, 2023

Conversation

antoniavonto
Copy link
Contributor

@antoniavonto antoniavonto commented Jul 6, 2023

To fix Issue: When you click on view my public profile your private involvements still show up for you
UI: gordon-cs/gordon-360-ui#1969

@antoniavonto antoniavonto marked this pull request as ready for review July 6, 2023 18:56
@antoniavonto antoniavonto requested review from russtuck, EjPlatzer and amos-cha and removed request for russtuck July 6, 2023 18:56
@antoniavonto antoniavonto self-assigned this Jul 6, 2023
@antoniavonto antoniavonto added s23 1 Point To measure progress labels Jul 6, 2023
@antoniavonto antoniavonto marked this pull request as draft July 7, 2023 13:07
@antoniavonto
Copy link
Contributor Author

antoniavonto commented Jul 7, 2023

involvements should show up grayed out because the user share those club sessions with himself and all people in the same club session can see its members.

Copy link
Contributor

@amos-cha amos-cha left a comment

Choose a reason for hiding this comment

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

1 change, but make sure it works. I want to say it does but I didn't test it.

/// <param name="involvementCode">Optional involvementCode filter</param>
/// <param name="username">Optional username filter</param>
/// <param name="sessionCode">Optional session code for which session memberships should be retrieved. Defaults to current session. Use "*" for all sessions.</param>
/// <param name="participationTypes">Optional list of participation types that should be retrieved. Defaults to all participation types.</param>
/// <returns>An IEnumerable of the matching MembershipViews</returns>
[HttpGet]
[StateYourBusiness(operation = Operation.READ_PARTIAL, resource = Resource.MEMBERSHIP)]
public ActionResult<IEnumerable<MembershipView>> GetMemberships(string? involvementCode = null, string? username = null, string? sessionCode = null, [FromQuery] List<string>? participationTypes = null)
public ActionResult<IEnumerable<MembershipView>> GetMemberships(bool? myProf, string? involvementCode = null, string? username = null, string? sessionCode = null, [FromQuery] List<string>? participationTypes = null)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
public ActionResult<IEnumerable<MembershipView>> GetMemberships(bool? myProf, string? involvementCode = null, string? username = null, string? sessionCode = null, [FromQuery] List<string>? participationTypes = null)
public ActionResult<IEnumerable<MembershipView>> GetMemberships(bool? myProf = false, string? involvementCode = null, string? username = null, string? sessionCode = null, [FromQuery] List<string>? participationTypes = null)

@@ -44,12 +45,13 @@ public ActionResult<IEnumerable<MembershipView>> GetMemberships(string? involvem
{
var authenticatedUserUsername = AuthUtils.GetUsername(User);
var viewerGroups = AuthUtils.GetGroups(User);
bool mp = myProf ?? false;
Copy link
Contributor

Choose a reason for hiding this comment

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

it occurred to me that we could make this variable defaulted in the parameter list

Suggested change
bool mp = myProf ?? false;


// User can see all their own memberships. SiteAdmin and Police can see all of anyone's memberships
if (!(username == authenticatedUserUsername
|| viewerGroups.Contains(AuthGroup.SiteAdmin)
|| viewerGroups.Contains(AuthGroup.Police)
))
) || !mp)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
) || !mp)
) || !myProf)

@antoniavonto
Copy link
Contributor Author

1 change, but make sure it works. I want to say it does but I didn't test it.

I will give it a time out on this project because we found out that it might be working as intended since involvements should show up grayed out because the user share those club sections with himself.

@antoniavonto antoniavonto merged commit 6ad249c into develop Jul 10, 2023
3 checks passed
@ArabellaJi ArabellaJi mentioned this pull request Jul 10, 2023
@amos-cha amos-cha deleted the s23-involvements-public-myProf branch July 24, 2023 19:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1 Point To measure progress s23
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants