feat(notification): Add support for custom conditional push rules #4587
Conversation
…NotificationSettings
…notification settings
jonasrichardrichter
requested review from
Hywan
and removed request for
a team
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4587 +/- ##
==========================================
- Coverage 85.41% 85.38% -0.04%
==========================================
Files 286 286
Lines 32213 32228 +15
==========================================
+ Hits 27514 27517 +3
- Misses 4699 4711 +12 ☔ View full report in Codecov by Sentry. |
| PredefinedOverrideRuleId::InviteForMe.as_str(), | ||
| enabled, | ||
| ) | ||
| .await?; | ||
| Ok(()) | ||
| } | ||
|
|
||
| pub async fn set_custom_push_rule( |
There was a problem hiding this comment.
Please add some docs for this function too.
| let new_push_rule = match rule_kind { | ||
| RuleKind::Override => NewPushRule::Override(new_conditional_rule), | ||
| RuleKind::Underride => NewPushRule::Underride(new_conditional_rule), | ||
| _ => todo!(), | ||
| _ => return Err(NotificationSettingsError::InvalidParameter("rule_kind".to_owned())), |
There was a problem hiding this comment.
Could you add some tests for the enum cases used here, please?
| Bool(bool), | ||
|
|
||
| /// Represents an integer. | ||
| Integer(i64), | ||
|
|
||
| /// Represents a string. | ||
| String(String), |
There was a problem hiding this comment.
Note this notation is not supported when exporting Kotlin bindings, you'd have to change it to a struct based enum case:
Bool { value: bool }
That's probably why the complement crypto tests are failing.
| SdkPushCondition::EventPropertyContains { key, value } => { | ||
| Self::EventPropertyContains { key, value: value.into() } | ||
| } | ||
| _ => Self::from(value), |
There was a problem hiding this comment.
Maybe I'm overlooking something, but won't this call the this same function recursively with the same parameter?
|
|
||
| Self::Custom { name, value: json_string } | ||
| } | ||
| _ => Tweak::from(value), |
There was a problem hiding this comment.
Same here, won't this create a stack overflow?
| @@ -26,6 +26,9 @@ All notable changes to this project will be documented in this file. | |||
| - Enable HTTP/2 support in the HTTP client. | |||
| ([#4566](https://github.com/matrix-org/matrix-rust-sdk/pull/4566)) | |||
|
|
|||
| - Add support for creating custom conditional push rules in `NotificationSettings::create_custom_conditional_push_rule`. | |||
This pull request adds the support of custom conditional push rules using
NotificationSettings. This feature is also implemented in the ffi crate.Signed-off-by: Jonas Richard Richter [email protected]