notion-sdk-py icon indicating copy to clipboard operation
notion-sdk-py copied to clipboard

Allow to set the in_trash property of a page using notion.pages.update()

Open jeromegit opened this issue 1 year ago • 1 comments

Hey @ramnes, Thanks for the great module!

Adding in_trash to the list in the line referenced below should do the trick: https://github.com/ramnes/notion-sdk-py/blob/d5e5f5c98ae5578ce4b0130b65ad6e8f9e92e02b/notion_client/api_endpoints.py#L235

Merci !

jeromegit avatar May 12 '24 00:05 jeromegit

Hey @jeromegit, thanks for opening the issue!

Indeed, we should add in_trash. Rlated changelog from Notion: https://developers.notion.com/page/changelog#changes-for-april-2024

PR welcome!

ramnes avatar May 17 '24 13:05 ramnes

Hey @jeromegit @ramnes , thanks for your comments on this issue! I've changed archived to in_trash in notion_client/api_endpoints.py

https://github.com/digzect/notion-sdk-py/blob/6b1eb8f9972bb851fc6585aa80a184b397bfd2d5/notion_client/api_endpoints.py#L235 , and the pytest coverage is now 100%. Am I ready to create a PR at this point? This is my first time contributing, so I would appreciate your guidance.

jjongguet avatar Jul 12 '24 16:07 jjongguet

Why removing archived? The changelog entry above says that both are still supported. Did it get removed in the meantime?

Otherwise yes, just push your commit to a branch on your repository and open a PR!

ramnes avatar Jul 12 '24 18:07 ramnes

Thank you for your response. I've reviewed the Notion API documentation:

  • https://developers.notion.com/reference/patch-page uses in_trash instead of archived
  • https://developers.notion.com/reference/update-a-block still uses archived

The commit in question is specifically for the PagesEndpoint, which is why I changed archived to in_trash. Additionally, When using both archived and in_trash together in the code, the pytest coverage remains at 100%.

Given this information, how would you suggest we proceed? Should we keep only in_trash for the PagesEndpoint, or include both for maximum compatibility?

jjongguet avatar Jul 13 '24 00:07 jjongguet

I'd say let's take a look at notion-sdk-js and do whatever the Notion developers did there. :)

ramnes avatar Jul 13 '24 12:07 ramnes

I've looked at notion-sdk-js and using both "archived" and "in_trash" together. https://github.com/makenotion/notion-sdk-js/blob/7950edc034d3007b0612b80d3f424baef89746d9/src/api-endpoints.ts#L10514 Thank you for your guidance!

jjongguet avatar Jul 13 '24 14:07 jjongguet

Fixed in #236. Thanks!

ramnes avatar Jul 13 '24 22:07 ramnes