-
Notifications
You must be signed in to change notification settings - Fork 14
Edition 2024 & GroupData image fields #705
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughAdds optional image fields (key, hash, nonce) to FlutterGroupDataUpdate across Dart and Rust, updates constructors, equality/hashCode, and FRB encoding/decoding for DCO/SSE paths, introduces U8Array12 optional (de)serializers, propagates fields in Rust conversions, and switches Rust edition to 2024. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dart as Dart (Flutter)
participant FRB_D as FRB Dart
participant FRB_R as FRB Rust
participant Rust as Rust API
Note over Dart,FRB_R: Create/receive FlutterGroupDataUpdate with imageKey/imageHash/imageNonce (optional)
Dart->>FRB_D: Encode FlutterGroupDataUpdate (SSE/DCO)
FRB_D->>FRB_R: Transmit fields incl. imageKey/hash/nonce
FRB_R->>Rust: sse_decode → FlutterGroupDataUpdate { image_* }
Rust->>Rust: Convert to NostrGroupDataUpdate (propagate image_*)
Rust-->>FRB_R: sse_encode FlutterGroupDataUpdate with image_*
FRB_R-->>FRB_D: Return serialized data
FRB_D-->>Dart: Decode incl. imageKey/hash/nonce
Note over Dart: Fields available in Dart model
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
josefinalliende
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅ Built it for ios and tried it in my iPhone. Everything seems to be working fine!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
ios/Podfile.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
lib/src/rust/api/groups.dart(2 hunks)lib/src/rust/frb_generated.dart(6 hunks)lib/src/rust/frb_generated.io.dart(3 hunks)rust/Cargo.toml(1 hunks)rust/src/api/groups.rs(1 hunks)rust/src/api/users.rs(1 hunks)rust/src/frb_generated.rs(7 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
**/*.dart: Always declare the type of each variable and function (parameters and return value). Avoid using 'any'. Create necessary types.
Don't leave blank lines within a function.
One export per file.
Use PascalCase for classes.
Use camelCase for variables, functions, and methods.
Use underscores_case for file and directory names.
Use UPPERCASE for environment variables. Avoid magic numbers and define constants.
Start each function with a verb.
Use verbs for boolean variables. Example: isLoading, hasError, canDelete, etc.
Use complete words instead of abbreviations and correct spelling, except for standard and well-known abbreviations (API, URL, i, j, err, ctx, req, res, next).
Write short functions with a single purpose. Less than 20 instructions.
Name functions with a verb and something else. If it returns a boolean, use isX or hasX, canX, etc. If it doesn't return anything, use executeX or saveX, etc.
Avoid nesting blocks by early checks and returns, or extraction to utility functions.
Use higher-order functions (map, filter, reduce, etc.) to avoid function nesting. Use arrow functions for simple functions (less than 3 instructions). Use named functions for non-simple functions.
Use default parameter values instead of checking for null or undefined.
Reduce function parameters using RO-RO: use an object to pass multiple parameters and to return results. Declare necessary types for input arguments and output.
Use a single level of abstraction in functions.
Don't abuse primitive types and encapsulate data in composite types.
Avoid data validations in functions and use classes with internal validation.
Prefer immutability for data. Use readonly for data that doesn't change. Use 'as const' for literals that don't change.
Declare interfaces to define contracts.
Write small classes with a single purpose. Less than 200 instructions, less than 10 public methods, less than 10 properties.
Use exceptions to handle errors you don't expect. If you catch an exception, it sh...
Files:
lib/src/rust/frb_generated.io.dartlib/src/rust/frb_generated.dartlib/src/rust/api/groups.dart
🧠 Learnings (2)
📚 Learning: 2025-08-08T13:39:00.500Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-08-08T13:39:00.500Z
Learning: Applies to **/*.dart : Use flutter_rust_bridge to access core functionality of the app.
Applied to files:
rust/src/frb_generated.rs
📚 Learning: 2025-08-08T13:39:00.500Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-08-08T13:39:00.500Z
Learning: Applies to **/*.dart : Use the whitenoise rust crate (via flutter_rust_bridge) as the source of all data and only way to trigger changes in our data model.
Applied to files:
rust/src/frb_generated.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Flutter CI
🔇 Additional comments (5)
rust/Cargo.toml (1)
4-4: Confirm toolchain is on a Rust release that supports edition 2024.
Edition 2024 requires at least the toolchain that shipped it (Rust 1.85+). Please double-check CI, build scripts, and developer setup to ensure they’re already on that release; otherwise this change will break builds.lib/src/rust/frb_generated.dart (4)
2754-2767: LGTM! DCO decoder correctly handles the new image fields.The decoder properly:
- Updates the array length check to 7 to accommodate the three new fields
- Decodes imageKey, imageHash (32-byte arrays), and imageNonce (12-byte array) as optional fields
- Maintains field order consistency with the SSE decoder
2991-2995: LGTM! New U8Array12 optional encoder/decoder functions are correctly implemented.The three new helper functions (DCO decoder, SSE decoder, SSE encoder) for
opt_u_8_array_12follow the established pattern for optional array handling and are consistent with the existingopt_u_8_array_32implementations.Also applies to: 3913-3922, 4770-4778
3520-3541: LGTM! SSE decoder correctly handles the new image fields.The SSE decoder properly:
- Decodes all three new fields (imageKey, imageHash, imageNonce) in the correct order
- Maintains consistency with the DCO decoder field order
- Uses appropriate optional decoders for the nullable array types
4445-4458: LGTM! SSE encoder correctly serializes the new image fields.The SSE encoder properly:
- Encodes all three new fields in the correct order
- Maintains field order consistency with both decoders
- Uses appropriate optional encoders matching the decoder types
The encode/decode symmetry is maintained across DCO and SSE codecs.
| pub image_key: Option<[u8; 32]>, | ||
| pub image_hash: Option<[u8; 32]>, | ||
| pub image_nonce: Option<[u8; 12]>, | ||
| } | ||
|
|
||
| impl From<FlutterGroupDataUpdate> for NostrGroupDataUpdate { | ||
| fn from(group_data: FlutterGroupDataUpdate) -> Self { | ||
| Self { | ||
| name: group_data.name, | ||
| description: group_data.description, | ||
| image_key: None, // TODO: Support image updates | ||
| image_hash: None, // TODO: Support image updates | ||
| image_nonce: None, // TODO: Support image updates | ||
| // Wrap in Some() to convert Option<T> to Option<Option<T>> | ||
| // None means don't update, Some(value) means set to value | ||
| image_key: group_data.image_key.map(Some), | ||
| image_hash: group_data.image_hash.map(Some), | ||
| image_nonce: group_data.image_nonce.map(Some), | ||
| // Will silently drop invalid relay inputs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cannot clear image fields with the current mapping.
FlutterGroupDataUpdate only carries Option<[u8; _]>, and the .map(Some) here collapses “set to null” into “don’t touch”. There is no way for Flutter to emit Some(None) for NostrGroupDataUpdate, so once an image key/hash/nonce is set it can never be cleared via this API. Please extend the DTO to carry a real tri-state (e.g. Option<Option<_>> or an explicit enum) so we can send Some(None) when the user wants to remove the image.
🤖 Prompt for AI Agents
In rust/src/api/groups.rs around lines 57 to 72, the current conversion maps
FlutterGroupDataUpdate's Option<[u8; _]> to NostrGroupDataUpdate using
.map(Some), which prevents sending a "clear" instruction (Some(None)) and makes
image fields impossible to unset; update the DTO to carry a tri-state (change
FlutterGroupDataUpdate fields to Option<Option<[u8; N]>> or an explicit enum) so
the outer Option represents "update vs no-op" and the inner Option represents
"set value vs clear", and adjust the From impl to map the new tri-state fields
directly into NostrGroupDataUpdate without collapsing None into no-op (respect
outer Some(None) as a clear).
This updates the rust edition to the latest 2024. This should have no effect on the app.
Summary by CodeRabbit