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

updates consultant roles #2818

Closed
wants to merge 3 commits into from
Closed

Conversation

brentkulwicki
Copy link
Contributor

@brentkulwicki brentkulwicki commented Jul 13, 2023

┆Issue is synchronized with this Monday item by Unito

PR for 4558680508

@brentkulwicki brentkulwicki marked this pull request as draft July 13, 2023 21:28
@brentkulwicki brentkulwicki marked this pull request as ready for review July 14, 2023 13:46
Copy link
Member

@CarsonF CarsonF left a comment

Choose a reason for hiding this comment

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

Your test failures are unrelated FYI

@brentkulwicki
Copy link
Contributor Author

I think this has it all squared away now.

Consultant + Consultant manager can see progress reports including the team updates, story and progress as members of a project

Consultant manager can see any of these things for any Low or Medium sensitivity projects

Comment on lines +25 to +31
r.ProgressReport.when(member).read.children((c) =>
[c.teamNews, c.communityStories, c.highlights].flatMap((it) => it.read),
),
r.ProgressReportVarianceExplanation.when(member).read,
r.ProgressReportTeamNews.read.specifically((p) => p.responses.read),
r.ProgressReportCommunityStory.read.specifically((p) => p.responses.read),
r.ProgressReportHighlight.read.specifically((p) => p.responses.read),
Copy link
Member

Choose a reason for hiding this comment

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

I believe what we want here is

[
  r.ProgressReportCommunityStory,
  r.ProgressReportHighlight,
  r.ProgressReportTeamNews,
  r.ProgressReportVarianceExplanation,
].flatMap((it) => it.when(member).read),

Your lines 26 is trying to be the same thing as 29-31. So we don't need both, and we prefer the non-children syntax for static parent cases.

The other difference is that 29-31 allows reading globally, instead of only if a member. Which I don't think is the intent.

Also I know we talked before about this, but the

.specifically((p) => p.responses.read),

shouldn't be needed as it's the same action permission as what the object level has.

@brentkulwicki
Copy link
Contributor Author

Fixed here: #2853 (review)

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.

None yet

2 participants