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

fix: Correct default_order for not case-sensitive StringBones #1165

Closed

Conversation

sveneberth
Copy link
Member

Without the .idx suffix an empty result is returned

Without the `.idx` suffix an empty result is returned
@sveneberth sveneberth added bug(fix) Something isn't working or address a specific issue or vulnerability Priority: High After critical issues are fixed, these should be dealt with before any further issues. labels May 17, 2024
@sveneberth sveneberth added this to the ViUR-core v3.6 milestone May 17, 2024
Copy link
Member

@phorward phorward left a comment

Choose a reason for hiding this comment

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

Hm, indeed this is a possible fix, but on the other side the order setting should be generally handled by the bones, and not in the prototypes, like in BaseBone.buildDBSort.

There's already pull request #1077 on this, which can be useful. Maybe it would be better to allow for a dict here as well, which is being fed to Query.mergeExternalFilter(), which then handles the ordering correctly?

@sveneberth
Copy link
Member Author

Hm, indeed this is a possible fix, but on the other side the order setting should be generally handled by the bones, and not in the prototypes, like in BaseBone.buildDBSort.

Yeah, absolutely. After I found this bug I thought the same thing. But after looking at buildDBSort, I saw this only works on queries, we don't have here. Therefore, it was too much work for a hotfix. I'd appreciate to use the help of the bones, but for now let's go with this fix to get things working.

phorward added a commit to phorward/viur-core that referenced this pull request May 23, 2024
- Also allows for dict-style filter definitions as default_order
- Alternative fix for viur-framework#1165
@phorward
Copy link
Member

But after looking at buildDBSort, I saw this only works on queries, we don't have here.

I've made an alternative proposal for this PR in #1169. It's not perfect, but uses the provided facilities.

Furthermore, #1077 was updated and tested now, and I would like to merge it into develop.

@sveneberth
Copy link
Member Author

Replaced by #1169

@sveneberth sveneberth closed this May 24, 2024
phorward added a commit that referenced this pull request May 24, 2024
- Also allows for dict-style filter definitions as default_order
- Alternative fix for #1165
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug(fix) Something isn't working or address a specific issue or vulnerability Priority: High After critical issues are fixed, these should be dealt with before any further issues.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants