KAZOO-6099: simplify annotate to fix last object removal quantity bug (…

…2600hz#5776)

* KAZOO-6099: simplify annotate to fix last object removal quanity bug

* fix dialyzer

* fmt

core/kazoo_documents/src/kzd_activation_item.erl

@@ -39,8 +39,8 @@ billable(Doc, Default) ->
     kz_json:get_integer_value([<<"billable">>], Doc, Default).
 
 -spec set_billable(doc(), non_neg_integer()) -> doc().
-set_billable(Doc, Category) ->
-    kz_json:set_value([<<"billable">>], Doc, Category).
+set_billable(Doc, Billable) ->
+    kz_json:set_value([<<"billable">>], Billable, Doc).
 
 %%------------------------------------------------------------------------------
 %% @doc
@@ -56,7 +56,7 @@ category(Doc, Default) ->
 
 -spec set_category(doc(), kz_term:api_ne_binary()) -> doc().
 set_category(Doc, Category) ->
-    kz_json:set_value([<<"category">>], Doc, Category).
+    kz_json:set_value([<<"category">>], Category, Doc).
 
 %%------------------------------------------------------------------------------
 %% @doc
@@ -72,7 +72,7 @@ item(Doc, Default) ->
 
 -spec set_item(doc(), kz_term:api_ne_binary()) -> doc().
 set_item(Doc, Item) ->
-    kz_json:set_value([<<"item">>], Doc, Item).
+    kz_json:set_value([<<"item">>], Item, Doc).
 
 %%------------------------------------------------------------------------------
 %% @doc
@@ -88,7 +88,7 @@ name(Doc, Default) ->
 
 -spec set_name(doc(), kz_term:api_ne_binary()) -> doc().
 set_name(Doc, Name) ->
-    kz_json:set_value([<<"name">>], Doc, Name).
+    kz_json:set_value([<<"name">>], Name, Doc).
 
 %%------------------------------------------------------------------------------
 %% @doc
@@ -104,7 +104,7 @@ rate(Doc, Default) ->
 
 -spec set_rate(doc(), kz_term:api_float()) -> doc().
 set_rate(Doc, Rate) ->
-    kz_json:set_value([<<"rate">>], Doc, Rate).
+    kz_json:set_value([<<"rate">>], Rate, Doc).
 
 %%------------------------------------------------------------------------------
 %% @doc
@@ -120,4 +120,4 @@ total(Doc, Default) ->
 
 -spec set_total(doc(), kz_term:api_float()) -> doc().
 set_total(Doc, Total) ->
-    kz_json:set_value([<<"total">>], Doc, Total).
+    kz_json:set_value([<<"total">>], Total, Doc).

core/kazoo_documents/src/kzd_item_plan.erl

@@ -177,7 +177,7 @@ prorate(ItemPlan) ->
         'undefined' ->
             kz_json:from_list(
               [{<<"additions">>, 'true'}
-              ,{<<"removals">>, 'false'}
+              ,{<<"removals">>, 'true'}
               ]
              );
         Else -> Else
@@ -197,7 +197,7 @@ prorate_additions(ItemPlan, Default) ->
 
 -spec prorate_removals(doc()) -> boolean().
 prorate_removals(ItemPlan) ->
-    prorate_removals(ItemPlan, 'false').
+    prorate_removals(ItemPlan, 'true').
 
 -spec prorate_removals(doc(), Default) -> boolean() | Default.
 prorate_removals(ItemPlan, Default) ->

core/kazoo_services/src/kz_services_invoice.erl

@@ -172,7 +172,9 @@ setters(Invoice, Routines) ->
 public_json(Invoice) ->
     %% TODO: calculate totals (taxes)...
     ItemsJObjs = kz_services_items:public_json(items(Invoice)),
-    ActivationCharges = kz_services_activation_items:public_json(activation_charges(Invoice)),
+    ActivationCharges = kz_services_activation_items:public_json(
+                          activation_charges(Invoice)
+                         ),
     PlanJObj = kz_services_plan:jobj(plan(Invoice)),
     Props = [{<<"activation_charges">>, ActivationCharges}
             ,{<<"items">>, ItemsJObjs}

core/kazoo_services/src/kz_services_invoices.erl

@@ -46,55 +46,77 @@ create(Services) ->
 
 -spec annotate(kz_services:services(), invoices(), invoices()) -> invoices().
 annotate(Services, CurrentInvoices, ProposedInvoices) ->
-    annotate(Services, CurrentInvoices, ProposedInvoices, []).
-
--spec annotate(kz_services:services(), invoices(), invoices(), invoices()) -> invoices().
-annotate(_Services, [], [], Invoices) -> Invoices;
-annotate(Services, [CurrentInvoice|CurrentInvoices], [], Invoices) ->
-    %% A bookkeeper was removed from the plan?
-    CurrentItems = kz_services_items:reset(
-                     kz_services_invoice:items(CurrentInvoice)
-                    ),
-    ProposedItems = kz_services_items:empty(),
+    Dict =
+        foldl(fun(Invoice, D) ->
+                      Key = kz_services_invoice:bookkeeper_hash(Invoice),
+                      dict:store(Key, {Invoice, 'undefined'}, D)
+              end
+             ,dict:new()
+             ,CurrentInvoices
+             ),
+    TentativeInvoices =
+        dict:to_list(
+          foldl(fun(Invoice, D) ->
+                        Key = kz_services_invoice:bookkeeper_hash(Invoice),
+                        dict:update(Key
+                                   ,fun({Current, _}) ->
+                                            {Current, Invoice}
+                                    end
+                                   ,{'undefined', Invoice}
+                                   ,D
+                                   )
+                end
+               ,Dict
+               ,ProposedInvoices
+               )
+         ),
+    do_annotate(Services, [Invoices || {_BookkeeperHash, Invoices} <- TentativeInvoices], []).
+
+-type tentative_invoices() :: [{kz_services_invoice:invoice() | 'undefined', kz_services_invoice:invoice() | 'undefined'}].
+-spec do_annotate(kz_services:services(), tentative_invoices(), invoices()) -> invoices().
+do_annotate(_Services, [], Invoices) -> Invoices;
+do_annotate(Services, [{CurrentInvoice, 'undefined'}|TentativeInvoices], Invoices) ->
+    %% All plans associated with a bookkeeper was removed
+    CurrentItems = kz_services_invoice:items(CurrentInvoice),
+    ProposedItems = kz_services_items:reset(CurrentItems),
     Items = kz_services_items:annotate(CurrentItems, ProposedItems),
     Setters = [{fun kz_services_invoice:set_items/2, Items}
-              ,{fun kz_services_invoice:set_activation_charges/2, kz_services_activation_items:empty()}
+              ,{fun kz_services_invoice:set_activation_charges/2
+               ,kz_services_activation_items:empty()
+               }
               ],
-    annotate(Services
-            ,CurrentInvoices
-            ,[]
-            ,[kz_services_invoice:setters(CurrentInvoice, Setters)|Invoices]
-            );
-annotate(Services, CurrentInvoices, [ProposedInvoice|ProposedInvoices], Invoices) ->
-    BookkeeperHash = kz_services_invoice:bookkeeper_hash(ProposedInvoice),
+    do_annotate(Services
+               ,TentativeInvoices
+               ,[kz_services_invoice:setters(CurrentInvoice, Setters)|Invoices]
+               );
+do_annotate(Services, [{'undefined', ProposedInvoice}|TentativeInvoices], Invoices) ->
+    %% A plan(s) associated with a new bookkeeper was added
     ProposedItems = kz_services_invoice:items(ProposedInvoice),
-    case split_invoices(CurrentInvoices, BookkeeperHash) of
-        {'undefined', RemainingCurrentInvoices} ->
-            %% this is the first time this bookkeeper is appeared in the plan
-            CurrentItems = kz_services_items:empty(),
-            Items = kz_services_items:annotate(CurrentItems, ProposedItems),
-            ActivationItems = kz_services_activation_items:create(Items),
-            Setters = [{fun kz_services_invoice:set_items/2, Items}
-                      ,{fun kz_services_invoice:set_activation_charges/2, ActivationItems}
-                      ],
-            annotate(Services
-                    ,RemainingCurrentInvoices
-                    ,ProposedInvoices
-                    ,[kz_services_invoice:setters(ProposedInvoice, Setters)|Invoices]
-                    );
-        {CurrentInvoice, RemainingCurrentInvoices} ->
-            CurrentItems = kz_services_invoice:items(CurrentInvoice),
-            Items = kz_services_items:annotate(CurrentItems, ProposedItems),
-            ActivationItems = kz_services_activation_items:create(Items),
-            Setters = [{fun kz_services_invoice:set_items/2, Items}
-                      ,{fun kz_services_invoice:set_activation_charges/2, ActivationItems}
-                      ],
-            annotate(Services
-                    ,RemainingCurrentInvoices
-                    ,ProposedInvoices
-                    ,[kz_services_invoice:setters(ProposedInvoice, Setters)|Invoices]
-                    )
-    end.
+    CurrentItems = kz_services_items:reset(ProposedItems),
+    Items = kz_services_items:annotate(CurrentItems, ProposedItems),
+    ActivationItems = kz_services_activation_items:create(Items),
+    Setters = [{fun kz_services_invoice:set_items/2, Items}
+              ,{fun kz_services_invoice:set_activation_charges/2
+               ,kz_services_activation_items:create(ActivationItems)
+               }
+              ],
+    do_annotate(Services
+               ,TentativeInvoices
+               ,[kz_services_invoice:setters(ProposedInvoice, Setters)|Invoices]
+               );
+do_annotate(Services, [{CurrentInvoice, ProposedInvoice}|TentativeInvoices], Invoices) ->
+    CurrentItems = kz_services_invoice:items(CurrentInvoice),
+    ProposedItems = kz_services_invoice:items(ProposedInvoice),
+    Items = kz_services_items:annotate(CurrentItems, ProposedItems),
+    Setters = [{fun kz_services_invoice:set_items/2, Items}
+              ,{fun kz_services_invoice:set_activation_charges/2
+               ,kz_services_activation_items:create(Items)
+               }
+              ],
+    do_annotate(Services
+               ,TentativeInvoices
+               ,[kz_services_invoice:setters(ProposedInvoice, Setters)|Invoices]
+               ).
 
 -spec create_current_invoices(kz_services:services()) -> invoices().
 create_current_invoices(Services) ->
@@ -127,19 +149,6 @@ reset(Services) ->
                ],
     lists:foldl(fun(F, S) -> F(S) end, Services, Routines).
 
--type api_invoice() :: kz_serivces_invoice:invoice() | 'undefined'.
--spec split_invoices(invoices(), kz_term:ne_binary()) -> {api_invoice(), invoices()}.
-split_invoices(Invoices, BookkeeperHash) ->
-    case lists:splitwith(fun(Invoice) ->
-                                 kz_services_invoice:bookkeeper_hash(Invoice) =:= BookkeeperHash
-                         end, Invoices)
-    of
-        {[], RemainingInvoices} ->
-            {'undefined', RemainingInvoices};
-        {[Invoice], RemainingInvoices} ->
-            {Invoice, RemainingInvoices}
-    end.
-
 %%------------------------------------------------------------------------------
 %% @doc
 %% @end

core/kazoo_services/src/kz_services_item.erl

@@ -62,6 +62,7 @@
 -export([has_changes/1]).
 -export([has_additions/1]).
 -export([has_billable_additions/1]).
+-export([hash/1]).
 
 -include("services.hrl").
 
@@ -773,3 +774,16 @@ has_billable_additions(Item) ->
             Key = [<<"difference">>, <<"billable">>],
             kz_json:get_integer_value(Key, Changes, 0) > 0
     end.
+
+%%------------------------------------------------------------------------------
+%% @doc
+%% @end
+%%------------------------------------------------------------------------------
+-spec hash(item()) -> kz_term:binary().
+hash(Item) ->
+    kz_binary:md5(
+      <<(category_name(Item))/binary
+       ,(item_name(Item))/binary
+       ,(kz_term:to_binary(is_masquerading(Item)))/binary
+      >>
+     ).

core/kazoo_services/src/kz_services_items.erl

@@ -88,12 +88,6 @@ get_plan_items(_Services, Plan) ->
      || CategoryName <- kzd_service_plan:categories(PlanJObj),
         ItemName <- kzd_service_plan:items(PlanJObj, CategoryName)
     ].
-%% TODO: figure out how to handle the billing object, used to be that it would
-%%   result in items regardless of what was on the service plan...
-%%    [{CategoryName, ItemName}
-%%     || CategoryName <- kz_services:list_categories(Services),
-%%        ItemName <- kz_services:list_items(Services, CategoryName)
-%%    ].
 
 %%------------------------------------------------------------------------------
 %% @doc
@@ -192,25 +186,50 @@ annotate(CurrentItems, ProposedItems) ->
 %%------------------------------------------------------------------------------
 -spec annotate(items(), items(), kz_term:api_binary()) -> items().
 annotate(CurrentItems, ProposedItems, Reason) ->
-    annotate(CurrentItems, ProposedItems, Reason, []).
+    Dict =
+        foldl(fun(Item, D) ->
+                      Key = kz_services_item:hash(Item),
+                      dict:store(Key, {Item, 'undefined'}, D)
+              end
+             ,dict:new()
+             ,CurrentItems
+             ),
+    TentativeItems =
+        dict:to_list(
+          foldl(fun(Item, D) ->
+                        Key = kz_services_item:hash(Item),
+                        dict:update(Key
+                                   ,fun({Current, _}) ->
+                                            {Current, Item}
+                                    end
+                                   ,{'undefined', Item}
+                                   ,D
+                                   )
+                end
+               ,Dict
+               ,ProposedItems
+               )
+         ),
+    do_annotate([Items || {_ItemHash, Items} <- TentativeItems], Reason, []).
 
--spec annotate(items(), items(), kz_term:api_binary(), items()) -> items().
-annotate([], [], _Reason, Items) -> Items;
-annotate([CurrentItem|CurrentItems], [], Reason, Items) ->
-    ProposedItem = kz_services_item:empty(),
+-type tentative_items() :: [{kz_services_item:item() | 'undefined', kz_services_item:item() | 'undefined'}].
+-spec do_annotate(tentative_items(), kz_term:api_bianry(), items()) -> items().
+do_annotate([], _Reason, Items) ->
+    Items;
+do_annotate([{CurrentItem, 'undefined'}|TentativeItems], Reason, Items) ->
+    ProposedItem = kz_services_item:reset(CurrentItem),
     Difference = difference(CurrentItem, ProposedItem),
-    Item = maybe_annotate(<<"removed">>, Difference, Reason, CurrentItem),
-    annotate(CurrentItems, [], Reason, [Item|Items]);
-annotate(CurrentItems, [ProposedItem|ProposedItems], Reason, Items) ->
-    case split_items(CurrentItems, ProposedItem) of
-        {'undefined', RemainingCurrentItems} ->
-            Item = maybe_annotate(<<"created">>, 'undefined', Reason, ProposedItem),
-            annotate(RemainingCurrentItems, ProposedItems, Reason, [Item|Items]);
-        {CurrentItem, RemainingCurrentItems} ->
-            Difference = difference(CurrentItem, ProposedItem),
-            Item = maybe_annotate(<<"modified">>, Difference, Reason, ProposedItem),
-            annotate(RemainingCurrentItems, ProposedItems, Reason, [Item|Items])
-    end.
+    Item = maybe_annotate(<<"removed">>, Difference, Reason, ProposedItem),
+    do_annotate(TentativeItems, Reason, [Item|Items]);
+do_annotate([{'undefined', ProposedItem}|TentativeItems], Reason, Items) ->
+    CurrentItem = kz_services_item:reset(ProposedItem),
+    Difference = difference(CurrentItem, ProposedItem),
+    Item = maybe_annotate(<<"created">>, Difference, Reason, ProposedItem),
+    do_annotate(TentativeItems, Reason, [Item|Items]);
+do_annotate([{CurrentItem, ProposedItem}|TentativeItems], Reason, Items) ->
+    Difference = difference(CurrentItem, ProposedItem),
+    Item = maybe_annotate(<<"modified">>, Difference, Reason, ProposedItem),
+    do_annotate(TentativeItems, Reason, [Item|Items]).
 
 -spec maybe_annotate(kz_term:ne_binary(), kz_term:api_object(), kz_term:api_binary(), kz_services_item:item()) ->
                             kz_services_item:item().
@@ -233,26 +252,6 @@ maybe_annotate(Type, Difference, Reason, Item) ->
             kz_services_item:set_changes(Item, kz_json:from_list(Changes))
     end.
 
--type api_item() :: kz_serivces_item:item() | 'undefined'.
--spec split_items(items(), kz_services_item:item()) -> {api_item(), items()}.
-split_items(Items, ProposedItem) ->
-    CategoryName = kz_services_item:category_name(ProposedItem),
-    ItemName = kz_services_item:item_name(ProposedItem),
-    Masqueraded = kz_services_item:is_masquerading(ProposedItem),
-    case lists:splitwith(fun(Item) ->
-                                 kz_services_item:category_name(Item) =:= CategoryName
-                                     andalso
-                                     kz_services_item:item_name(Item) =:= ItemName
-                                     andalso
-                                     kz_services_item:is_masquerading(Item) =:= Masqueraded
-                         end, Items)
-    of
-        {[], RemainingItems} ->
-            {'undefined', RemainingItems};
-        {[Item], RemainingItems} ->
-            {Item, RemainingItems}
-    end.
-
 %%------------------------------------------------------------------------------
 %% @doc Compares two service items, returning the difference
 %% @end