Skip to content

Use exposed names in Request Context OrderByColumns#690

Merged
aaronburtle merged 18 commits into
mainfrom
dev/aaronburtle/OrderByBackingColumnFix
Aug 23, 2022
Merged

Use exposed names in Request Context OrderByColumns#690
aaronburtle merged 18 commits into
mainfrom
dev/aaronburtle/OrderByBackingColumnFix

Conversation

@aaronburtle

Copy link
Copy Markdown
Contributor

Bugfix associated with this issue: #650

Authorization presumes we have exposed names in the request context. Currently we have backing column names in the OrderByColumns that are created for the request context. This change modifies the request parser so that we use the exposed names, allowing authorization to handle those columns without issue. We then add in a helper function to convert the exposed names of the columns to backing column names later when we form the sql query structure.

@aaronburtle aaronburtle self-assigned this Aug 11, 2022
@aaronburtle aaronburtle added the bug Something isn't working label Aug 11, 2022
Comment thread src/Service/Resolvers/Sql Query Structures/SqlQueryStructure.cs Outdated
@jarupatj

Copy link
Copy Markdown
Contributor

Need to have tests for this scenario.

Comment thread src/Service/Parsers/RequestParser.cs Outdated
Comment thread src/Service/Resolvers/Sql Query Structures/SqlQueryStructure.cs Outdated
Comment thread src/Service/Resolvers/Sql Query Structures/SqlQueryStructure.cs
Comment thread src/Service/Parsers/RequestParser.cs Outdated
Comment thread src/Service/Parsers/RequestParser.cs Outdated
Comment thread src/Service/Resolvers/Sql Query Structures/SqlQueryStructure.cs Outdated
Comment thread src/Service/Resolvers/Sql Query Structures/SqlQueryStructure.cs Outdated
Comment thread src/Service.Tests/SqlTests/RestApiTests/Find/PostgreSqlFindApiTests.cs Outdated
Comment thread src/Service/Parsers/RequestParser.cs
Comment thread src/Service/Parsers/RequestParser.cs Outdated
Comment thread src/Service/Parsers/RequestParser.cs

@Aniruddh25 Aniruddh25 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@seantleonard seantleonard self-requested a review August 23, 2022 01:01

@ayush3797 ayush3797 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

@aaronburtle aaronburtle dismissed jarupatj’s stale review August 23, 2022 06:04

Stale request, addressed changes, PR is blocking ayush

@aaronburtle aaronburtle merged commit 9c02112 into main Aug 23, 2022
@aaronburtle aaronburtle deleted the dev/aaronburtle/OrderByBackingColumnFix branch August 23, 2022 06:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OrderByClauseInUrl contain backing column names leading to failure at Column Authorization stage

5 participants