feat: Add Contact::get_or_gen_color. Use it in CFFI and JSON-RPC to avoid gray self-color (#7374) #7388
Conversation
iequidoo
force-pushed
the
iequidoo/no-gray-self-color
branch
from
65c569d to
ab8d5f3
Compare
deltachat-ffi/src/lib.rs
Outdated
| } | ||
| let ffi_contact = &*contact; | ||
| let ctx = &*ffi_contact.context; | ||
| if ffi_contact.contact.id == ContactId::SELF |
There was a problem hiding this comment.
This logic should go into Contact.get_color() or Contact loading code in deltachat crate, not into CFFI. This will also make it easy to test with a Rust test.
This issue also exists on Desktop and desktop is not using EDIT: did not notice that there is a similar fix in deltachat-jsonrpc, this will be unnecessary then.deltachat-ffi at all, so this does not fix the issue for dekstop.
Generally, no complex logic should go into deltachat-ffi and deltachat-jsonrpc crates, they are thin wrappers providing the APIs. deltachat-jsonrpc has a bit of logic here and there, but it is a bad example.
There was a problem hiding this comment.
Moved the logic to Contact::get_color(). Already tried to generate own keypair in get_by_id() before and that broke Core tests on key import
There was a problem hiding this comment.
And even that didn't work. JSON-RPC tests call get_color() in unclear ways and instead of trying to understand how it happens i decided to add get_or_gen_color(), this makes the API clearer and protects tests on key import from future breakages
iequidoo
force-pushed
the
iequidoo/no-gray-self-color
branch
from
ab8d5f3 to
652a700
Compare
iequidoo
changed the title
iequidoo
force-pushed
the
iequidoo/no-gray-self-color
branch
2 times, most recently
from
bee79fd to
3e8496f
Compare
iequidoo
changed the title
…void gray self-color (#7374) `Contact::get_color()` returns gray if own keypair doesn't exist yet and we don't want any UIs displaying it. Keypair generation can't be done in `get_color()` or `get_by_id_optional()` to avoid breaking Core tests on key import. Also this makes the API clearer, pure getters shouldn't modify any visible state.
iequidoo
force-pushed
the
iequidoo/no-gray-self-color
branch
from
3e8496f to
e3ddcb4
Compare
|
For rustfmt issue on the |
Contact::get_color()returns gray if own keypair doesn't exist yet and we don't want any UIsdisplaying it. Keypair generation can't be done in
get_color()orget_by_id_optional()to avoidbreaking Core tests on key import. Also this makes the API clearer, pure getters shouldn't modify
any visible state.
A previous failed attempt: #7289
Close #7374