Repository navigation
fix(chat): fall back to the iOS edit and delete windows when the server sends none - #1486
Merged
Merged
Conversation
…er sends none `message_edit_window` and `message_delete_window` are message-typed, so the server can leave them unset, and it does. Android read that as no limit and kept Edit and Delete on the menu forever; iOS substitutes 15 minutes and 48 hours. The same message therefore offered different rows depending on the platform. Match iOS. `MessagePolicy.fromFlags` substitutes `FallbackEditWindow` / `FallbackDeleteWindow` for anything the flags leave unset, and `Default` — the policy in force before flags are read — is now those two windows rather than unbounded. Every upstream state collapses to the same input: flags not fetched, a failed fetch, and unset window fields all arrive as null and all get the fallback. The cost is that a message old enough can lose Edit or Delete where the server would have taken the request. That is the trade iOS already made, and the worse failure is the opposite one: an affordance the server answers `CANNOT_EDIT` or `CANNOT_DELETE`. The values are a product choice, not one the contract supplies — the proto documents what the fields mean but never what an absent field implies. Nothing enforces the match with iOS, so a test on each side asserts the literal values and names its counterpart. The constructor no longer defaults either window. Absence decides whether the fallback applies, so it is worth stating at the call site.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
message_edit_windowandmessage_delete_windoware message-typed, so the server can leave them unset — and it does. Android read that as no limit and kept Edit and Delete on the menu forever; iOS substitutes 15 minutes and 48 hours. The same message offered different rows depending on which app you opened it in.This matches iOS.
MessagePolicy.fromFlagssubstitutesFallbackEditWindow/FallbackDeleteWindowfor anything the flags leave unset, andDefault— the policy in force before flags are read — is now those two windows rather than unbounded. Every upstream state collapses to the same input: flags not yet fetched, a failed fetch, and unset window fields all arrive asnulland all get the fallback, so there is no second path to keep in step.The cost is that a message old enough can lose Edit or Delete where the server would in fact have taken the request. That is the trade iOS already made, and the opposite failure is worse: offering an affordance the server answers
CANNOT_EDITorCANNOT_DELETE.The values are a guess
The proto documents what the two fields mean but never what an absent one implies, so 15 minutes and 48 hours are a product choice iOS made in code-payments/code-ios-app#724 — on the stated grounds that Android already used them, which was not true. Android had no such constant. The comment asserting the match is corrected in code-payments/code-ios-app#797, along with the design spec, which argued for the unbounded reading Android had implemented.
Nothing enforces the pair now that both sides carry it, so a test on each side asserts the literal values and names its counterpart; a failure is the prompt to change the other repo in the same release.
Whether real users ever reach this branch is still unknown — the cached-flag reads that could have answered it were all staff accounts.
Also
The
MessagePolicyconstructor no longer defaults either window. Absence is a real input, since it decides whether the fallback applies, so it is worth stating at each call site rather than inheriting. Two reducer tests that relied on the old unbounded default now build an explicit unbounded policy, because their bubbles are timestamped in 1970 and the fallback windows would otherwise strip Edit and Delete before the assertion could say anything about the reducer.