-
Notifications
You must be signed in to change notification settings - Fork 2
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
Add Group & GroupAuditLogs model & query #7
Conversation
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.
Lacks integration tests;
#[derive(Default, serde::Deserialize)]
pub struct TestConfig {
pub friend_id: Option<vrc::id::User>,
pub self_id: Option<vrc::id::User>,
pub world_id: Option<vrc::id::World>,
pub group_id: Option<vrc::id::Group>,
pub moderating_group_id: Option<vrc::id::Group>,
}
and tests/groups.rs
#![cfg(feature = "api_client")]
use std::default;
use vrc::{
api_client::{ApiClient, ApiError},
model::{Group, GroupAuditLogs},
};
mod common;
#[tokio::test]
#[ignore]
async fn group() -> Result<(), ApiError> {
let group_id = match &common::TEST_CONFIG.group_id {
Some(v) => v,
None => return Ok(()),
};
let api_client = common::api_client()?;
let query = vrc::query::Group { id: group_id.clone() };
let group: Group = api_client.query(query).await?;
dbg!(&group);
Ok(())
}
#[tokio::test]
#[ignore]
async fn group_audit_logs() -> Result<(), ApiError> {
let group_id = match &common::TEST_CONFIG.moderating_group_id {
Some(v) => v,
None => return Ok(()),
};
let api_client = common::api_client()?;
let query =
vrc::query::GroupAuditLogs { id: group_id.clone(), n: None, offset: None };
let audit_logs: GroupAuditLogs = api_client.query(query).await?;
dbg!(&audit_logs);
Ok(())
}
with possibly some additional assertions sprinkled in would be nice.
My API testing user is not a moderator of any groups, so I can't really validate the audit logs portion as it gives a 403.
Also can change the status in the README table for groups the be partial.
src/model/groups.rs
Outdated
#[derive(Debug, Clone, PartialEq, Eq, Deserialize, Serialize)] | ||
#[serde(rename_all = "camelCase")] | ||
/// Represents additional data associated with a group audit log entry. | ||
pub struct Data { |
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.
Since this is just re-exported from vrc::model
, should be called GroupAuuditLogData
or something similar instead of just Data
.
src/model/groups.rs
Outdated
#[derive(Debug, Clone, PartialEq, Eq, Deserialize, Serialize)] | ||
#[serde(rename_all = "camelCase")] | ||
/// Represents a change in description associated with a group audit log entry. | ||
pub struct DescriptionChange { | ||
/// The old description before the change. | ||
pub old: String, | ||
/// The new description after the change. | ||
pub new: String, | ||
} | ||
|
||
#[derive(Debug, Clone, PartialEq, Eq, Deserialize, Serialize)] | ||
#[serde(rename_all = "camelCase")] | ||
/// Represents a change in join state associated with a group audit log entry. | ||
pub struct JoinStateChange { | ||
/// The old join state before the change. | ||
pub old: String, | ||
/// The new join state after the change. | ||
pub new: String, | ||
} | ||
|
||
#[derive(Debug, Clone, PartialEq, Eq, Deserialize, Serialize)] | ||
#[serde(rename_all = "camelCase")] | ||
/// Represents a change in order associated with a group audit log entry. | ||
pub struct OrderChange { | ||
/// The old order before the change. | ||
pub old: u32, | ||
/// The new order after the change. | ||
pub new: u32, | ||
} |
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.
Couldn't these be unified under a common ChangeEntry<T>
or such type?
src/query/groups.rs
Outdated
format!("{}/groups/{}/auditLogs?", crate::API_BASE_URI, self.id.as_ref()); | ||
|
||
if let Some(n) = self.n { | ||
let param = format!("&n={n}"); | ||
query.push_str(¶m); | ||
} | ||
|
||
if let Some(offset) = self.offset { | ||
let param = format!("&offset={offset}"); | ||
query.push_str(¶m); | ||
} |
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.
This'd result in .../auditLogs?&n=
... Should only add the &
/?
when required... Could for example turn the optional params into a vec, join with &
and then if there's results prefix with ?
before constructing the query with format!
.
I think I got everything requested, let me know if there's anything else. |
I switched the total_count from usize to u32 because there's no safe conversions from usize to anything but u128, and there's no safe conversion from u64 to f64 to allow for floating point calculations using the number (safely). I don't if there's a way to see what that's really being stored in or if it could be exceeded, but I figured a limit of 4.3b is a good trade-off to allow for safe float conversion. |
pub member_count: i64, | ||
/// The times tamp when the member count was last synchronized. | ||
pub member_count_synced_at: String, | ||
/// Indicates whether the group is verified. | ||
pub is_verified: bool, | ||
/// The join state of the group. | ||
pub join_state: String, | ||
// pub tags: Vec<Value>, | ||
// pub galleries: Vec<Value>, | ||
/// The time stamp when the group was created. | ||
pub created_at: String, | ||
/// The count of online members in the group. | ||
pub online_member_count: i64, |
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.
...I guess it indeed is better to support a case where the member counts are negative for whatever reason, even though on first thought it seemed wrong 😄
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.
lol I think that's just what Json -> Serde defaulted to
I added
Group
andGroupAuditLogs
models and queries.I'm also working on file downloads (so I can download images), should I include that with this?
Let me know if there's anything you want changed