-
Notifications
You must be signed in to change notification settings - Fork 411
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
fix: Updates user segment tab when a segment is created/updated #4950
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
Docker builds report
|
Uffizzi Preview |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm... @tiagoapolo this still doesn't seem to work correctly. It seems like it doesn't update each time, only after you update the segment twice? See recording here:
4950_202501071057.mov
As discussed on the standup with @tiagoapolo, this is actually caused by the fact that the logic to get the segments for a given identity uses the environment document, which is updated asynchronously by the task processor. See the code here, and subsequently here. Thus, the issue raised by myself above actually requires an improvement to be implemented in the API. Some options for this are:
My preference would likely be (3) but will probably require changes to the engine as well. Another, simpler issue would be to add a 'reload' option to the 'Users' tab so that users can manually reload the list which would allow them to reload after making a change to the segment. What do you think about this approach @tiagoapolo ? |
Makes sense! Also by having a refresh btn we don't need to refetch the list when the tab is loaded. |
frontend/web/components/modals/CreateSegmentUsersTabContent.tsx
Outdated
Show resolved
Hide resolved
2. updates user identity list after segment update/creation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me now
Thanks for submitting a PR! Please check the boxes below:
docs/
if required so people know about the feature!Changes
Ref: #4859
CreateSegmentRulesTabForm
andCreateSegmentUsersTabContent
components.How did you test this code?