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 inconsistent tenantId format #2357

Closed
wants to merge 1 commit into from
Closed

Conversation

goxiaoy
Copy link
Contributor

@goxiaoy goxiaoy commented Dec 10, 2019

In module FeatureManager, file TenantFeatureManagerExtensions ToString() is used instead of ToString("N")
Incosistent format will cause TenantFeatureValueProvider always get the incorrect value

public static class TenantFeatureManagerExtensions
    {
        public static Task<string> GetOrNullForTenantAsync(this IFeatureManager featureManager, [NotNull] string name, Guid tenantId, bool fallback = true)
        {
            return featureManager.GetOrNullAsync(name, TenantFeatureValueProvider.ProviderName, tenantId.ToString(), fallback);
        }

        public static Task<List<FeatureNameValue>> GetAllForTenantAsync(this IFeatureManager featureManager, Guid tenantId, bool fallback = true)
        {
            return featureManager.GetAllAsync(TenantFeatureValueProvider.ProviderName, tenantId.ToString(), fallback);
        }

        public static Task SetForTenantAsync(this IFeatureManager featureManager, Guid tenantId, [NotNull] string name, [CanBeNull] string value, bool forceToSet = false)
        {
            return featureManager.SetAsync(name, value, TenantFeatureValueProvider.ProviderName, tenantId.ToString(), forceToSet);
        }
    }

@maliming
Copy link
Member

@hikalkan
The feature-management module also has inconsistencies,
But I didn't find the code using ToString (" N ") in the setting module.

@hikalkan hikalkan added this to the 1.1.1 milestone Dec 10, 2019
@hikalkan
Copy link
Member

@goxiaoy you only changed in one place. What about others?
@maliming can you check all and fix the problem completely if there is a problem.
We will probably release v1.1.1 tomorrow.

@goxiaoy
Copy link
Contributor Author

goxiaoy commented Dec 10, 2019

I am going to close this pr. I have not noticed that there are many places using this format and I think it’s better for your guys to decide which one is recommended.

@goxiaoy goxiaoy closed this Dec 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants