Conversation
- Fixes #3179
dessalines
requested review from
Nutomic,
SleeplessOne1917,
dullbananas and
phiresky
as code owners
| &None, | ||
| ), | ||
| }) | ||
| .collect::<LemmyResult<Vec<Item>>>()?; |
There was a problem hiding this comment.
This is where a trait could be useful. Basically define a trait like this and implement it for all the modlog structs:
trait ModlogItem {
fn moderator(&self) -> Option<Person>;
fn published(&self) -> DateTime<Utc>;
...
}And then change build_modlog_item to take T: ModlogItem and call these methods. Though it only makes sense if the trait can also be used elsewhere in the code. If its only needed here then we can leave it like this.
There was a problem hiding this comment.
It's only needed here. I agree it would make sense if we needed to match on all the other modlog types elsewhere.
| }); | ||
| let author = mod_ | ||
| .as_ref() | ||
| .map(|mod_| format!("/u/{} <a href=\"{}\">(link)</a>", mod_.name, mod_.ap_id)); |
There was a problem hiding this comment.
ap_id is wrong because it will link directly to remote instances for remote users. Below in build_item the author is also generated wrong if its a remote user. There is format_actor_url in markdown_links.rs that you can use for both.
There was a problem hiding this comment.
I fixed, and moved this to the ApubActor trait now.
| ) -> LemmyResult<Vec<Item>> { | ||
| // All of these go to your modlog url | ||
| let modlog_url = format!( | ||
| "{}/modlog?listing_type=ModeratorView", |
There was a problem hiding this comment.
listing_type=ModeratorView is only for post listing, not for modlog.
There was a problem hiding this comment.
Its also there now: https://github.com/LemmyNet/lemmy/blob/main/crates/api_common/src/site.rs#L93
| let guid = Some(Guid { | ||
| permalink: true, | ||
| value: url.to_owned(), | ||
| }); |
There was a problem hiding this comment.
Guid means "Globally unique identifier" so it doesnt make sense to have the same url for multiple items.
There was a problem hiding this comment.
I'll set it to the action string then, since I'm not sure what else to use as a unique id.
| format!("{local_protocol_and_hostname}/u/{}", self.name) | ||
| }; | ||
| Ok(Url::parse(&url)?) | ||
| } |
There was a problem hiding this comment.
Would be good to deduplicate this code with a helper function. In fact that helper already exists in markdown_links.rs (format_actor_url), so you only need to move that to db_schema, then call it from here and community.
There was a problem hiding this comment.
Good call, I'll do that now, while still keeping the trait functions.
There was a problem hiding this comment.
You can also delete this function now.
https://github.com/LemmyNet/lemmy/blob/main/crates/apub_objects/src/utils/markdown_links.rs#L77
There was a problem hiding this comment.
This already is removed in the PR.
| &v.moderator, | ||
| &v.mod_remove_community.published, | ||
| &modlog_url, | ||
| &format!("Removed community /c/{}", &v.community.name), |
There was a problem hiding this comment.
These all dont respect the removed field, so if a comment is restored it will show in the feed as removed. And as this field is used for guid, it means remove, restore actions will be deduplicated by feed readers and only one of them shown.
| "unblocked" | ||
| }, | ||
| &v.instance.domain | ||
| ), |
There was a problem hiding this comment.
Some of these are used more than once so you can make a helper function for each of blocked, removed, banned like:
fn removed_or_added(bool: removed) -> &'static str {
if removed {
"Removed"
} else {
"Added"
}
}
* Adding Modlog RSS feed. - Fixes #3179 * Addressing PR comments * Fixing clippy. * Fixing markdown test. * Creating common format_actor_url function. * Clippy * Adding boolean strings for remove/restore * Addressing PR comments
2 participants
This is missing a few things currently:
But its a good start, and those can be tweaked later based on feedback.