Add optional additional entities to authz request #25
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Added an option to pass in entities to authorization requests that would not overwrite the configuration already stored in memory, but modify it by adding new entities to it. Should be used if your use case has a large amount of entities that is infeasible to store in memory and you have easy access to all relevant entities when making the authorization request.
omer9564
reviewed
Dec 27, 2023
There was a problem hiding this comment.
It looks good in general.
I understand that you didn't implement tests because this happens in the API layer and we currently only have tests for the service layer.
I do think you can still create tests here,
you can extract the new behavior to a function and create unit test for that function.
besides that, we currently don't have CI to run the test so we run them manually, please attach a screenshot of the successfully running tests after your changes :)
|
Extracted the new behaviour and the existing entities override to a function and added a test. Attached is console output of a test run. Plenty of warnings unrelated to these changes, but all tests pass. |
omer9564
approved these changes
Jan 21, 2024
# for free
to join this conversation on GitHub.
Already have an account?
# to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Added an option to pass in entities to authorization requests that would not overwrite the configuration already stored in memory, but modify it by adding new entities to it.
Should be used if your use case has a large amount of entities that is infeasible to store in memory and you have easy access to all relevant entities when making the authorization request.
Fixes #22
This is my first Rust PR. Open and welcoming to feedback.
Would have added tests if the architecture was set up already. Sadly, for integration tests it is not and unit tests are meaningless here.