Map DELETE account data (PSG-647) #1651
Conversation
alfogrillo
requested review from
a team and
Anderas
and removed request for
a team
Codecov ReportBase: 25.32% // Head: 25.31% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## develop #1651 +/- ##
===========================================
- Coverage 25.32% 25.31% -0.01%
===========================================
Files 587 587
Lines 92813 92866 +53
Branches 40258 40273 +15
===========================================
+ Hits 23505 23510 +5
- Misses 68526 68576 +50
+ Partials 782 780 -2
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
There was a problem hiding this comment.
Looks good incl the added tests. Just a small comment regarding NS_REFINED_FOR_SWIFT.
Unrelatedly, will the application handle recieving {} as account data in sync gracefully? This actually has to be backwards compatible as well, if older clients start recieving this type of response. Or is this what the clients have been effectively doing already?
MatrixSDK/Data/MXAccountData.h
Outdated
| /** | ||
| Delete the account data with the a given type. | ||
|
|
||
| For internal use only. |
There was a problem hiding this comment.
If for internal use only, is there another API that a user of the SDK can call, see the alternatives we suggest in updateDataWithType etc?
There was a problem hiding this comment.
Agreed, deleted the new API here -> fe60354
There was a problem hiding this comment.
Oh I just meant the comments itself, looking at the other APIs that mention "For internal use only", they suggest what alternatives to use instead. Having the deleteAccount is probably useful. I think this is a legacy problem, where we have to add "internal" API to a public header file that anyone can call, so we limit the use via the comments.
There was a problem hiding this comment.
Oh, I misunderstood your comment.
Reverted the original implementation and updated the comment here -> 4f9a088
MatrixSDK/MXRestClient.h
Outdated
| */ | ||
| - (MXHTTPOperation*)deleteAccountDataWithType:(NSString*)type | ||
| success:(void (^)(void))success | ||
| failure:(void (^)(NSError *error))failure NS_REFINED_FOR_SWIFT; |
There was a problem hiding this comment.
If setting the NS_REFINED_FOR_SWIFT macro, you should set the refined swift variant in MXRestClient.swift. Alternatively you could just remove the macro.
There was a problem hiding this comment.
Thanks for spotting it!
I think I don't need it actually.
Deleted here -> 9582483
Yeah, old clients will store locally |
This reverts commit fe60354.
Description
This PR add a new endpoint to delete user's account data.
More context here (MSC3391).