feat: Support AccountInfo.is_default in II backend API
#3363
Conversation
aterga
requested review from
sea-snake,
lmuntaner and
danblackadder
and removed request for
sea-snake
| last_used : opt Timestamp; | ||
| // Configurable properties | ||
| name : opt text; | ||
| is_default : opt bool; |
There was a problem hiding this comment.
@danblackadder, this is how the FE can tell which account is the default.
In case this field is null, I suggest assuming that the default account = the primary account.
There was a problem hiding this comment.
Why do we need to set it as optional? I don't think we need to keep backwards compatibility.
| pub origin: FrontendHostname, | ||
| pub last_used: Option<Timestamp>, | ||
| pub name: Option<String>, | ||
| pub is_default: Option<bool>, |
There was a problem hiding this comment.
Does it need to be an Option?
There was a problem hiding this comment.
Adding all new record fields as optional is the recommended way to extend a Candid API.
Since the II has been developed in a special way (with BE and FE changes being synchronized), I've explored also the alternative of adding the field as non-optional, see here
We should discuss which approach to follow (I'm still leaning towards optional fields).
Motivation
This is the first step towards supporting the new concept of default accounts. The FE needs a way to distinguish default and non-default accounts associates with a (identity, application) pair.
Changes
Add the
is_defaultto the Candid API and propagate it through the Rust types. For now, we keep the value set to None in all places.Tests
Existing CI tests should suffice.