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

FF: The order of individualForSegment is reversed when fetched from backend #1827

Closed
zackcl opened this issue Aug 13, 2024 · 10 comments · Fixed by #1978
Closed

FF: The order of individualForSegment is reversed when fetched from backend #1827

zackcl opened this issue Aug 13, 2024 · 10 comments · Fixed by #1978
Assignees
Labels

Comments

@zackcl
Copy link
Collaborator

zackcl commented Aug 13, 2024

Version where bug was found:
5.2.0

Describe the bug
The order of individualForSegment is reversed when fetched from the backend. The order appears correct in the Postgres DB, but it is reversed when retrieved from the backend.

To Reproduce
Steps to reproduce the behavior:

  1. On the feature flag details page, click the "Add Include List" button.
  2. Select "Individual" type and define 3 values (e.g., user1, user2, user3).
  3. Add the Include List, then refresh the details page.
  4. Click "Edit" to see the defined values.
  5. You will see that the order of the values is reversed (e.g., user3, user2, user1).

Expected behavior
The order of individualForSegment in the fetched FeatureFlag data should match the order stored in the DB.

Additional context
The order of the fetched featureFlagSegmentInclusion (which contains the include lists) also doesn’t seem to match the order stored in the Postgres DB. We currently sort the array by the createdAt date in the frontend (selector), but ideally, sorting in the frontend shouldn’t be necessary.

@zackcl
Copy link
Collaborator Author

zackcl commented Aug 13, 2024

@ppratikcr7 @Yagnik56 @RidhamShah Could any of you please take a look at this and see if it can be easily fixed in the backend? Thank you!

@zackcl
Copy link
Collaborator Author

zackcl commented Aug 19, 2024

I tried this again, and it seems the issue doesn't occur consistently. The order isn't always reversed; sometimes, it's randomly rearranged. It would be helpful if there were a way to guarantee the order matches the user's input.

@zackcl
Copy link
Collaborator Author

zackcl commented Sep 4, 2024

@RidhamShah Have you worked on this issue? I can't reproduce the order of the values being reversed for some reason. It seems like the values are now ordered alphabetically after refreshing the page.

Ideally, I think it should always be the same order as what the user originally entered. For example, if the user entered user2, user3, and user1, then this order should remain even after page refresh to avoid confusion.

Maybe we can discuss this in the meeting today.

Agreed. Order of creation should be used, even on refresh.

@zackcl
Copy link
Collaborator Author

zackcl commented Sep 9, 2024

@RidhamShah Just to remind you. We have agreed that the order of creation should be used, even on refresh.

@ppratikcr7 ppratikcr7 self-assigned this Sep 19, 2024
@ppratikcr7
Copy link
Collaborator

@zackcl Yes, I am looking into this issue.

@ppratikcr7
Copy link
Collaborator

@zackcl So, we are storing the complete list elements again on edit and all the list elements will have the same creation time, so maintaining a proper order is not possible until we add some unique identifier numbering during insertion. So we cant return the order from DB as the createdAt time is same for all the list values of a FF. Better we take alphabetical approach and sort users alphabetically and that will be retained on each refresh, something like shown below:

Image

@zackcl
Copy link
Collaborator Author

zackcl commented Sep 23, 2024

OK, let's sort the elements alphabetically for now. I think sorting should be applied whenever the user updates the Values field and loses focus (so that users will see the sorted order before they click "Create" or "Save").

@ppratikcr7
Copy link
Collaborator

ppratikcr7 commented Sep 23, 2024

So, I have it working as discussed, will create a PR soon. But observed a small UI bug where the scroll is not working as expected, when 3rd row of values are added, the top row is half visible. Will create a separate for anyone who wants to take that up.
https://github.com/user-attachments/assets/157597ca-ed69-49da-b76b-aaee23e65023

@ppratikcr7
Copy link
Collaborator

@zackcl Can you please QA this?

@zackcl
Copy link
Collaborator Author

zackcl commented Sep 24, 2024

QA: Passed

@zackcl zackcl moved this from QA to Done in UpGrade Project Sep 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
3 participants