Improve/property access

[b3ngg]

This is kind of a bigger one.
I always found it pretty annoying to access a property value and hard to debug when you got something wrong. It wasn't possible to access a property by ID.
So this first of all adds get_by_name and get_by_id to the Properties struct.

To make it easier to access the values of a property in cases where you know the property type or expect a specific property type, this adds the FromPropertyValue trait and a bunch of convenience methods.

The trait also allows the user to define a specific type in which they want to receive the value of a property.

For easy access to the boolean state of a checkbox property, the following boilerplate was necessary, and you still had annoying error handling.

let is_active = match page.properties.properties.get("active").unwrap() {
     PropertyValue::Checkbox { checkbox, .. } => Ok(checkbox),
      _ => Err("Wrong property type"), // What property types is it instead???
};

The above code can now be done as follows:

let is_active = page
    .properties
    .get_by_name("active")
    .unwrap() // None if property is missing
    .expect_checkbox() // or `.expect_value::<bool>()` if not explicit about the property type
    .unwrap() // Helpful error message with expected and actual types

Example using the trait for a custom type:

// We can use the new type pattern to create a custom type that implements FromPropertyValue
// This allows us easy access to property value in our desired type
struct TextValue(String);
impl FromPropertyValue for TextValue {
    fn from_property_value(value: PropertyValue) -> Result<Self, WrongPropertyTypeError> {
        match value {
            PropertyValue::Text { rich_text, .. } => {
                let combined_text = rich_text
                    .iter()
                    .map(|rt| rt.plain_text().to_string())
                    .collect::<Vec<String>>()
                    .join("");
                Ok(TextValue(combined_text))
            }
            _ => Err(WrongPropertyTypeError {
                expected: vec!["Text".to_string()],
                actual: value.type_name(),
            }),
        }
    }
}

assert_eq!(
    page.properties
        .get_by_name("Text")
        .unwrap()
        .expect_value::<TextValue>()
        .unwrap()
        .0,
    "hello world".to_string()
);

I've found this very useful in my application where I have to access a lot of properties. But it is, of course, a larger addition. Let me know what you think.

[atcol]

I like the idea. Did you consider TryFrom?

There are quite a lot of Clippy errors BTW.

[b3ngg]

Using TryFrom instead of a custom trait would definitely be a consideration. I don't know. I mostly end up writing my own trait as it feels more explicit for me. What would be the benefit of using TryFrom?

Where do you see the Clippy errors? Locally, there are none showing up for me, and in CI, there are also none…

[b3ngg]

In preparation for better page creation and support for page property updates, the structure of properties has to change more fundamentally. This, unfortunately, also includes breaking changes. I currently see no way around this.

The problem is that page creation or updating doesn't need the ID inside a property variant; it just needs the property value. The new, more flexible structure solves this and also makes the code more concise in other cases.

I also removed the custom FromPropertyValue trait in favor of TryFrom, which is indeed much more idiomatic. The convenience methods for this are also now wrapped inside the ExpectProperty trait, which makes the change a bit less intrusive.