feat: allow creation of space.board.post events (WIP)
#15
Conversation
avdb13
changed the title
space.board events (WIP)space.board.post events (WIP)
Cargo.toml
Outdated
| @@ -21,3 +22,6 @@ tracing = "0.1.40" | |||
| tracing-subscriber = "0.3.18" | |||
| uuid = { version = "1.6.1", features = ["v4"] } | |||
| url = "2.4.1" | |||
| ruma = { version = "0.9.0", features = ["events", "api", "client", "client-api-c", "rand"] } | |||
There was a problem hiding this comment.
Can we have feature flags in the Cargo.toml of the crate that uses them?
There was a problem hiding this comment.
Oh my bad, we can enable certain features per crate? I thought we had to specify feature X Y Z at the root so that crate A can use feature X Y and B feature Z.
There was a problem hiding this comment.
So the correct way is to specify the dependency without feature flags at the root, but specify the necessary ones per crate?
crates/core/Cargo.toml
Outdated
| ruma = { workspace = true, features = ["events"] } | ||
| ruma-common = { workspace = true } | ||
| ruma-macros = { workspace = true } |
There was a problem hiding this comment.
Please keep matrix related dependencies in the matrix crate.
I suggest you reexport or provide wrappers on functions/primitives from ruma that could be transparently replaced without damaging core references.
crates/core/src/events/service.rs
Outdated
| use std::sync::Arc; | ||
|
|
||
| use futures::future::join_all; | ||
| use matrix; | ||
| use matrix::admin::resources::room::{ListParams, MessagesParams, RoomService}; | ||
| use matrix::admin::resources::room_id::RoomId; | ||
|
|
||
| use crate::{Result, Error}; | ||
| use matrix::Client as MatrixAdminClient; | ||
| use matrix::client::resources::events::{Events, SendMessageResponse}; | ||
| use ruma::events::{AnyTimelineEvent, MessageLikeEventContent}; |
There was a problem hiding this comment.
Please re-arrange imports by groups.
- Standard library
- External crates
- Local crates (Workspace)
- This crate imports
crates/core/src/events/service.rs
Outdated
| room_id: impl AsRef<str>, | ||
| txn_id: impl AsRef<str>, |
There was a problem hiding this comment.
Deplicated constraints. Can we have them in a generic param?
U: AsRef<str>
There was a problem hiding this comment.
I switched to the ID types provided by ruma_events. These provide more context while allowing referenced as well as owned variants.
crates/core/src/events/service.rs
Outdated
| }, | ||
| ) | ||
| .await | ||
| .unwrap(); |
There was a problem hiding this comment.
Please avoid unwraps. Use error handling.
crates/core/src/events/space.rs
Outdated
| body, | ||
| formatted: Some(FormattedBody::html(html_body)), | ||
| links: None, | ||
| // relates_to: None, |
There was a problem hiding this comment.
| // relates_to: None, |
| use super::room_id::RoomId; | ||
| use crate::http::Client; |
There was a problem hiding this comment.
Nit!
| use super::room_id::RoomId; | |
| use crate::http::Client; | |
| use crate::http::Client; | |
| use super::room_id::RoomId; |
| // should we inline or pull this from `ruma`? | ||
| // pub filter: Option<ruma::api::Filter>, |
There was a problem hiding this comment.
As we cant expose public fields with private symbols. Then I suggest wrap and provide a valuo if its required as public api.
|
|
||
| #[derive(Debug, Default, Serialize)] | ||
| pub struct TimestampToEventParams { | ||
| pub ts: Option<u64>, |
There was a problem hiding this comment.
Can we have alias for timestamp?
| #[derive(Default, Debug, Deserialize)] | ||
| pub struct State { | ||
| #[serde(rename = "type")] | ||
| pub kind: String, |
There was a problem hiding this comment.
| pub kind: String, | |
| pub r#type: String, |
|
@avdb13 if this is not ready, I suggest turning into a draft perhaps? |
merge: token aware routes with middleware (commune-sh#14)
Provides the handler required to create a post.