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: allow using boolean as values in tenant configs when getting settings by organization #197

Merged
merged 1 commit into from
Feb 29, 2024

Conversation

bra-i-am
Copy link
Contributor

@bra-i-am bra-i-am commented Feb 22, 2024

Description

This PR is created for a necessity arisen in the PR #808 from edunext-platform and allows using boolean types as values because when some tenant properties have a value False or True, this is used to run two conditionals. e.g. if a tenant setting ENABLE_COURSE_AUTHORING_MFE has a value False, even though we want to send the value False, the conditional is skipped and is returned a None instead of False

Testing instructions

At this moment the useful way to test this PR would be running the PR #808 from edunext-platform

Copy link
Contributor

@MaferMazu MaferMazu left a comment

Choose a reason for hiding this comment

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

This looks good to me ✨
Please rebase to ensure the quality test is still passing!

@bra-i-am bra-i-am force-pushed the bc/fix-tenant-values-handler branch from ed8f10a to b4bb9d3 Compare February 29, 2024 18:51
@bra-i-am bra-i-am changed the title fix: allow boolean types as values fix: allow use boolean as values in tenant configs when getting settings by organization Feb 29, 2024
@bra-i-am bra-i-am changed the title fix: allow use boolean as values in tenant configs when getting settings by organization fix: allow using boolean as values in tenant configs when getting settings by organization Feb 29, 2024
@bra-i-am bra-i-am merged commit 898a4cd into master Feb 29, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants