Skip to content
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.

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

Sdk range request #7489

Merged
merged 6 commits into from
Oct 17, 2024
Merged

Sdk range request #7489

merged 6 commits into from
Oct 17, 2024

Conversation

charlag
Copy link
Contributor

@charlag charlag commented Aug 30, 2024

(stacked on top of #7438

#7679


impl MailFolder {
fn mail_set_kind(&self) -> MailSetKind {
MailSetKind::try_from(self.folderType as u64).unwrap()
Copy link
Contributor

Choose a reason for hiding this comment

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

This could break if we add more MailSetKind types, and it won't be nice to debug. I'd do one of these two things:

  • expect("Invalid mail set kind") if this is not going to happen
  • unwrap_or(MailSetKind::CUSTOM) if it is a possibility

.memberships
.iter()
.find(|m| m.groupType == Some(GroupType::Mail.raw_value() as i64))
.unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

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

I am a little cautious around unwrapping based on data provided by the server.

I don't think there will ever be an instance where we won't have a Mail group, but just in case, I'd maybe use expect("User is not part of a mail group")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Most of these mail functions are not production ready yet but you are right, we should harden them a bit

}

impl GroupType {
pub fn raw_value(&self) -> u64 {
Copy link
Contributor

Choose a reason for hiding this comment

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

For field-less enums, you can use the as operator instead of trying to match every variant. You can also derive Copy/Clone so that you can pass by value without needing to call .clone() or create a reference.

Additionally, you can manually specify what each variant is equal to.

Example:

#[repr(u64)]
#[derive(Copy, Clone, PartialEq, Debug)]
pub enum GroupType {
    User = 0,
    Admin = 1,
    MailingList = 2,
    // etc
}

impl GroupType {
    pub fn raw_value(self) -> u64 {
        self as u64
    }
}

assert_eq!(2, GroupType::MailingList as u64);
assert_eq!(2, GroupType::MailingList.raw_value());

Copy link
Contributor

Choose a reason for hiding this comment

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

I see you're also using num_enum, too, which could also be what you want.

@charlag
Copy link
Contributor Author

charlag commented Oct 10, 2024

Should be better now! thank you for the suggestions (I amended one syntax issue)

@wrdhub wrdhub requested a review from paw-hub October 14, 2024 07:38
@charlag charlag merged commit 827d5c8 into dev-mail Oct 17, 2024
2 checks passed
@charlag charlag deleted the sdk-range-request branch October 17, 2024 12:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support loading encrypted entities w/ former group key in SDK
2 participants