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.

Sign up for GitHub

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

Jump to bottom

Implement page creation #37

Closed
wants to merge 6 commits into from
Closed

Implement page creation #37

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
06319d2
tracing: include request bodies
lf- Jan 14, 2022
5e14213
Add as_id for PropertyValue and PropertyConfiguration
lf- Jan 14, 2022
bfbc587
fix: rename incorrect "last_edit_by" to "last_edited_by"
lf- Jan 14, 2022
787d1a9
Remove SelectedValue which duplicates SelectOption
lf- Jan 14, 2022
9756257
Implement page creation
lf- Jan 14, 2022
39c82b3
Add the url property to Page objects
lf- Jan 16, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
38 changes: 37 additions & 1 deletion src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ use crate::models::error::ErrorResponse;
use crate::models::search::{DatabaseQuery, SearchRequest};
use crate::models::{Block, Database, ListResponse, Object, Page};
use ids::{AsIdentifier, PageId};
use models::properties::PropertyValue;
use models::{PageCreate, Parent, Properties};
use reqwest::header::{HeaderMap, HeaderValue};
use reqwest::{header, Client, ClientBuilder, RequestBuilder};
use tracing::Instrument;
Expand Down Expand Up @@ -82,7 +84,10 @@ impl NotionApi {
tracing::trace!(
method = request.method().as_str(),
url = url.as_str(),
"Sending request"
body = request
.body()
.and_then(|body| std::str::from_utf8(body.as_bytes()?).ok()),
"Sending JSON request"
);
let json = self
.client
Expand Down Expand Up @@ -176,6 +181,37 @@ impl NotionApi {
}
}

pub async fn create_page(
&self,
parent: &Parent,
properties: &[PropertyValue],
children: &[Block],
) -> Result<Page, Error> {
let new_page = PageCreate {
parent: parent.to_owned(),
properties: Properties {
properties: properties
.iter()
.map(|prop| (prop.as_id().to_string(), prop.to_owned()))
Copy link
Collaborator

Choose a reason for hiding this comment

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

What happens if the PropertyValue supplied doesn't have an identifier?

Copy link
Author

Choose a reason for hiding this comment

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

Can you clarify what you mean by not having an identifier? PropertyValue guarantees that the property has its respective id.

Copy link
Author

Choose a reason for hiding this comment

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

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.

Copy link
Collaborator

Choose a reason for hiding this comment

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

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.

Copy link
Author

Choose a reason for hiding this comment

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

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.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think where my head was at (from the other comment) was that the existing "response" models shouldn't be reused as "request" models.

.collect(),
},
children: children.to_owned(),
};

let result = self
.make_json_request(
self.client
.post("https://api.notion.com/v1/pages")
.json(&new_page),
)
.await?;

match result {
Object::Page { page } => Ok(page),
response => Err(Error::UnexpectedResponse { response }),
}
}

/// Query a database and return the matching pages.
pub async fn query_database<D, T>(
&self,
Expand Down
8 changes: 8 additions & 0 deletions src/models.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,13 @@ impl Properties {
}
}

#[derive(Serialize, Deserialize, Debug, Eq, PartialEq, Clone)]
pub struct PageCreate {
pub parent: Parent,
pub properties: Properties,
Copy link
Collaborator

Choose a reason for hiding this comment

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

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.

Copy link
Author

@lf- lf- Feb 7, 2022 •
edited

Choose a reason for hiding this comment

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

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.

pub children: Vec<Block>,
}

#[derive(Serialize, Deserialize, Debug, Eq, PartialEq, Clone)]
pub struct Page {
pub id: PageId,
Expand All @@ -189,6 +196,7 @@ pub struct Page {
pub archived: bool,
pub properties: Properties,
pub parent: Parent,
pub url: String,
}

impl Page {
Expand Down
67 changes: 56 additions & 11 deletions src/models/properties.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use crate::models::text::RichText;
use crate::models::users::User;

use super::{DateTime, Number, Utc};
use crate::ids::{DatabaseId, PageId, PropertyId};
use crate::ids::{AsIdentifier, DatabaseId, PageId, PropertyId};
use chrono::NaiveDate;
use serde::{Deserialize, Serialize};

Expand Down Expand Up @@ -185,14 +185,33 @@ pub enum PropertyConfiguration {
/// See <https://developers.notion.com/reference/database#last-edited-time-configuration>
LastEditedTime { id: PropertyId },
/// See <https://developers.notion.com/reference/database#last-edited-by-configuration>
LastEditBy { id: PropertyId },
LastEditedBy { id: PropertyId },
}

#[derive(Serialize, Deserialize, Debug, Eq, PartialEq, Clone)]
pub struct SelectedValue {
pub id: SelectOptionId,
pub name: String,
pub color: Color,
impl AsIdentifier<PropertyId> for PropertyConfiguration {
fn as_id(&self) -> &PropertyId {
match self {
PropertyConfiguration::Title { id, .. } => id,
PropertyConfiguration::Text { id, .. } => id,
PropertyConfiguration::Number { id, .. } => id,
PropertyConfiguration::Select { id, .. } => id,
PropertyConfiguration::MultiSelect { id, .. } => id,
PropertyConfiguration::Date { id, .. } => id,
PropertyConfiguration::Formula { id, .. } => id,
PropertyConfiguration::Relation { id, .. } => id,
PropertyConfiguration::Rollup { id, .. } => id,
PropertyConfiguration::People { id, .. } => id,
PropertyConfiguration::Files { id, .. } => id,
PropertyConfiguration::Checkbox { id, .. } => id,
PropertyConfiguration::Url { id, .. } => id,
PropertyConfiguration::Email { id, .. } => id,
PropertyConfiguration::PhoneNumber { id, .. } => id,
PropertyConfiguration::CreatedTime { id, .. } => id,
PropertyConfiguration::CreatedBy { id, .. } => id,
PropertyConfiguration::LastEditedTime { id, .. } => id,
PropertyConfiguration::LastEditedBy { id, .. } => id,
}
}
}

#[derive(Serialize, Deserialize, Debug, Eq, PartialEq, Clone)]
Expand Down Expand Up @@ -266,11 +285,11 @@ pub enum PropertyValue {
/// <https://developers.notion.com/reference/page#select-property-values>
Select {
id: PropertyId,
select: Option<SelectedValue>,
select: Option<SelectOption>,
},
MultiSelect {
id: PropertyId,
multi_select: Option<Vec<SelectedValue>>,
multi_select: Option<Vec<SelectOption>>,
bspradling marked this conversation as resolved.
Show resolved Hide resolved
},
Date {
id: PropertyId,
Expand Down Expand Up @@ -334,6 +353,32 @@ pub enum PropertyValue {
},
}

impl AsIdentifier<PropertyId> for PropertyValue {
fn as_id(&self) -> &PropertyId {
match self {
PropertyValue::Title { id, .. } => id,
PropertyValue::Text { id, .. } => id,
PropertyValue::Number { id, .. } => id,
PropertyValue::Select { id, .. } => id,
PropertyValue::MultiSelect { id, .. } => id,
PropertyValue::Date { id, .. } => id,
PropertyValue::Formula { id, .. } => id,
PropertyValue::Relation { id, .. } => id,
PropertyValue::Rollup { id, .. } => id,
PropertyValue::People { id, .. } => id,
PropertyValue::Files { id, .. } => id,
PropertyValue::Checkbox { id, .. } => id,
PropertyValue::Url { id, .. } => id,
PropertyValue::Email { id, .. } => id,
PropertyValue::PhoneNumber { id, .. } => id,
PropertyValue::CreatedTime { id, .. } => id,
PropertyValue::CreatedBy { id, .. } => id,
PropertyValue::LastEditedTime { id, .. } => id,
PropertyValue::LastEditedBy { id, .. } => id,
}
}
}

/// <https://developers.notion.com/reference/page#rollup-property-value-element>
#[derive(Serialize, Deserialize, Debug, Eq, PartialEq, Clone)]
#[serde(tag = "type")]
Expand All @@ -350,10 +395,10 @@ pub enum RollupPropertyValue {
},
/// <https://developers.notion.com/reference/page#select-property-values>
Select {
select: Option<SelectedValue>,
select: Option<SelectOption>,
},
MultiSelect {
multi_select: Option<Vec<SelectedValue>>,
multi_select: Option<Vec<SelectOption>>,
},
Date {
date: Option<DateValue>,
Expand Down
3 changes: 2 additions & 1 deletion src/models/tests/page.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,5 +54,6 @@
}
]
}
}
},
"url": "https://www.notion.so/Avocado-b55c9c91384d452b81dbd1ef79372b75"
}
3 changes: 2 additions & 1 deletion src/models/tests/query_result.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@
}
]
}
}
},
"url": "https://www.notion.so/2e01e904febd43a0ad028eedb903a82c"
}
],
"next_cursor": null,
Expand Down
16 changes: 16 additions & 0 deletions src/models/text.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,9 @@ pub struct Annotations {
#[derive(Serialize, Deserialize, Debug, Eq, PartialEq, Clone)]
pub struct RichTextCommon {
pub plain_text: String,
#[serde(skip_serializing_if = "Option::is_none")]
pub href: Option<String>,
#[serde(skip_serializing_if = "Option::is_none")]
pub annotations: Option<Annotations>,
}

Expand Down Expand Up @@ -91,4 +93,18 @@ impl RichText {
}
}
}

pub fn from_plain_text(s: &str) -> RichText {
RichText::Text {
rich_text: RichTextCommon {
plain_text: s.to_string(),
href: None,
annotations: None,
},
text: Text {
content: s.to_string(),
link: None,
},
}
}
}