PipedreamHQ / pipedream

Connect APIs, remarkably fast. Free for developers.
https://pipedream.com
Other
8.32k stars 5.27k forks source link

3877 notion page property item #3945

Closed ljdatasci closed 1 year ago

ljdatasci commented 1 year ago

I removed the async options from the propertyId. In order to get a paginated list of propertyIds, you need to request them from the Retrieve a Database endpoint, for the pages that are part of a database and have a database_id https://developers.notion.com/reference/retrieve-a-page-property. If the page is not part of a database, then the only propertyId that can be used is 'title'. The user can create a step prior to Retrieve a Database and pass a propertyId as a step variable. Below is a screenshot of the results of the Retrieve P Screen Shot 2022-08-06 at 9 01 51 AM age Property Item action:

vercel[bot] commented 1 year ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
pipedream-docs ✅ Ready (Inspect) Visit Preview Aug 10, 2022 at 4:07PM (UTC)
pipedream-docs-redirect-do-not-edit ✅ Ready (Inspect) Visit Preview Aug 10, 2022 at 4:07PM (UTC)
dylburger commented 1 year ago

Thank you so much for submitting this! We've added it to our backlog to review, and our team has been notified.

dylburger commented 1 year ago

Thanks for submitting this PR! When we review PRs, we follow the Pipedream component guidelines. If you're not familiar, here's a quick checklist:

ctrlaltdylan commented 1 year ago

This PR closes #3877.

ctrlaltdylan commented 1 year ago

I have moved the corresponding issue for this PR into the QA column, it should be picked up by a QA engineer and they'll leave feedback. Thanks again for submitting a PR! :)

andrewjschuang commented 1 year ago

Just wondering, would your use case be resolved using the Retrieve Page action?

ljdatasci commented 1 year ago

Yes the use case would be resolved using the Retrieve Page actions (see screenshot). As I'm reviewing the Notion API documentation again, the property object can be retrieved from the Retrieve a Database endpoint. The documentation says that prior to version 2022-06-28, the Page Object displayed the page properties. As of version 2022-06-28, the Page Object displays the page properties only when there's a Create or Update page request.

From my test, the user can get the property IDs from both the Retrieve Database action and Retrieve a Page action. Going forward with version 2022-06-28, the user can only get the property IDs from the Page Object if there's a Create or Update request. So it seems that it would be best if the user runs one of the two actions and passes the property ID as a step variable. I tried a paginated list of property ID, but with the newest version, the Retrieve a Page action would not work unless run the Create or Update a Page action first https://developers.notion.com/reference/property-value-object. What do you think?

On Mon, Aug 8, 2022 at 10:22 AM Andrew Chuang @.***> wrote:

Just wondering, would your use case be resolved using the Retrieve Page action?

— Reply to this email directly, view it on GitHub https://github.com/PipedreamHQ/pipedream/pull/3945#issuecomment-1208197284, or unsubscribe https://github.com/notifications/unsubscribe-auth/AMO5IV6P6AHEQHLK3VMZ2BLVYEJ3RANCNFSM55YTBRJA . You are receiving this because you were mentioned.Message ID: @.***>

andrewjschuang commented 1 year ago

I see, so the endpoints don't return the values, only the ids? Yes, we can proceed with the newly developed action

ljdatasci commented 1 year ago

I updated the above to say 'property object' not 'property keys'.

On Mon, Aug 8, 2022 at 12:18 PM Andrew Chuang @.***> wrote:

I see, so the endpoints don't return the values, only the ids? Yes, we can proceed with the newly developed action

— Reply to this email directly, view it on GitHub https://github.com/PipedreamHQ/pipedream/pull/3945#issuecomment-1208332277, or unsubscribe https://github.com/notifications/unsubscribe-auth/AMO5IV53QI3BIWO6X6NUY43VYEXNNANCNFSM55YTBRJA . You are receiving this because you were mentioned.Message ID: @.***>

andrewjschuang commented 1 year ago

Thanks for the explanation and the links. What if we add a databaseId prop before the pageId prop and use async options() to fetch the database properties and display them in the propertyId prop?

ljdatasci commented 1 year ago

We would need to check the parent.type on the Page object. If parent.type is 'workspace' or 'page_id', then the only property ID allowed is 'title'. If parent.type is 'database_id', then we use the database_id from the Page Object, run a Retrieve a Database action, and return the property IDs.

On Mon, Aug 8, 2022 at 2:42 PM Andrew Chuang @.***> wrote:

Thanks for the explanation and the links. What if we add a databaseId prop before the pageId prop and use async options() to fetch the database properties and display them in the propertyId prop?

— Reply to this email directly, view it on GitHub https://github.com/PipedreamHQ/pipedream/pull/3945#issuecomment-1208476405, or unsubscribe https://github.com/notifications/unsubscribe-auth/AMO5IV6OBYQR2N4DGOEY5GLVYFIKLANCNFSM55YTBRJA . You are receiving this because you were mentioned.Message ID: @.***>

andrewjschuang commented 1 year ago

We would need to check the parent.type on the Page object. If parent.type is 'workspace' or 'page_id', then the only property ID allowed is 'title'. If parent.type is 'database_id', then we use the database_id from the Page Object, run a Retrieve a Database action, and return the property IDs.

Sounds good to me!

ljdatasci commented 1 year ago

I'll start working on it and add the code to the async options for the pageId.

On Mon, Aug 8, 2022 at 3:06 PM Andrew Chuang @.***> wrote:

We would need to check the parent.type on the Page object. If parent.type is 'workspace' or 'page_id', then the only property ID allowed is 'title'. If parent.type is 'database_id', then we use the database_id from the Page Object, run a Retrieve a Database action, and return the property IDs.

Sounds good to me!

— Reply to this email directly, view it on GitHub https://github.com/PipedreamHQ/pipedream/pull/3945#issuecomment-1208500583, or unsubscribe https://github.com/notifications/unsubscribe-auth/AMO5IV6DIKZGTCMRTQQS7DTVYFLFDANCNFSM55YTBRJA . You are receiving this because you were mentioned.Message ID: @.***>

andrewjschuang commented 1 year ago

Thank you!

ljdatasci commented 1 year ago

Thank you!

I'm getting error {"name":"UserError","message":"bad options response for prop: propertyId"} and I'm sure that my code is correct? I'm trying to pull the propertyIds from response object. Are other users having the same issue?

andrewjschuang commented 1 year ago

I'm getting error {"name":"UserError","message":"bad options response for prop: propertyId"} and I'm sure that my code is correct? I'm trying to pull the propertyIds from response object. Are other users having the same issue?

The error is that const { propIds } extracts undefined in this line below: const { propIds } = propKeys.length === 1 && propKeys.includes("title") It should be const propIds =

ljdatasci commented 1 year ago

I'm getting error {"name":"UserError","message":"bad options response for prop: propertyId"} and I'm sure that my code is correct? I'm trying to pull the propertyIds from response object. Are other users having the same issue?

The error is that const { propIds } extracts undefined in this line below: const { propIds } = propKeys.length === 1 && propKeys.includes("title") It should be const propIds =

Thank you. Here are the results of my testing. I can pull the property Ids for the pages whose parent type is 'database_id as seen in the screenshot below: Screen Shot 2022-08-09 at 12 03 40 PM

But when I try to pull a property Id for a page whose parent type is 'page_id' or 'workspace', nothing returns. What should return is a single property ID of 'title': Screen Shot 2022-08-09 at 12 04 07 PM

I committed my changes to the branch in the fork of the repo on my account.

andrewjschuang commented 1 year ago

Sorry for the delay, for some reason I missed the notification.

In this line, response.properties should be just response:

const { properties } = parentType === "database_id"
  ? await this.notion.retrieveDatabase(response.parent.database_id)
  : response;
ljdatasci commented 1 year ago

Sorry for the delay, for some reason I missed the notification.

In this line, response.properties should be just response:

const { properties } = parentType === "database_id"
  ? await this.notion.retrieveDatabase(response.parent.database_id)
  : response;

It's working now. Here are the results: Screen Shot 2022-08-09 at 6 35 20 PM

Screen Shot 2022-08-09 at 6 42 12 PM

andrewjschuang commented 1 year ago

Thanks for the changes! Moving to QA!

vunguyenhung commented 1 year ago

Hi everyone, all test cases are passed! Ready for release!

Test report Notion_3945.pdf

andrewjschuang commented 1 year ago

Merging on behalf of @ljdatasci. Thanks again for the contribution!