Merge pull request #3630 from esl/inbox/flaky_fixes

Inbox flaky tests fixes

big_tests/tests/inbox_SUITE.erl

@@ -569,14 +569,13 @@ check_total_unread_count_and_active_conv_count(Config) ->
 check_total_unread_count_when_there_are_no_active_conversations(Config) ->
     escalus:fresh_story(Config, [{kate, 1}, {mike, 1}], fun(Kate, Mike) ->
         inbox_helper:send_and_mark_msg(Kate, Mike),
-
         inbox_helper:get_inbox(Mike, #{count => 1, unread_messages => 0, active_conversations => 0})
-  end).
+    end).
 
 total_unread_count_and_active_convs_are_zero_at_no_activity(Config) ->
     escalus:fresh_story(Config, [{kate, 1}], fun(Kate) ->
         inbox_helper:get_inbox(Kate, #{count => 0, unread_messages => 0, active_conversations => 0})
-                                                end).
+    end).
 
 try_to_reset_unread_counter_with_bad_marker(Config) ->
     escalus:fresh_story(Config, [{kate, 1}, {mike, 1}], fun(Kate, Mike) ->
@@ -831,7 +830,7 @@ leave_and_remove_conversation(Config) ->
         check_inbox(Bob, [], #{box => inbox}),
         if_async_check_bin(Config, Bob, [#conv{unread = 2, from = RoomJid, to = BobJid,
                                                verify = fun inbox_helper:verify_is_none_aff_change/2}])
-      end).
+    end).
 
 %% this test combines options:
 %% ...

big_tests/tests/inbox_extensions_SUITE.erl

@@ -70,6 +70,7 @@ groups() ->
         % box
         box_move_to_other_works_successfully,
         box_active_entry_gets_archived,
+        box_other_entry_does_not_get_unarchived,
         box_archived_entry_gets_active_on_request,
         box_archived_entry_gets_active_for_the_sender_on_new_message,
         box_archived_entry_gets_active_for_the_receiver_on_new_message,
@@ -312,6 +313,23 @@ box_active_entry_gets_archived(Config) ->
                                  #{box => archive})
     end).
 
+box_other_entry_does_not_get_unarchived(Config) ->
+    escalus:fresh_story(Config, [{alice, 1}, {bob, 1}], fun(Alice, Bob) ->
+        % Alice sends a message to Bob and then she moves the conversation to the 'other' box
+        Body1 = <<"Hi Bob">>,
+        inbox_helper:send_msg(Alice, Bob, Body1),
+        inbox_helper:check_inbox(Alice, [#conv{unread = 0, from = Alice, to = Bob, content = Body1}]),
+        set_inbox_properties(Alice, Bob, [{box, other}]),
+        % But then Alice keeps writing
+        Body2 = <<"Hi Bob again">>,
+        inbox_helper:send_msg(Alice, Bob, Body2),
+        inbox_helper:get_inbox(Alice, #{box => all, hidden_read => false}, #{count => 1}),
+        % Then the conversation is still in the same box
+        inbox_helper:check_inbox(Alice, [], #{box => inbox}),
+        inbox_helper:check_inbox(Alice, [#conv{unread = 0, from = Alice, to = Bob, content = Body2}],
+                                 #{box => other})
+    end).
+
 box_archived_entry_gets_active_on_request(Config) ->
     escalus:fresh_story(Config, [{alice, 1}, {bob, 1}], fun(Alice, Bob) ->
         % Alice sends a message to Bob and Bob archives it immediately

src/inbox/mod_inbox_rdbms.erl

@@ -67,20 +67,23 @@ init(HostType, _Options) ->
     mongoose_rdbms:prepare(inbox_delete_domain, inbox,
                            [lserver], <<"DELETE FROM inbox WHERE lserver = ?">>),
     % upserts
+    BoxQuery = <<"CASE WHEN ?='bin' THEN 'bin'",
+                     " WHEN inbox.box='archive' THEN 'inbox'",
+                     " ELSE inbox.box END">>,
     UniqueKeyFields = [<<"luser">>, <<"lserver">>, <<"remote_bare_jid">>],
     InsertFields = UniqueKeyFields ++ [<<"msg_id">>, <<"box">>, <<"content">>, <<"unread_count">>, <<"timestamp">>],
     rdbms_queries:prepare_upsert(HostType, inbox_upsert, inbox,
                                  InsertFields,
                                  [<<"msg_id">>,
-                                  {expression, <<"box">>, <<"CASE WHEN inbox.box='archive' THEN ? ELSE inbox.box END">>},
+                                  {expression, <<"box">>, BoxQuery},
                                   <<"content">>,
                                   <<"unread_count">>,
                                   <<"timestamp">>],
                                  UniqueKeyFields, <<"timestamp">>),
     rdbms_queries:prepare_upsert(HostType, inbox_upsert_incr_unread, inbox,
                                  InsertFields,
                                  [<<"msg_id">>,
-                                  {expression, <<"box">>, <<"CASE WHEN inbox.box='archive' THEN ? ELSE inbox.box END">>},
+                                  {expression, <<"box">>, BoxQuery},
                                   <<"content">>,
                                   {expression, <<"unread_count">>, <<"inbox.unread_count + ?">>},
                                   <<"timestamp">>],

src/inbox/mod_inbox_rdbms_async.erl

@@ -174,23 +174,15 @@ empty_global_bin(HostType, TS) ->
     mod_inbox_rdbms:empty_global_bin(HostType, TS).
 
 -spec aggregate(CurrentlyAccumulatedTask :: task(), NewTask :: task()) -> FinalTask :: task().
-%%% First task being processed, just take that one
-aggregate(undefined, Task) ->
-    Task;
-
 %%% if new task is remove_row, do the previous with an updated box
-    % {reset_unread, mod_inbox:entry_key(), id(), integer()}.
-aggregate({reset_unread, _, _, _},
-          {remove_inbox_row, _} = OldTask) ->
-    OldTask;
 aggregate({set_inbox, Entry, Content, Count, MsgId, Timestamp, _},
           {remove_inbox_row, _}) ->
     {set_inbox, Entry, Content, Count, MsgId, Timestamp, <<"bin">>};
 aggregate({set_inbox_incr_unread, Entry, Content, MsgId, Timestamp, Incrs, _},
           {remove_inbox_row, _}) ->
     {set_inbox_incr_unread, Entry, Content, MsgId, Timestamp, Incrs, <<"bin">>};
-aggregate(_, {remove_inbox_row, _} = OldTask) ->
-    OldTask;
+aggregate(_, {remove_inbox_row, _} = NewTask) ->
+    NewTask;
 
 %%% if the last task was remove_row, this task should now only be an insert
 aggregate({remove_inbox_row, _} = OldTask, {reset_unread, _, _, _}) ->