IronCoreLabs / ironoxide

Rust SDK for IronCore Privacy Platform
https://docs.rs/ironoxide
GNU Affero General Public License v3.0
10 stars 3 forks source link

`GroupGetResult` has redundant types #269

Open skeet70 opened 2 years ago

skeet70 commented 2 years ago

image

Seems like Option<Vec<UserId>> is just Vec<UserId> with extra steps. Is there information communicated by None that isn't communicated by [ ]? If so, we should document that. If not, we should improve the interface so you don't have to check for both None and .len() > 0 when using it.

giarc3 commented 2 years ago

I believe the intended distinction is that None comes back from admin_list, member_list, and owner if you aren't a member/admin of the group. But this should definitely be documented better.

BobWall23 commented 2 years ago

Currently, the group permissions include is_admin and is_member - if both are false, you can't get any information about the group admins and members. So this information is somewhat redundant with the none Option for the admin / member lists.

Getting rid of the Option around the vecs gets rid of an extra layer of unwrapping in the consumer, which seems good. We would put this in a v2 ironoxide API feature branch. Not needed any time soon - we can accumulate things in the feature branch until we have enough to release.

skeet70 commented 2 years ago

@coltfred brought up that we could return an enum type of the GroupMeta and GroupFull types with an impl that lets you use the things that will always be there, and you could match if you need to use something that might not be there (owner or needs_rotation).