qf-knm: quick fixes to KNM (#3801)

* qf-knm: remove number features_allowed/denied from UI response

* remove external features from the top-level default list of allowed features

* qf-knm: add all_features/0 maintenance command

* qf-knm: test features differently

core/kazoo_number_manager/doc/README.md

@@ -113,7 +113,7 @@ One can set the lists in this hierarchical order (if set, one takes precedence o
 1. On the number's reseller account document (`undefined` by default)
 1. In `system_config/number_manager`:
     * if `system_config/number_manager/providers` is set it will be used as the default list of allowed features
-    * otherwise, allows usage of all features by default
+    * otherwise, the defaults of `"features"."allow"` are used
     * Note: local numbers are the only exception (see note below)
 
 Each of these documents can both have a list of features allowed for use and forbidden from use:
@@ -129,13 +129,8 @@ By default, here is what the `system_config/number_manager` document looks like:
        "features": {
            "allow": [
                "carrier_name",
-               "cnam",
-               "e911",
                "failover",
                "force_outbound",
-               "inbound_cnam",
-               "outbound_cnam",
-               "port",
                "prepend",
                "ringback"
            ]

core/kazoo_number_manager/src/kazoo_number_manager_maintenance.erl

@@ -35,6 +35,7 @@
 -export([purge_deleted/0, purge_deleted/1]).
 -export([update_number_services_view/1]).
 
+-export([all_features/0]).
 -export([feature_permissions_on_number/1]).
 -export([add_allowed_feature_on_number/2
         ,remove_allowed_feature_on_number/2
@@ -670,10 +671,13 @@ is_feature_valid(Thing) ->
 %% @private
 -spec invalid_feature(ne_binary()) -> no_return.
 invalid_feature(Feature) ->
-    io:format("Feature '~s' is not a known feature. Known features:\n"
-              "\t~s\n"
-             ,[Feature, list_features(?ALL_KNM_FEATURES)]
-             ),
+    io:format("Feature '~s' is not a known feature.\n", [Feature]),
+    all_features().
+
+%% @public
+-spec all_features() -> no_return.
+all_features() ->
+    io:format("Known features:\n\t~s\n", [list_features(?ALL_KNM_FEATURES)]),
     no_return.
 
 %% @private

core/kazoo_number_manager/src/knm.hrl

@@ -56,7 +56,7 @@
 -define(FEATURES_DENIED_RESELLER(AccountId),
         kapps_account_config:get_from_reseller(AccountId, ?KNM_CONFIG_CAT, ?KEY_FEATURES_DENY)).
 
--define(DEFAULT_FEATURES_ALLOWED_SYSTEM, ?ALL_KNM_FEATURES).
+-define(DEFAULT_FEATURES_ALLOWED_SYSTEM, ?KAZOO_NUMBER_FEATURES ++ ?ADMIN_ONLY_FEATURES).
 -define(FEATURES_ALLOWED_SYSTEM(Default),
         kapps_config:get_ne_binaries(?KNM_CONFIG_CAT, ?KEY_FEATURES_ALLOW, Default)).
 

core/kazoo_number_manager/src/knm_phone_number.erl

@@ -550,8 +550,6 @@ to_public_json(PN) ->
             ,UsedBy
             ,Features
             ,{<<"features_available">>, knm_providers:available_features(PN)}
-            ,{<<"features_allowed">>, features_allowed(PN)}
-            ,{<<"features_denied">>, features_denied(PN)}
             ,{<<"carrier_module">>, ModuleName}
             ])
          ),
@@ -1075,34 +1073,10 @@ remove_denied_feature(PN=#knm_phone_number{features_denied = Denied}, Feature=?N
     end.
 
 -spec features_allowed(knm_phone_number()) -> ne_binaries().
--ifdef(TEST).
-features_allowed(#knm_phone_number{number = ?TEST_TELNYX_NUM}) ->
-    [?FEATURE_CNAM
-    ,?FEATURE_E911
-    ,?FEATURE_FAILOVER
-    ,?FEATURE_FORCE_OUTBOUND
-    ,?FEATURE_PREPEND
-    ,?FEATURE_RINGBACK
-    ,?FEATURE_RENAME_CARRIER
-    ];
-features_allowed(#knm_phone_number{features_allowed = Features}) -> Features.
--else.
 features_allowed(#knm_phone_number{features_allowed = Features}) -> Features.
--endif.
 
 -spec features_denied(knm_phone_number()) -> ne_binaries().
--ifdef(TEST).
-features_denied(#knm_phone_number{number = ?TEST_TELNYX_NUM}) ->
-    [?FEATURE_PORT
-    ,?FEATURE_FAILOVER
-    ];
-features_denied(#knm_phone_number{number = ?BW_EXISTING_DID}) ->
-    [?FEATURE_E911
-    ];
-features_denied(#knm_phone_number{features_denied = Features}) -> Features.
--else.
 features_denied(#knm_phone_number{features_denied = Features}) -> Features.
--endif.
 
 %%--------------------------------------------------------------------
 %% @public

core/kazoo_number_manager/src/providers/knm_providers.erl

@@ -31,13 +31,24 @@
 
 -define(PP(NeBinaries), kz_util:iolist_join($,, NeBinaries)).
 
+-ifdef(TEST).
+-record(feature_parameters, {is_local = false :: boolean()
+                            ,is_admin = false :: boolean()
+                            ,assigned_to :: api_ne_binary()
+                            ,used_by :: api_ne_binary()
+                            ,allowed_features = [] :: ne_binaries()
+                            ,denied_features = [] :: ne_binaries()
+                            ,num :: ne_binary() %% TEST-only
+                            }).
+-else.
 -record(feature_parameters, {is_local = false :: boolean()
                             ,is_admin = false :: boolean()
                             ,assigned_to :: api_ne_binary()
                             ,used_by :: api_ne_binary()
                             ,allowed_features = [] :: ne_binaries()
                             ,denied_features = [] :: ne_binaries()
                             }).
+-endif.
 -type feature_parameters() :: #feature_parameters{}.
 
 %%--------------------------------------------------------------------
@@ -146,6 +157,17 @@ is_local(PN) ->
         orelse lists:member(?FEATURE_LOCAL, knm_phone_number:features_list(PN)).
 
 -spec feature_parameters(knm_phone_number:knm_phone_number()) -> feature_parameters().
+-ifdef(TEST).
+feature_parameters(PhoneNumber) ->
+    FP = feature_parameters(is_local(PhoneNumber)
+                           ,knm_phone_number:is_admin(PhoneNumber)
+                           ,knm_phone_number:assigned_to(PhoneNumber)
+                           ,knm_phone_number:used_by(PhoneNumber)
+                           ,knm_phone_number:features_allowed(PhoneNumber)
+                           ,knm_phone_number:features_denied(PhoneNumber)
+                           ),
+    FP#feature_parameters{num = knm_phone_number:number(PhoneNumber)}.
+-else.
 feature_parameters(PhoneNumber) ->
     feature_parameters(is_local(PhoneNumber)
                       ,knm_phone_number:is_admin(PhoneNumber)
@@ -154,6 +176,7 @@ feature_parameters(PhoneNumber) ->
                       ,knm_phone_number:features_allowed(PhoneNumber)
                       ,knm_phone_number:features_denied(PhoneNumber)
                       ).
+-endif.
 
 -spec feature_parameters(boolean(), boolean(), api_ne_binary(), api_ne_binary(), ne_binaries(), ne_binaries()) -> feature_parameters().
 feature_parameters(IsLocal, IsAdmin, AssignedTo, UsedBy, Allowed, Denied) ->
@@ -193,9 +216,41 @@ system_allowed_features() ->
     Features.
 
 -spec number_allowed_features(feature_parameters()) -> ne_binaries().
+-ifdef(TEST).
+number_allowed_features(#feature_parameters{num = ?TEST_OLD5_1_NUM}) ->
+    AllowedFeatures = [?FEATURE_CNAM
+                      ,?FEATURE_E911
+                      ,?FEATURE_RENAME_CARRIER
+                      ],
+    ?LOG_DEBUG("allowed features set on number document: ~s", [?PP(AllowedFeatures)]),
+    AllowedFeatures;
+number_allowed_features(#feature_parameters{num = ?TEST_VITELITY_NUM}) ->
+    AllowedFeatures = [?FEATURE_CNAM
+                      ,?FEATURE_E911
+                      ,?FEATURE_PREPEND
+                      ,?FEATURE_RENAME_CARRIER
+                      ],
+    ?LOG_DEBUG("allowed features set on number document: ~s", [?PP(AllowedFeatures)]),
+    AllowedFeatures;
+number_allowed_features(#feature_parameters{num = ?TEST_TELNYX_NUM}) ->
+    AllowedFeatures = [?FEATURE_CNAM
+                      ,?FEATURE_E911
+                      ,?FEATURE_FAILOVER
+                      ,?FEATURE_FORCE_OUTBOUND
+                      ,?FEATURE_PREPEND
+                      ,?FEATURE_RINGBACK
+                      ,?FEATURE_RENAME_CARRIER
+                      ],
+    ?LOG_DEBUG("allowed features set on number document: ~s", [?PP(AllowedFeatures)]),
+    AllowedFeatures;
+number_allowed_features(#feature_parameters{allowed_features = AllowedFeatures}) ->
+    ?LOG_DEBUG("allowed features set on number document: ~s", [?PP(AllowedFeatures)]),
+    AllowedFeatures.
+-else.
 number_allowed_features(#feature_parameters{allowed_features = AllowedFeatures}) ->
     ?LOG_DEBUG("allowed features set on number document: ~s", [?PP(AllowedFeatures)]),
     AllowedFeatures.
+-endif.
 
 -spec list_denied_features(feature_parameters()) -> ne_binaries().
 list_denied_features(Parameters) ->
@@ -241,9 +296,26 @@ used_by_denied_features(#feature_parameters{used_by = UsedBy}) ->
     Features.
 
 -spec number_denied_features(feature_parameters()) -> ne_binaries().
+-ifdef(TEST).
+number_denied_features(#feature_parameters{num = ?TEST_TELNYX_NUM}) ->
+    DeniedFeatures = [?FEATURE_PORT
+                     ,?FEATURE_FAILOVER
+                     ],
+    ?LOG_DEBUG("denied features set on number document: ~s", [?PP(DeniedFeatures)]),
+    DeniedFeatures;
+number_denied_features(#feature_parameters{num = ?BW_EXISTING_DID}) ->
+    DeniedFeatures = [?FEATURE_E911
+                     ],
+    ?LOG_DEBUG("denied features set on number document: ~s", [?PP(DeniedFeatures)]),
+    DeniedFeatures;
 number_denied_features(#feature_parameters{denied_features = DeniedFeatures}) ->
     ?LOG_DEBUG("denied features set on number document: ~s", [?PP(DeniedFeatures)]),
     DeniedFeatures.
+-else.
+number_denied_features(#feature_parameters{denied_features = DeniedFeatures}) ->
+    ?LOG_DEBUG("denied features set on number document: ~s", [?PP(DeniedFeatures)]),
+    DeniedFeatures.
+-endif.
 
 -spec maybe_deny_admin_only_features(feature_parameters()) -> ne_binaries().
 maybe_deny_admin_only_features(#feature_parameters{is_admin = true}) -> [];
@@ -279,7 +351,7 @@ requested_modules(Number) ->
                         ],
     ?LOG_DEBUG("asked on public fields: ~s", [?PP(RequestedFeatures)]),
     ExistingFeatures = knm_phone_number:features_list(PhoneNumber),
-    ?LOG_DEBUG("already allowed: ~s", [?PP(ExistingFeatures)]),
+    ?LOG_DEBUG("previously allowed: ~s", [?PP(ExistingFeatures)]),
     %% ?FEATURE_LOCAL is never user-writable thus must not be included.
     Features = (RequestedFeatures ++ ExistingFeatures) -- [?FEATURE_LOCAL],
     provider_modules(Features, AccountId).