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

feat: setup UserStore and basic visual feedback throughout editor based on access #2214

Merged
merged 17 commits into from
Sep 26, 2023

Conversation

jessicamcinchak
Copy link
Member

@jessicamcinchak jessicamcinchak commented Sep 8, 2023

Changes

  • Adds a UserStore with simple state management getters & setters for working with users & their roles within all authenticated routes
  • Adds visual feedback throughout the editor UI to indicate if you have edit- or view-access for that team (this is crude and I'm not a designer! but hopefully gives us some starting ideas to improve on now 🙂)

Visual feedback examples

Teams

  • Shows an icon next to each team to indicate edit- (:pen:) or view- (:eye:) access (in the future, we may want to think about ordering so edit-access is at the top etc)
    Screenshot from 2023-09-22 10-27-15

Team

  • Shows an icon in the team header to indicate edit- or view-access (we only restrict at team-level right now, not flows)
  • If view-access, hides the copy/move/rename/delete menu & the "add a new service" button at the bottom of the list of services
  • If edit-access, all options display as "normal"
    Screenshot from 2023-09-22 10-30-42

Flow

  • If view-access, disables the "check for changes to publish" button
  • If view-access, hides all <Hanger /> points in the graph which makes it impossible to move, add or clone nodes (in the UI at least, not the actual operations table!)
  • If view-access, I can still navigate through external portals, and that external portal flow loads correctly depending on my permissions to its' team
  • If edit-access, all options display as "normal"
  • It shows an icon next to the flow name in the header regardless of access
  • It shows my real initial (!) regardless of access
    Screenshot from 2023-09-22 13-28-40

Node

  • If view-access, disables the delete/make unique/update buttons in the node dialog. You can still close using the upper "X" or clicking outside
  • If view-access, disables the "image upload" icon preventing new images from being uploaded and disables the "remove" item in the menu of existing uploaded images
  • If view access, disables the "add new", "drag/re-order" and "remove" icons for list options
  • If view access, inputs are not disabled to allow easier copy + paste, but no changes will persist or be written to the DB
    Screenshot from 2023-09-26 14-11-49

** I learned our original hypothesis about not connecting to ShareDB altogether, rather than restricting per operation method, doesn't actually work very well here because the connect & connectTo Editor store methods are actually responsible for loading the whole flow - so if we don't proceed with the connection, it just shows an empty graph!

Open questions / future improvements

  • Where should the TeamStore be initiated ideally? For now, I often rely on window.location.pathname to get the teamSlug instead (which works totally fine, but doesn't feel the cleanest and assumes the editor will never be hosted at a custom subdomain!)
  • I haven't touched global settings or settings pages yet
  • How to consistently show user initial menu across teams, team and flow pages? It's inconsistent for me now on teams and team (eg sometimes shows if I'm navigating back from a flow, but not on initial load)
  • TESTS - there's a few that got skipped here because we'll need to figure out how to mock an authenticated user with platform admin permissions for buttons to be visible etc

@github-actions
Copy link

github-actions bot commented Sep 8, 2023

Removed vultr server and associated DNS entries

@jessicamcinchak
Copy link
Member Author

@jessicamcinchak jessicamcinchak changed the title feat: setup user store feat: setup UserStore and basic visual feedback throughout editor based on access Sep 22, 2023
@jessicamcinchak jessicamcinchak marked this pull request as ready for review September 22, 2023 11:02
@jessicamcinchak jessicamcinchak requested a review from a team September 22, 2023 11:02
Copy link
Contributor

@DafyddLlyr DafyddLlyr left a comment

Choose a reason for hiding this comment

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

A few comments on PR, no show stoppers! Looking great and feels very solid 👏 I'm sure Alastair and Ian will have some design thoughts but that's 100% a separate issue.

A few unit tests for maybe some components and UserStore methods would probably be wise as a next step 👍

Hitting a few issues small on the Pizza mainly related to the teamStore not being populated. I think this is related to an issue we've discussed before around how teams are handled within the editor (window.api.getState().getTeam() is perfect in /preview and /unpublished but inconsistent in the Editor as you've flagged in this PR.

I think we'll need to resolve this before going for a prod deploy - I'll queue on up just now so we can get the code changes from already merged to main pushed forward in the meantime.

@DafyddLlyr
Copy link
Contributor

Just realised in the demo...

  • We need to disable drag drop in the modal
  • We need to hide "delete" buttons in modal

I'm not exactly sure when the "update" happens - I think it's on save of the modal but juuuuust in case? And I guess it's about expected behaviour also.

@@ -365,7 +365,6 @@ export const editorStore: StateCreator<
toBefore,
})(get().flow);
send(ops);
get().resetPreview();
Copy link
Member Author

@jessicamcinchak jessicamcinchak Sep 26, 2023

Choose a reason for hiding this comment

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

so this is a notable change! moveNode is the only graph method that called this shared store method, which causes the window to reload, the user to no longer be set, and therefore the flow incorrectly reloads as if it's now view-only rather than edit-access as intended.

from what I can tell, resetPreview is intended to be called outside the Editor based on the variables it's setting, and in testing I can't see any in-editor issues with removing it (the operation is still successful, the flow order after move persists as expected, etc). But please do test this!

@jessicamcinchak jessicamcinchak merged commit cf1e311 into main Sep 26, 2023
12 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