Project-based permissioning `TopNavigation` #6027

Twixes · 2021-09-20T09:44:09Z

Changes

This adjusts TopNavigation UI for #5976 like this:

Project switcher with an allowed project and a restricted project (restricted only if project-based permissioning is available and enabled)

Change extracted from #5976 for clarity.

Resolves #4263 when merged with #6068.

…issions

…ion switch

Twixes · 2021-09-21T19:23:16Z

@mariusandra I want to add TopNavigation to Storybook to demonstrate this PR, but despite copying code from a story that already works (Preflight I think) and following the README, TopNavigation.stories.tsx fails with this:

Do you perhaps have clue what's wrong?

mariusandra · 2021-09-22T10:24:35Z

I think if you manually mount featureFlagLogic inside resetKeaWithState in kea-story.tsx, it'll work. What seems to be happening here is that some logic somewhere is calling featureFlagLogic in an afterMount block, where things do not get auto-mounted (bug in kea I guess?). That logic is normally always mounted, so we might as well always have it here.

Twixes · 2021-09-22T11:52:09Z

Thanks! ❤️ Mounting featureFlagLogic does allow rendering the component (seems like an issue within navigationLogic).

However there's still a different type of issue:
Providing initial state is not useful for logics that load their data from the API after mount. And since our components (especially high-level ones like TopNavigation) are very logic-heavy, there isn't a way of just passing the data to the component. Because of this, there's no data to display in the story.

In Jest we have a way of mocking the fetch API – I suppose this is possible for Storybook too, but as a precaution: do we have a way of doing it already?

mariusandra · 2021-09-22T12:18:58Z

We currently don't. My undocumented plan was to extend this widget:

... and turn it into a practical network request recording/playback tool. Basically it would have 2-4 modes: 1) live (connect somewhere like now), 2) live, but recording every fetch - maybe even saves as a .json file in the right folder for the story? 3) playback from the live state either with recorded network requests 4), and/or with recorded state.

It should be quite easy to extend it to support this, but I haven't found the time yet.

mariusandra · 2021-09-22T14:14:32Z

Here's a HN comment by somebody else who has actually done just that, but no code to share. And here's one that's also interesting from this thread.

However, I think there's also a bug here. Normally we shouldn't load users/@me as this is passed directly from django via getAppContext. This is not properly mocked in the stories...

paolodamico

lgtm! only thing is that the component doesn't seem to be rendering correctly on Storybook

frontend/src/layout/__stories__/Navigation.stories.tsx

frontend/src/lib/storybook/kea-story.tsx

frontend/src/layout/navigation/TopNavigation.tsx

paolodamico · 2021-09-23T10:02:10Z

frontend/src/layout/navigation/TopNavigation.tsx

@@ -251,6 +282,10 @@ export function TopNavigation(): JSX.Element {
                        }
                    )
                }
+                style={{
+                    cursor: isProjectCreationForbidden ? 'default' : undefined,
+                    color: isProjectCreationForbidden ? 'var(--text-muted)' : undefined,


Suggested change

color: isProjectCreationForbidden ? 'var(--text-muted)' : undefined,

color: isProjectCreationForbidden ? 'var(--text-muted)' : 'var(--primary)',

Would prefer not to, to keep consistency between "Create new project" and "Create new organization"

I don't think we need consistency among these, they're in different places, plus adding a project is probably a lot more frequent than an org. I'm also wondering if the create org should also be primary color.

Alrighty, made both primary

Co-authored-by: Paolo D'Amico <paolodamico@users.noreply.github.com>

mariusandra

Had a quick look, seems fine in general. Some nits though :)

Not sure if outside this PR, but there's a "cancel" icon on the mouse even though the button works:

mariusandra · 2021-09-28T07:37:31Z

frontend/src/layout/__stories__/Navigation.stories.tsx

+        <Layout>
+            <MainNavigation />
+            <Layout>
+                <TopNavigation />


Somehow the top-right part of the navigation disappears:

That's because of the issue discussed in #6027 (comment)
This looks like it could somehow help, but I've no idea how to make use of it:

mariusandra · 2021-09-28T07:41:50Z

frontend/src/layout/navigation/TopNavigation.tsx

+function ProjectRow({ team }: { team: TeamBasicType }): JSX.Element {
+    const { currentTeam } = useValues(teamLogic)
+    const { updateCurrentTeam } = useActions(userLogic)
+    const { push } = router.actions


Nit: This could also be a useActions(router) for consistency. Makes no real difference, as push is an action and router is always mounted, but still nit nit nit.

Co-authored-by: Paolo D'Amico <paolodamico@users.noreply.github.com>

* Fix Access Control restriction tooltip * Add `TeamMemberAccessPermission` and use it in viewsets * Add `ErrorProjectUnavailable` scene * Ignore mypy * Update MainNavigation.tsx * Update explicit_team_member.py * Fix frontend detection of unavailable project * Fix some tests and edge cases * Fix basic permissions * Add more tests * Simplify `ExplicitTeamMemberViewSet` permissions * Improve restrictions and add moar tests * Update frontend * Fix a couple of things * Fix import * Fix some edge cases * Fix typing errors * Use hedgehog instead of moth Co-authored-by: Paolo D'Amico <paolodamico@users.noreply.github.com> * Address feedback * Add proper permissioning to dashboard views * Update ee/api/test/test_dashboard.py Co-authored-by: Paolo D'Amico <paolodamico@users.noreply.github.com> Co-authored-by: Paolo D'Amico <paolodamico@users.noreply.github.com>

Twixes added 30 commits September 8, 2021 15:32

Refactor AvailableFeature from strings to an enum everywhere

dca7d26

Merge branch 'available-features-refactor' into 4263-per-project-perm…

cda34b0

…issions

Fix circular dependency and type

4556dac

Merge branch 'available-features-refactor' into 4263-per-project-perm…

53dda58

…issions

Add "Per-project access" feature flag, premium feature, and organizat…

d7e46a7

…ion switch

Merge branch 'master' into 4263-per-project-permissions

8d4cd1b

Rename OrganizationMembershipLevel to OrganizationAccessLevel

a4ccf08

Create ExplicitTeamMembership model

f3b7f06

Show whether projects are restricted in the project switcher

2937e10

Update organizations API code

f4f6eef

Merge branch 'master' into 4263-per-project-permissions

4e82889

Fix migrations

bb9139b

Move organization tests that require EE to ee

1f75964

Revert OrganizationMembershipLevel rename

0f393ef

Fix organization tests

a516df4

Merge branch 'master' into 4263-per-project-permissions

29e3761

Update migration

a27bb9f

Fix schema and add Members to Project Settings

716a1b0

Build out test memberships API with security tests

a587c22

Update TeamMembers and teamMembersLogic

e102592

Move "Per-project access" description to tooltip

e43d256

Add moar tests

951e4d7

Fix Project Members list logic

de62b39

Add additional membership checks

3f02307

Merge branch 'master' into 4263-per-project-permissions

c95fc2c

Update migrations

7a8ea92

Fix typing

b74d270

Adjust explicit team memberships API similarly

43823a1

Fix typo

1d6b4d0

Unify ExplicitTeamMemberSerializer

6f01d1e

Make project icon in top nav itself dynamic as well

fe208d6

Twixes temporarily deployed to posthog-pr-6027 September 21, 2021 17:27 Inactive

Twixes marked this pull request as ready for review September 21, 2021 17:32

Twixes requested a review from mariusandra September 21, 2021 19:20

Twixes changed the title ~~Per-project access TopNavigation~~ Project-based permissioning TopNavigation Sep 21, 2021

Fix story

af50905

Twixes temporarily deployed to posthog-pr-6027 September 22, 2021 11:44 Inactive

Base automatically changed from 4263-per-project-permissions-framework to master September 22, 2021 16:30

Twixes added 2 commits September 22, 2021 18:59

Merge branch 'master' into 4263-top-nav

52ecdf3

Delete 0169_project_based_permissioning.py

031149c

Twixes temporarily deployed to posthog-pr-6027 September 22, 2021 16:59 Inactive

Twixes requested a review from paolodamico September 22, 2021 17:13

paolodamico approved these changes Sep 23, 2021

View reviewed changes

Twixes and others added 2 commits September 23, 2021 18:13

Merge branch 'master' into 4263-top-nav

a1c695c

Apply suggestions from code review

54caf7c

Co-authored-by: Paolo D'Amico <paolodamico@users.noreply.github.com>

mariusandra approved these changes Sep 28, 2021

View reviewed changes

Twixes and others added 6 commits September 29, 2021 17:40

Update frontend/src/layout/navigation/TopNavigation.tsx

6443129

Co-authored-by: Paolo D'Amico <paolodamico@users.noreply.github.com>

Address feedback

6606026

Merge branch 'master' into 4263-top-nav

2e1f58a

Run prettier

fa8b319

Remove debug code

7635a8f

Twixes enabled auto-merge (squash) September 29, 2021 22:43

Twixes merged commit 3a67367 into master Sep 29, 2021

Twixes deleted the 4263-top-nav branch September 29, 2021 22:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Project-based permissioning `TopNavigation` #6027

Project-based permissioning `TopNavigation` #6027

Twixes commented Sep 20, 2021 •

edited

Loading

Twixes commented Sep 21, 2021 •

edited

Loading

mariusandra commented Sep 22, 2021

Twixes commented Sep 22, 2021 •

edited

Loading

mariusandra commented Sep 22, 2021

mariusandra commented Sep 22, 2021

paolodamico left a comment

paolodamico Sep 23, 2021

Twixes Sep 24, 2021

paolodamico Sep 27, 2021

Twixes Sep 29, 2021

mariusandra left a comment

mariusandra Sep 28, 2021

Twixes Sep 29, 2021

mariusandra Sep 28, 2021

	color: isProjectCreationForbidden ? 'var(--text-muted)' : undefined,
	color: isProjectCreationForbidden ? 'var(--text-muted)' : 'var(--primary)',

Project-based permissioning TopNavigation #6027

Project-based permissioning TopNavigation #6027

Conversation

Twixes commented Sep 20, 2021 • edited Loading

Changes

Project switcher with an allowed project and a restricted project (restricted only if project-based permissioning is available and enabled)

Twixes commented Sep 21, 2021 • edited Loading

mariusandra commented Sep 22, 2021

Twixes commented Sep 22, 2021 • edited Loading

mariusandra commented Sep 22, 2021

mariusandra commented Sep 22, 2021

paolodamico left a comment

Choose a reason for hiding this comment

paolodamico Sep 23, 2021

Choose a reason for hiding this comment

Twixes Sep 24, 2021

Choose a reason for hiding this comment

paolodamico Sep 27, 2021

Choose a reason for hiding this comment

Twixes Sep 29, 2021

Choose a reason for hiding this comment

mariusandra left a comment

Choose a reason for hiding this comment

mariusandra Sep 28, 2021

Choose a reason for hiding this comment

Twixes Sep 29, 2021

Choose a reason for hiding this comment

mariusandra Sep 28, 2021

Choose a reason for hiding this comment

Project-based permissioning `TopNavigation` #6027

Project-based permissioning `TopNavigation` #6027

Twixes commented Sep 20, 2021 •

edited

Loading

Twixes commented Sep 21, 2021 •

edited

Loading

Twixes commented Sep 22, 2021 •

edited

Loading