[MS-604] Fix SDK selection for configurations with a mix of restricted and unrestricted SDKs#846
Merged
BurningAXE merged 1 commit intoAug 5, 2024
Conversation
ea345c2 to
1aca64b
Compare
meladRaouf
approved these changes
Aug 1, 2024
Collaborator
|
@BurningAXE I have created this test case to be executed after your PR is merged |
891a1f5 to
bfdfd49
Compare
bfdfd49 to
8530a37
Compare
luhmirin-s
approved these changes
Aug 5, 2024
| * of the project's face and fingerprint SDK configurations. | ||
| * Note that some of these age groups might be unsupported! | ||
| */ | ||
| fun ProjectConfiguration.sortedUniqueAgeGroups(): List<AgeGroup> { |
Contributor
There was a problem hiding this comment.
This function can now be tested directly ;)
…d and unrestricted SDKs
8530a37 to
4c9da8e
Compare
|
luhmirin-s
approved these changes
Aug 5, 2024
meladRaouf
approved these changes
Aug 5, 2024
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in 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.



Previous implementation where AllowedAgeRange for SDKs was optional had issues when the project has a mix of restricted and unrestricted SDKs. I found the cleanest way to solve this to be to make AllowedAgeRange always present. So, when an SDK is unrestricted it will have a range of [0, null) which is the same as the previous null value. This affects mostly conversion from API to domain models and domain models to/from Proto. I don't anticipate any issues but I'm a little nervous about migrations - have I missed anything?
To fix the bug I also changed the implementation of
allowedAgeRanges()andisAgeRestricted()- they will now account for empty age ranges of unrestricted SDKs. I think I've covered it but this might affect any component using these functions so again - have I missed anything?