Skip to content

api: rename properties to be more consistent #7756

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.

Sign up for GitHub

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

Open
nicodh wants to merge 3 commits into
base: main
Choose a base branch
from
Open

api: rename properties to be more consistent #7756

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
74fc239
api: rename properties to be more consistent
nicodh Jan 16, 2026
ace4dc7
api: deprecate self_in_group from chatlist item
nicodh Jan 24, 2026
ac13c26
Expect instead allow clippy large enum
nicodh Jan 24, 2026
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
6 changes: 4 additions & 2 deletions deltachat-jsonrpc/src/api/types/chat.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ pub struct FullChat {

is_device_chat: bool,
/// Note that this is different from
/// [`ChatListItem::is_self_in_group`](`crate::api::types::chat_list::ChatListItemFetchResult::ChatListItem::is_self_in_group`).
/// [`ChatListItem::self_in_group`](`crate::api::types::chat_list::ChatListItemFetchResult::ChatListItem::self_in_group`).
Copy link
Collaborator

@Hocuri Hocuri Jan 26, 2026

Choose a reason for hiding this comment

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

This comment is wrong, then (apparently cargo doctest doesn't run in CI for deltachat-jsonrpc? Otherwise, it would have caught that).

Can just revert to the previous comment

/// This property should only be accessed
/// when [`FullChat::chat_type`] is [`Chattype::Group`].
//
Expand Down Expand Up @@ -178,8 +178,9 @@ pub struct BasicChat {
is_self_talk: bool,
color: String,
is_contact_request: bool,

// deprecated 2026-01, use is_device_talk instead
is_device_chat: bool,
is_device_talk: bool,
is_muted: bool,
}

Expand Down Expand Up @@ -207,6 +208,7 @@ impl BasicChat {
color,
is_contact_request: chat.is_contact_request(),
is_device_chat: chat.is_device_talk(),
is_device_talk: chat.is_device_talk(),
is_muted: chat.is_muted(),
})
}
Expand Down
16 changes: 14 additions & 2 deletions deltachat-jsonrpc/src/api/types/chat_list.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ pub enum ChatListItemFetchResult {
ChatListItem {
id: u32,
name: String,
/// deprecated 2026-01, use profile_image instead
avatar_path: Option<String>,
profile_image: Option<String>,
color: String,
chat_type: JsonrpcChatType,
last_updated: Option<i64>,
Expand Down Expand Up @@ -61,9 +63,15 @@ pub enum ChatListItemFetchResult {
is_self_talk: bool,
is_device_talk: bool,
is_sending_location: bool,
/// deprecated 2026-01, use self_in_group instead
is_self_in_group: bool,
self_in_group: bool,
/// deprecated 2026-01, use archived instead
is_archived: bool,
archived: bool,
/// deprecated 2026-01, use pinned instead
is_pinned: bool,
pinned: bool,
is_muted: bool,
is_contact_request: bool,
/// contact id if this is a dm chat (for view profile entry in context menu)
Expand Down Expand Up @@ -105,7 +113,7 @@ pub(crate) async fn get_chat_list_item_by_id(

let visibility = chat.get_visibility();

let avatar_path = chat
let profile_image = chat
.get_profile_image(ctx)
.await?
.map(|path| path.to_str().unwrap_or("invalid/path").to_owned());
Expand Down Expand Up @@ -150,7 +158,8 @@ pub(crate) async fn get_chat_list_item_by_id(
Ok(ChatListItemFetchResult::ChatListItem {
id: chat_id.to_u32(),
name: chat.get_name().to_owned(),
avatar_path,
profile_image: profile_image.clone(),
avatar_path: profile_image.clone(),
color,
chat_type: chat.get_type().into(),
last_updated,
Expand All @@ -164,8 +173,11 @@ pub(crate) async fn get_chat_list_item_by_id(
is_self_talk: chat.is_self_talk(),
is_device_talk: chat.is_device_talk(),
is_self_in_group: chat.is_self_in_chat(ctx).await?,
self_in_group: chat.is_self_in_chat(ctx).await?,
Copy link
Collaborator

@Hocuri Hocuri Jan 22, 2026

Choose a reason for hiding this comment

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

chat.is_self_in_chat(ctx).await? is quite an expensive call, as it does a database request (just as most async functions). So, you should put let self_in_group = chat.is_self_in_chat(ctx).await?; above, and then use the variable here.

Copy link
Contributor

@r10s r10s Jan 22, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

i am wondering if [is]_self_in_group field is needed at all, esp. when it comes at costs.

[is]_self_in_group is currently only used to check whether to show the "N members" subtitle or "...", but that is probably not the ChalistItem then. and even if, it could be done by checking the anyway loaded contact list for CONTACT_ID_SELF (android code, desktop code)

(as a rule of thumb, we should try to avoid database calls needed to set a property that is not always needed - otherwise we have for the chatlist lots of never needed database calls, cffi is pretty strict on that, jsonrpc less in the past, resulting in hard to get slownesses, we're getting better there, but we should avoid that in the future)

Copy link
Contributor Author

@nicodh nicodh Jan 22, 2026

Choose a reason for hiding this comment

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

Based on that property we show the "leave group" context menu item, In chatlist where don't have the contact IDs of each listed group so we can't check if CONTACT_ID_SELF is in that list.

This precalculated property should avoid that we have to load the full chat when opening the context menu.

But in the end an extra DB call for each group item in the chat list (which is shown all the time) might be more expensive than the less frequent case, when the context menu item is opened. I would say it's fine to load the full chat when the context menu is opened (which I just introduced in the last refactoring) and then check in the contact list.

So let's get rid of self_in_group

is_sending_location: chat.is_sending_locations(),
archived: visibility == ChatVisibility::Archived,
is_archived: visibility == ChatVisibility::Archived,
pinned: visibility == ChatVisibility::Pinned,
is_pinned: visibility == ChatVisibility::Pinned,
is_muted: chat.is_muted(),
is_contact_request: chat.is_contact_request(),
Expand Down
Loading