You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Expose leave request fallback text via bindings_ffi::mls::leave_request_fallback and set xmtp_content_types::leave_request::LeaveRequestCodec::encode to populate EncodedContent.fallback
This PR moves the fallback text for LeaveRequest content type from being hardcoded as None to a defined constant and exposes it via an FFI function. The implementation is clean and follows the existing patterns in the codebase.
Findings
✅ Code Quality
Implementation is idiomatic and consistent with codebase conventions
The new FALLBACK_TEXT constant is properly documented
The FFI function follows the same pattern as other decode functions
⚠️ Test Coverage
Missing test: The new leave_request_fallback() FFI function at bindings_ffi/src/mls.rs:3144 has no test coverage
Existing test at bindings_ffi/src/mls/tests/content_types.rs:1269 (test_leave_request_encode_decode) doesn't verify the fallback text in the encoded content
Consider adding:
#[tokio::test]asyncfntest_leave_request_fallback(){let fallback = leave_request_fallback();assert_eq!(fallback,"A member has requested leaving the group");// Also verify it matches what's in encoded contentlet leave_request = FfiLeaveRequest{authenticated_note:None};let encoded_bytes = encode_leave_request(leave_request).unwrap();let encoded_content = EncodedContent::decode(encoded_bytes.as_slice()).unwrap();assert_eq!(encoded_content.fallback,Some(fallback));}
ℹ️ Minor Observations
The impl LeaveRequestCodec block at xmtp_content_types/src/leave_request.rs:21 is separate from the main impl block (line 15). While valid, this could be consolidated for consistency
The fallback text is now set during encoding (xmtp_content_types/src/leave_request.rs:44), which is the correct behavior
Recommendation
The implementation is solid, but adding test coverage for the new FFI function would improve confidence in the change. No blocking issues identified.
@cameronvoell we had this discussion last week, I made this POC PR to check if this is what we want?
1- since we don't have a field for fallback texts in all codecs, that makes it messy since we need an extra function per codec to extract the text, we can make it cleaner by either having a single function to return the associated text for each content type.
2- we already have the fallback texts hardcoded in the sdks, so I would like to extract those if we need to do that as a practice.
3- Since we're having this conversation about the fallback texts, I would like to make it easier for integrators to give them an option to make it easier to translate this texts, so instead of giving them a hardcoded text, I would like to introduce a struct to hold not only the predefined english fallback text, but also gives them a code or a key to help them make a translation based on those keys, for example:
ContentTypeFallbackText{
Key: "LeaveRequest-PendingRemove",
Lan: "En",
Value: "use has requested leaving the group"
}
And so on.
wdyt?
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
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.
Expose leave request fallback text via
bindings_ffi::mls::leave_request_fallbackand setxmtp_content_types::leave_request::LeaveRequestCodec::encodeto populateEncodedContent.fallbackAdd
LeaveRequestCodec::FALLBACK_TEXTand use it inencodeto setEncodedContent.fallback; add a UniFFI functionleave_request_fallback()that returns the fallback text via FFI in bindings_ffi/src/mls.rs and xmtp_content_types/src/leave_request.rs.📍Where to Start
Start with the UniFFI export
leave_request_fallback()in bindings_ffi/src/mls.rs, then reviewLeaveRequestCodec::FALLBACK_TEXTand its use inencodein xmtp_content_types/src/leave_request.rs.Macroscope summarized 932d1c1.