Skip to content

Add an 'extras' property to comments #807

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

Merged
merged 8 commits into from
Aug 8, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions wp_api/src/comments.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::{
UserAvatarSize, UserId, WpApiParamOrder, WpResponseString,
AnyJson, UserAvatarSize, UserId, WpApiParamOrder, WpResponseString,
date::WpGmtDateTime,
impl_as_query_value_for_new_type, impl_as_query_value_from_to_string,
posts::PostId,
Expand All @@ -8,7 +8,7 @@ use crate::{
},
};
use serde::{Deserialize, Serialize};
use std::{collections::HashMap, num::ParseIntError, str::FromStr};
use std::{collections::HashMap, num::ParseIntError, str::FromStr, sync::Arc};
use strum_macros::IntoStaticStr;
use wp_contextual::WpContextual;

Expand Down Expand Up @@ -528,6 +528,10 @@ pub struct SparseComment {
pub comment_type: Option<CommentType>,
#[WpContext(edit, embed, view)]
pub author_avatar_urls: Option<HashMap<UserAvatarSize, WpResponseString>>,
#[serde(flatten)]
#[WpContext(edit, embed, view)]
#[WpContextualExcludeFromFields]
pub additional_fields: Option<Arc<AnyJson>>,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This field is included in the generated SparseCommentFields, which it should not be. I guess we'll have to implement a logic to exclude certain fields. We can either check the #[serde(flatten)] attribute (if that's possible), or add a new attribute like #[WpContextualExcludeFromFields]. What do you think? @oguzkocer

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was checking what needed to be done for this and figured I'd quickly implement it, so you don't have to do the same: #833

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

I have got this error using the new attribute, do you mind having a look?

$ cargo build
   Compiling wp_api v0.1.0 (/Users/tonyli/Projects/wordpress-rs/wp_api)
error[E0599]: no variant or associated item named `AdditionalFields` found for enum `SparseCommentFieldWithEditContext` in the current scope
   --> wp_api/src/comments.rs:495:57
    |
495 | #[derive(Debug, Serialize, Deserialize, uniffi::Record, WpContextual)]
    |                                                         ^^^^^^^^^^^^
    |                                                         |
    |                                                         variant or associated item not found in `SparseCommentFieldWithEditContext`
    |                                                         variant or associated item `AdditionalFields` not found for this enum
    |
    = note: this error originates in the derive macro `WpContextual` (in Nightly builds, run with -Z macro-backtrace for more info)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in c0cbafd. (after checking with @crazytonyli that it'd be OK for me to push directly to his branch)

// meta field is omitted for now: https://github.com/Automattic/wordpress-rs/issues/422
}

Expand Down
35 changes: 35 additions & 0 deletions wp_api/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use plugins::*;
use serde::{Deserialize, Serialize};
use serde_json::Value;
use std::collections::HashMap;
use users::*;
use wp_localization::{MessageBundle, WpMessages, WpSupportsLocalization};
Expand Down Expand Up @@ -118,6 +119,14 @@ pub enum JsonValue {
Object(HashMap<String, JsonValue>),
}

/// Similar to `JsonValue`, but exported as a Uniffi object.
#[derive(Debug, Clone, Eq, PartialEq, Hash, Serialize, Deserialize, uniffi::Object)]
#[uniffi::export(Eq, Hash)]
pub struct AnyJson {
#[serde(flatten)]
pub raw: Value,
}

uniffi::custom_newtype!(WpResponseString, Option<String>);
#[derive(Debug, Serialize, Deserialize)]
#[serde(try_from = "BoolOrString")]
Expand Down Expand Up @@ -235,4 +244,30 @@ mod tests {
fn test_orderby_string_conversion(#[case] orderby: WpApiParamOrder) {
assert_eq!(orderby, orderby.to_string().parse().unwrap());
}

#[derive(Deserialize, Debug)]
struct Person {
name: String,
#[serde(flatten)]
other_fields: AnyJson,
}

#[test]
fn test_parse_any_json() {
let json = r#"{"name": "Alice", "age": 30, "city": "Wonderland"}"#;
let person: Person = serde_json::from_str(json).unwrap();
assert_eq!(person.name, "Alice");
assert_eq!(
person.other_fields.raw,
serde_json::json!({"age": 30, "city": "Wonderland"})
);
}

#[test]
fn test_parse_empty_any_json() {
let json = r#"{"name": "Alice"}"#;
let person: Person = serde_json::from_str(json).unwrap();
assert_eq!(person.name, "Alice");
assert_eq!(person.other_fields.raw, serde_json::json!({}));
}
}
2 changes: 1 addition & 1 deletion wp_api/src/uniffi_serde.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use wp_localization::{MessageBundle, WpMessages, WpSupportsLocalization};
use wp_localization_macro::WpDeriveLocalizable;

#[derive(Debug, thiserror::Error, uniffi::Error, WpDeriveLocalizable)]
pub(crate) enum UniffiSerializationError {
pub enum UniffiSerializationError {
Serde { reason: String },
}

Expand Down
1 change: 1 addition & 0 deletions wp_api/src/wp_com/endpoint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use crate::{
use std::sync::Arc;
use strum::IntoEnumIterator;

pub mod extensions;
pub mod followers_endpoint;
pub mod jetpack_connection_endpoint;
pub mod oauth2;
Expand Down
42 changes: 42 additions & 0 deletions wp_api/src/wp_com/endpoint/extensions/comments.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
use serde::Deserialize;

use crate::{AnyJson, uniffi_serde::UniffiSerializationError};

#[derive(Debug, Deserialize, uniffi::Record)]
pub struct WpComCommentExtension {
#[serde(rename = "extended_post")]
pub post: Option<WpComCommentExtensionPostInfo>,
#[serde(rename = "extended_i_replied")]
pub i_replied: bool,
#[serde(rename = "extended_like_count")]
pub like_count: u32,
#[serde(rename = "extended_i_like")]
pub i_like: bool,
}

#[derive(Debug, Deserialize, uniffi::Record)]
pub struct WpComCommentExtensionPostInfo {
pub id: u64,
pub title: String,
#[serde(rename = "type")]
pub kind: String,
pub link: String,
}

#[uniffi::export(with_foreign)]
pub trait WpComCommentExtensionProvider: Send + Sync {
fn parse_wpcom_comments_extension(
&self,
) -> Result<WpComCommentExtension, UniffiSerializationError>;
}

#[uniffi::export]
impl WpComCommentExtensionProvider for AnyJson {
fn parse_wpcom_comments_extension(
&self,
) -> Result<WpComCommentExtension, UniffiSerializationError> {
serde_json::to_string(&self.raw)
.and_then(|json| serde_json::from_str(&json))
.map_err(Into::into)
}
}
1 change: 1 addition & 0 deletions wp_api/src/wp_com/endpoint/extensions/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
pub mod comments;
22 changes: 21 additions & 1 deletion wp_api_integration_tests/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::prelude::*;
use wp_api::wp_com::client::WpComApiClient;
use wp_api::wp_com::{WpComBaseUrl, client::WpComApiClient, endpoint::WpComDotOrgApiUrlResolver};

pub mod mock;
pub mod prelude;
Expand Down Expand Up @@ -42,6 +42,7 @@ pub struct WpComTestCredentials {
pub site_id: u64,
pub wp_com_subscriber_user_id: i64,
pub email_subscriber_subscription_id: u64,
pub comment_id: i64,
}

pub mod backend;
Expand Down Expand Up @@ -152,6 +153,25 @@ pub fn wp_com_client() -> WpComApiClient {
})
}

pub fn api_client_backed_by_wp_com(site_id: String) -> WpApiClient {
WpApiClient::new(
Arc::new(WpComDotOrgApiUrlResolver::new(
site_id,
WpComBaseUrl::Production,
)),
WpApiClientDelegate {
auth_provider: Arc::new(WpAuthenticationProvider::static_with_auth(
WpAuthentication::Bearer {
token: WpComTestCredentials::instance().bearer_token.to_string(),
},
)),
request_executor: Arc::new(ReqwestRequestExecutor::default()),
middleware_pipeline: Arc::new(WpApiMiddlewarePipeline::default()),
app_notifier: Arc::new(EmptyAppNotifier),
},
)
}

pub fn test_site_url() -> ParsedUrl {
let mut url: Url = TestCredentials::instance()
.site_url
Expand Down
18 changes: 18 additions & 0 deletions wp_api_integration_tests/tests/test_comments_immut.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use serde_json::Value;
use wp_api::{
comments::{
CommentId, CommentListParams, CommentRetrieveParams, CommentStatus, CommentType,
Expand Down Expand Up @@ -207,6 +208,23 @@ async fn list_comments_with_edit_context_parse_author_avatar_urls(
});
}

#[tokio::test]
#[parallel]
async fn parse_extras() {
let comment = api_client()
.comments()
.retrieve_with_edit_context(&FIRST_COMMENT_ID, &CommentRetrieveParams::default())
.await
.assert_response()
.data;
match comment.additional_fields.raw {
Value::Object(ref map) => {
assert!(map.contains_key("_links"));
}
_ => panic!("Expected extras to be an object"),
}
}

#[template]
#[rstest]
#[case::default(CommentListParams::default())]
Expand Down
72 changes: 72 additions & 0 deletions wp_api_integration_tests/tests/test_wp_com_comments_immut.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
use wp_api::{
comments::{CommentId, CommentRetrieveParams},
wp_com::endpoint::extensions::comments::WpComCommentExtensionProvider,
};

use wp_api_integration_tests::{WpComTestCredentials, api_client_backed_by_wp_com, prelude::*};

#[tokio::test]
#[parallel]
#[ignore]
async fn parse_extension_view_context() {
let site_id = WpComTestCredentials::instance().site_id.to_string();
let comment_id = CommentId(WpComTestCredentials::instance().comment_id);
let client = api_client_backed_by_wp_com(site_id);

let comment = client
.comments()
.retrieve_with_view_context(&comment_id, &CommentRetrieveParams::default())
.await
.assert_response()
.data;
assert!(
comment
.additional_fields
.parse_wpcom_comments_extension()
.is_ok()
);
}

#[tokio::test]
#[parallel]
#[ignore]
async fn parse_extension_edit_context() {
let site_id = WpComTestCredentials::instance().site_id.to_string();
let comment_id = CommentId(WpComTestCredentials::instance().comment_id);
let client = api_client_backed_by_wp_com(site_id);

let comment = client
.comments()
.retrieve_with_edit_context(&comment_id, &CommentRetrieveParams::default())
.await
.assert_response()
.data;
assert!(
comment
.additional_fields
.parse_wpcom_comments_extension()
.is_ok()
);
}

#[tokio::test]
#[parallel]
#[ignore]
async fn parse_extension_embed_context() {
let site_id = WpComTestCredentials::instance().site_id.to_string();
let comment_id = CommentId(WpComTestCredentials::instance().comment_id);
let client = api_client_backed_by_wp_com(site_id);

let comment = client
.comments()
.retrieve_with_embed_context(&comment_id, &CommentRetrieveParams::default())
.await
.assert_response()
.data;
assert!(
comment
.additional_fields
.parse_wpcom_comments_extension()
.is_ok()
);
}
3 changes: 2 additions & 1 deletion wp_com_test_credentials.json-example
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,6 @@
"bearer_token": "replace_with_your_oauth2_token",
"site_id": 0,
"wp_com_subscriber_user_id": 0,
"email_subscriber_subscription_id": 0
"email_subscriber_subscription_id": 0,
"comment_id": 0
}
3 changes: 3 additions & 0 deletions wp_contextual/src/wp_contextual.rs
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,9 @@ fn generate_integration_test_helper(
let mut assertions = Vec::with_capacity(fields.len());
let mut rs_test_cases = Vec::with_capacity(fields.len());
for f in fields {
if f.is_wp_contextual_exclude_from_fields {
continue;
}
if let Some(f_ident) = &f.field.ident {
let variant_ident = format_ident!("{}", f_ident.to_string().to_case(Case::UpperCamel));
let field_ident_str = f_ident.to_string();
Expand Down