[NEW] Sync deleted messages

[luismachado]

Checks for deleted messages every time messages are fetched (direct messages)

For now I've only implemented it for direct messages (and it's lacking optimisation), but would this be a viable way to sync (i.e. remove from the device) deleted messages?

@RocketChat/ios

Closes #2208

[CLAassistant]

All committers have signed the CLA.

[CLAassistant]

Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[codecov[bot]]

Codecov Report

Merging #2617 into develop will increase coverage by 0.16%. The diff coverage is 66.66%.

@@             Coverage Diff             @@
##           develop    #2617      +/-   ##
===========================================
+ Coverage    26.15%   26.32%   +0.16%     
===========================================
  Files          463      464       +1     
  Lines        17216    17252      +36     
===========================================
+ Hits          4503     4541      +38     
+ Misses       12713    12711       -2

Impacted Files	Coverage Δ
...API/Requests/Room/RoomDeletedMessagesRequest.swift	55% <55%> (ø)
Rocket.Chat/API/Clients/SubscriptionsClient.swift	67.14% <77.27%> (+0.13%)	:arrow_up:
...anagers/Model/AuthManager/AuthManagerRecover.swift	36.36% <0%> (-60.61%)	:arrow_down:
Rocket.Chat/Extensions/Models/AuthExtensions.swift	66.66% <0%> (-33.34%)	:arrow_down:
...Chat/Models/Subscription/SubscriptionQueries.swift	70% <0%> (-30%)	:arrow_down:
Rocket.Chat/Managers/PushManager.swift	24.13% <0%> (-10.35%)	:arrow_down:
Rocket.Chat/Managers/AppManager.swift	36.04% <0%> (-4.07%)	:arrow_down:
Rocket.Chat/API/APIRequest.swift	97.29% <0%> (+2.7%)	:arrow_up:
...Chat/Controllers/Chat/MessagesViewController.swift	34.25% <0%> (+2.75%)	:arrow_up:
... and 7 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update ecad166...c4d7b22. Read the comment docs.

[rafaelks]

@luismachado Amazing, thanks for the PR! Gonna review it for release 3.5, we're wrapping up 3.4 right now. 👏

[rafaelks]

@luismachado This PR is missing your signature of CLA, could you do that please?

[luismachado]

Hello @rafaelks,

I do understand what you're saying (and the fix definitely needs some improvements) but I'm not sure I would 100% agree with you (or maybe I didn't understand you fully :P).

• What I was thinking would be to grab the oldest message timestamp (I think this is already done) and then use that timestamp to fetch the deleted list.
• I'm not sure that we can avoid fetching deleted info that was already fetched because the users can delete any message, old or new, so we have to make sure that we always have the most recent info on deleted messages for the our current scope (i.e. the messages we have on cache).

We don't need to request it every time we request the history of messages In which situations could we avoid it? Maybe we could only call the endpoint for the first time the history is requested (i.e, when the messages controller is opened). I'm thinking this because (and please correct me if I'm wrong) if a (unfetched) message was deleted in the meanwhile it won't be returned by the api.

[rafaelks]

What I was thinking would be to grab the oldest message timestamp (I think this is already done) and then use that timestamp to fetch the deleted list.

I'm not sure that we can avoid fetching deleted info that was already fetched because the users can delete any message, old or new, so we have to make sure that we always have the most recent info on deleted messages for the our current scope (i.e. the messages we have on cache).

AFAIK the date you pass as a parameter is not the date of the message, but the date that the "updates" happen, so if a message from 5 days ago was deleted today and you fetch the API with the "since" from today, you will get the deleted message information, you know?

[luismachado]

Ah that makes much more sense :) I'm going to go over it then but I'll probably only have news next week.

On Wed, Apr 3, 2019 at 1:09 PM Rafael Kellermann Streit < [email protected]> wrote:

What I was thinking would be to grab the oldest message timestamp (I think this is already done) and then use that timestamp to fetch the deleted list.

I'm not sure that we can avoid fetching deleted info that was already fetched because the users can delete any message, old or new, so we have to make sure that we always have the most recent info on deleted messages for the our current scope (i.e. the messages we have on cache).

AFAIK the date you pass as a parameter is not the date of the message, but the date that the "updates" happen, so if a message from 5 days ago was deleted today and you fetch the API with the "since" from today, you will get the deleted message information, you know?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/RocketChat/Rocket.Chat.iOS/pull/2617#issuecomment-479461901, or mute the thread https://github.com/notifications/unsubscribe-auth/AHtxo4AWWK1PRd5jWBhFIv6m-cxkp1gCks5vdJndgaJpZM4cSIC7 .

-- Luís Machado Software Engineer / iOS Developer - Website http://lfmdevelopment.com