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.

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

feat(notion): added notion create database row action #195

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

AndrewKaranja
Copy link
Contributor

@AndrewKaranja AndrewKaranja commented Jan 14, 2025

Describe your changes

Implementation of Create Database Row Action.

Dynamic Schema Mapping:

  • Leveraged the fetch-database action to retrieve and parse the schema for the target database.
  • Supported all Notion property types (title, select, relation, etc.) by dynamically mapping input data to the inferred schema.

Added tests to validate the action with various database schemas.

Issue ticket number and link

[304] (https://linear.app/nango/issue/EXT-340/notion-create-database-row-action)

Checklist before requesting a review (skip if just adding/editing APIs & templates)

  • I added tests, otherwise the reason is:
  • External API requests have retries
  • Pagination is used where appropriate
  • The built in nango.paginate call is used instead of a while (true) loop
  • Third party requests are NOT parallelized (this can cause issues with rate limits)
  • If a sync requires metadata the nango.yaml has auto_start: false
  • If the sync is a full sync then track_deletes: true is set
  • I followed the best practices and guidelines from the Writing Integration Scripts doc

@AndrewKaranja AndrewKaranja self-assigned this Jan 14, 2025
Copy link

linear bot commented Jan 14, 2025


export default async function runAction(nango: NangoAction, input: CreateDatabaseRowInput): Promise<SuccessResponse> {
const { databaseId, properties } = input;

Copy link
Member

Choose a reason for hiding this comment

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

We should verify that properties and databaseId are set at the beginning of the action

};

const entries: RowEntry[] = [];
for await (const dbPages of nango.paginate<any>(queryProxyConfig)) {
Copy link
Member

Choose a reason for hiding this comment

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

This should be typed, I believe NotionDatabase, correct?


const entries: RowEntry[] = [];
for await (const dbPages of nango.paginate<any>(queryProxyConfig)) {
for (const dbPage of dbPages) {
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason to iterate over every single database row. If you're building out the schema I would think only one row is necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mostly to cater for edge cases. In some notion setups, data.properties doesn’t always list every column. Especially if some columns were never populated. Scanning all rows ensures we pick up columns that might not appear in the top-level database object or in the first rows. Though I have updated the logic to first check if the top level data.properties exists before iterating through every single row.

Copy link
Member

@khaliqgant khaliqgant left a comment

Choose a reason for hiding this comment

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

See feedback inline. Casing should be handled as well:

{
  "properties": {
    "Name": "khaliq",
    "Tags": [
      "abc",
      "def"
    ]
  },
  "databaseId": "076a5b04-b18c-4636-b78a-bd6cebacad2d"
}

and

{
  "properties": {
    "name": "khaliq",
    "tags": [
      "abc",
      "def"
    ]
  },
  "databaseId": "076a5b04-b18c-4636-b78a-bd6cebacad2d"
}

should both work

- [Documentation History](https://github.com/NangoHQ/integration-templates/commits/main/integrations/notion/actions/create-database-row.md)

<!-- END GENERATED CONTENT -->

Copy link
Member

Choose a reason for hiding this comment

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

It would be useful to give more details of what steps are actually involved with passing in content.

Some things to document:

  1. You can find the databaseId by using the databases sync https://github.com/NangoHQ/integration-templates/blob/main/integrations/notion/syncs/databases.ts
  2. The properties input consists of the database column name and the value you want to populate the column with. This action dynamically formulates the data to correctly create the row in Notion

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@@ -0,0 +1,3 @@
{
"success": true
Copy link
Member

Choose a reason for hiding this comment

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

This output isn't representative of the updated output. Can you regenerate the mock data using the --save-responses flag?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Copy link
Member

@khaliqgant khaliqgant left a comment

Choose a reason for hiding this comment

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

Looks good. Couple of additional tweaks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants