Implement page creation #37
Conversation
This is useful since you can then pass a SelectOption directly into a PropertyValue from a PropertyConfiguration.
| #[derive(Serialize, Deserialize, Debug, Eq, PartialEq, Clone)] | ||
| pub struct PageCreate { | ||
| pub parent: Parent, | ||
| pub properties: Properties, |
There was a problem hiding this comment.
The Properties struct today is geared towards the Response model (as every field contains its identifier and optional values). We probably need a struct to model the Create Request that needs to remove the identifier fields and enforce values.
There was a problem hiding this comment.
I don't think this is possible: imagine that the property gets renamed/created/etc between the request to find available properties and the one to create things. Then you're missing a property in spite of requiring the client to give all the known ones.
It's not a problem to intentionally exclude some properties as the caller, it just means your row will be missing some properties as well, which is not an error.
| properties: Properties { | ||
| properties: properties | ||
| .iter() | ||
| .map(|prop| (prop.as_id().to_string(), prop.to_owned())) |
There was a problem hiding this comment.
What happens if the PropertyValue supplied doesn't have an identifier?
There was a problem hiding this comment.
Can you clarify what you mean by not having an identifier? PropertyValue guarantees that the property has its respective id.
There was a problem hiding this comment.
Oh! What if it doesn't have a property? Then that property is blank in the resulting row. I know this because my program just ignores some properties.
There was a problem hiding this comment.
I was more coming at a perspective of should the Caller have to know of the PropertyValue's identifier before making the create_page call. This would require them to always have to make a GET request first to retrieve them in the case of a database.
Frankly, the identifiers for PropertyValues from Notion are a bit odd. Notion doesn't allow you to create multiple properties with the same name so the name is effectively the same as the identifier. The API Spec for Create Page even calls out that the key of the Map should be the Name OR identifier.
There was a problem hiding this comment.
In that case, I think maybe the Property thing should be refactored to store an alternative between the two possibilities, maybe? I'm not sure how to best approach it.
One strategy would be a slight abuse: put the name in as the identifier value even though it's not an identifier really.
There was a problem hiding this comment.
I think where my head was at (from the other comment) was that the existing "response" models shouldn't be reused as "request" models.
|
I don't think I have time to work on this more. If someone wants to take it and run with it, be my guest. |
I am currently working on a create page feature to use in a program I'm writing to do more pleasant data entry.
This PR is a work in progress, I still need to write some tests (after I get my program working past the risk-of-abandonment stage).