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

Authorize uuid for existing object in sortable table #4287

Merged

Conversation

julienanne
Copy link
Contributor

@julienanne julienanne commented Mar 1, 2022

Description

In the backend section, from Ref #3591 we can't sort object list when we use uuid for the object's id.
We need to allow objId to be an uuid.
This change need to be backported until the version 2.11

Hope it can help.

PR v2.11 : #4299
PR v3.0 : #4300
PR v3.1 : #4301

Checklist:

  • I have followed Pull Request guidelines
  • I have added a detailed description into each commit message
    - [ ] I have updated Guides and README accordingly to this change (if needed)
    - [ ] I have added tests to cover this change (if needed) On this point i have encoutered many issues with teaspoon on docker and i don't see existing tests about that part of the backend.
    - [ ] I have attached screenshots to this PR for visual changes (if needed)

@julienanne
Copy link
Contributor Author

The test marked as failed is a flaky test but i can't rerun the workflow from start ;)

Copy link
Member

@kennyadsl kennyadsl left a comment

Choose a reason for hiding this comment

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

Thanks, if you need this in 2.11, can you please backport this into 2.11, 3.0 and 3.1 branches as well with different PRs?

Also, non-blocking but do you think there's a way to add some specs around this behavior? Just wanted to be sure we won't be removing this feature in the future without noticing.

@julienanne
Copy link
Contributor Author

Thanks, if you need this in 2.11, can you please backport this into 2.11, 3.0 and 3.1 branches as well with different PRs?

Also, non-blocking but do you think there's a way to add some specs around this behavior? Just wanted to be sure we won't be removing this feature in the future without noticing.

Ok I open the three PRs one for each version with the same content.
For the testing part I don't know, if I have more time I will check if I can do it in another PRs.
I noticed that you also have a big discussion about uuid on this issue #2807 (#2807), maybe this PR can be part of that discussion, maybe it is not the only breaking changes on UI with uuid ?
Thanks for approval.

Copy link
Member

@spaghetticode spaghetticode left a comment

Choose a reason for hiding this comment

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

@julienanne thank you!

@kennyadsl kennyadsl merged commit 547d4ea into solidusio:master Apr 19, 2022
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.

3 participants