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.

Sign up for GitHub

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

Jump to bottom

Manage members in organization [INS-2705] #7949

Merged
merged 4 commits into from
Sep 24, 2024
Merged

Manage members in organization [INS-2705] #7949

merged 4 commits into from
Sep 24, 2024

Conversation

yaoweiprc
Copy link
Contributor

@yaoweiprc yaoweiprc commented Sep 12, 2024 •
edited
Loading

Now users can

  • invite new members
  • change member's role
  • remove existing member
  • revoke invitation
  • search members and invites
    in app. It's basically migrated from insomnia-website-app. The logic for permission validation and error messages is essentially the same as in the web version. Except that:
  • existing members and pending invites are combined into one list
  • users can choose role when inviting
image

Other changes:

  • Add an react hook useStateRef to get latest state immediately after setState, rather than in next round of render.
  • Add an extra param onlyResolveOnSuccess to insomniaFetch so that it can reject when the response status code is not 2xx.

@yaoweiprc yaoweiprc force-pushed the feat/in-app-invite branch 2 times, most recently from b80ae5a to 9c13211 Compare September 12, 2024 10:53
jackkav
jackkav reviewed Sep 12, 2024
View reviewed changes
.vscode/settings.json Outdated Show resolved Hide resolved
@notjaywu notjaywu marked this pull request as draft September 13, 2024 08:07
@yaoweiprc yaoweiprc force-pushed the feat/in-app-invite branch 4 times, most recently from e43aa2d to ef85afb Compare September 19, 2024 11:27
@yaoweiprc yaoweiprc changed the title Manage members in organization. Manage members in organization [INS-2705] Sep 19, 2024
@yaoweiprc yaoweiprc force-pushed the feat/in-app-invite branch 4 times, most recently from 9485e3f to 2d06c9b Compare September 20, 2024 03:01
@yaoweiprc yaoweiprc marked this pull request as ready for review September 20, 2024 03:17
@cwangsmv
Copy link
Contributor

It is better to show a skeleton like web when user opens the invite modal. Users will see "no member" first for now.

Screen.Recording.2024-09-20.at.14.23.57.mp4

@cwangsmv
Copy link
Contributor

Should we change the title and hide stack trace for RBAC error:
Screenshot 2024-09-20 at 14 33 42

Web behavior:

Screenshot 2024-09-20 at 14 35 09

@CurryYangxx
Copy link
Member

This pr include some changes that are not part of this feature:

  • add a new react hook
  • add a new param in insomniaFetch

It is better to explain these changes in pr description to make review easier

@CurryYangxx
Copy link
Member

There are much typescript definition and util functions in component file, we can discuss if it is better to move them to some other dir together.

@cwangsmv cwangsmv self-requested a review September 20, 2024 08:16
@yaoweiprc
Copy link
Contributor Author

This pr include some changes that are not part of this feature:

  • add a new react hook
  • add a new param in insomniaFetch

It is better to explain these changes in pr description to make review easier

Yeah, I have added it.

@notjaywu
Copy link
Contributor

Should we change the title and hide stack trace for RBAC error: Screenshot 2024-09-20 at 14 33 42

Web behavior:

Screenshot 2024-09-20 at 14 35 09
image

@yaoweiprc
Copy link
Contributor Author

It is better to show a skeleton like web when user opens the invite modal. Users will see "no member" first for now.

Screen.Recording.2024-09-20.at.14.23.57.mp4

image
Add a spin icon when loading.

@yaoweiprc
Copy link
Contributor Author

Should we change the title and hide stack trace for RBAC error: Screenshot 2024-09-20 at 14 33 42

Web behavior:

Screenshot 2024-09-20 at 14 35 09

image
Use another alert modal.

cwangsmv
cwangsmv previously approved these changes Sep 23, 2024
View reviewed changes
Copy link
Contributor

@cwangsmv cwangsmv left a comment

Choose a reason for hiding this comment

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

lgtm

@yaoweiprc yaoweiprc force-pushed the feat/in-app-invite branch 2 times, most recently from 27d0f6f to afdc4bf Compare September 24, 2024 09:04
@yaoweiprc
Manage members in organization.
e8f6547
@yaoweiprc
Set invite modal un-dismissable so that the invite modal does not clo…
c1766a5 
…se when close another error modal.

Clarify situations of remove button's disable state.
Refetch organization info and user info every time invite modal is opened, so that we can get latest status if org's plan or user's role are changed.
@yaoweiprc
Add spin icon when loading.
6f84cbd 
Use another alert modal.
Generate errMsg in insomniaFetch.
@yaoweiprc
Add optimistic update when updating current user's role in org.
2fc2fa2
@yaoweiprc yaoweiprc merged commit 44eb7ea into develop Sep 24, 2024
8 checks passed
@yaoweiprc yaoweiprc deleted the feat/in-app-invite branch September 24, 2024 10:00
@sentry-io Sentry.io
Copy link

sentry-io bot commented Sep 25, 2024

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ Error: Failed to fetch roles app:///index.html View Issue

Did you find this useful? React with a 👍 or 👎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@CurryYangxx CurryYangxx CurryYangxx left review comments

@jackkav jackkav jackkav left review comments

@cwangsmv cwangsmv cwangsmv approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@yaoweiprc @cwangsmv @CurryYangxx @notjaywu @jackkav