[Fix] Introducing Pagination Support for Notion API in Page and Database Responses #59

brf153 · 2024-01-31T17:08:03Z

Issue(s)

closes #38

Acceptance Criteria fulfillment

Check how the pagination is working in the api
Make changes to searchPage and searchDatabase methods.

Proposed changes (including videos or screenshots):

Made changes to searchPage method. Now, we are getting all the pages made in a workspace.
Screenshot of the length of the final results after concatenating all the response.data.results

Signed-off-by: brf153 <153hsb@gmail.com>

brf153 · 2024-01-31T17:09:47Z

@Nabhag8848 I have resolved the merge conflicts, but unfortunately, I mistakenly force-pushed the changes, leading to the dissolution of my previous pull request. However, upon further inspection, I can confirm that the changes are still present.

brf153 · 2024-01-31T17:11:54Z

The Changes

WhatsApp.Video.2024-01-23.at.10.13.26.mp4

WhatsApp.Video.2024-01-24.at.10.30.00.mp4

WhatsApp.Video.2024-01-24.at.10.04.29.mp4

Nabhag8848 · 2024-03-09T06:18:35Z

@brf153 Reviewing this right now, Sorry for So much delay on this. 🙏🏻

brf153 · 2024-03-09T13:46:05Z

@Nabhag8848 No problem at all! I appreciate you getting back to me. Take your time with the review, and let me know if there are any further changes required. Looking forward to hearing your thoughts!

Nabhag8848

@brf153 Added examples and comments for changes, lemme know once you are done, will review back, quickly.

Nabhag8848 · 2024-03-09T18:12:20Z

src/lib/NotionSDK.ts

+
+    private async recursiveSearchPages(token: string, cursor: string): Promise<Array<IPage>> {
+        const response = await this.http.post(NotionApi.SEARCH, {
+            data: {
+                start_cursor: cursor,
+            },
+            headers: {
+                Authorization: `Bearer ${token}`,
+                "Content-Type": NotionApi.CONTENT_TYPE,
+                "User-Agent": NotionApi.USER_AGENT,
+                "Notion-Version": this.NotionVersion,
+            },
+        });
+
+        if (response.statusCode.toString().startsWith("2")) {
+
+
+            let results: Array<IPage> = response.data.results;
+
+            if (response.data.has_more === true) {
+
+                const recursiveResults = await this.recursiveSearchPages(token, response.data.next_cursor);
+
+                results.push(...recursiveResults);
+            }
+
+            return results;
+        } else {
+            throw new AppsEngineException(`Error While Searching Pages: ${response.content}`);
+        }
+    }
+


Suggested change

private async recursiveSearchPages(token: string, cursor: string): Promise<Array<IPage>> {

const response = await this.http.post(NotionApi.SEARCH, {

data: {

start_cursor: cursor,

},

headers: {

Authorization: `Bearer ${token}`,

"Content-Type": NotionApi.CONTENT_TYPE,

"User-Agent": NotionApi.USER_AGENT,

"Notion-Version": this.NotionVersion,

},

});

if (response.statusCode.toString().startsWith("2")) {

let results: Array<IPage> = response.data.results;

if (response.data.has_more === true) {

const recursiveResults = await this.recursiveSearchPages(token, response.data.next_cursor);

results.push(...recursiveResults);

}

return results;

} else {

throw new AppsEngineException(`Error While Searching Pages: ${response.content}`);

}

}

- Better Approach: public async searchPages( token: string, cursor?: string ): Promise<Array<IPage> | Error> { try { const data = { filter: { value: NotionObjectTypes.PAGE, property: NotionObjectTypes.PROPERTY, }, start_cursor: cursor, }; const response = await this.http.post(NotionApi.SEARCH, { data, headers: { Authorization: `Bearer ${token}`, "Content-Type": NotionApi.CONTENT_TYPE, "User-Agent": NotionApi.USER_AGENT, "Notion-Version": this.NotionVersion, }, }); if (!response.statusCode.toString().startsWith("2")) { return this.handleErrorResponse( response.statusCode, `Error While Searching Pages: `, response.content ); } const { results, next_cursor, has_more } = response.data; if (has_more === true) { const recursiveResults = await this.searchPages( token, next_cursor ); if (recursiveResults instanceof Error) { return recursiveResults; } results.push(...recursiveResults); } if (cursor) return results; const result: Array<IPage> = []; results.forEach(async (item) => { const pageObject = await this.getPageObjectFromResults(item); if (pageObject) { result.push(pageObject); } }) return result; } catch (err) { throw new AppsEngineException(err as string); } }

Hey @brf153 Can't we have instead implement recursion without using the helper extra function. similar to above, can we change THIS ONE and REST OF THE Implementation USING this apporach make sure we don't need to use extra method and avoid having duplicacy of code.

Also make sure about this case what if during the paginated requests one of the requests get error from server in that case we need to assume that whole request is Error. above example solves that. Your implementation doesn't include that.

lets use forEach loops and lets stick to code already have to maintain consistency.

Also there is a bug in getPageObjectFromResults:

private async getPageObjectFromResults( item, emoji: boolean = false ): Promise<IPage | null> { const typesWithTitleProperty = [ NotionObjectTypes.WORKSPACE.toString(), NotionObjectTypes.PAGE_ID.toString(), ]; const parentType: string = item.parent.type; const properties = item.properties; const pageId: string = item.id; if (typesWithTitleProperty.includes(parentType)) { const pageName: string = properties.title.title[0]?.text?.content || NotionObjectTypes.UNTITLED; return this.returnPage(pageName, pageId, emoji); } // title property either be at first or last position const columns = Object.keys(properties); const firstColumn = columns[0]; const lastColumn = columns[columns.length - 1]; // title at first position and has subpage if (properties[firstColumn].title) { <- Removed the length statement here this is updated version, remove in your PR code as well const name: string = properties[firstColumn].title[0]?.text?.content || NotionObjectTypes.UNTITLED; return this.returnPage(name, pageId, emoji); } //title at last position and has subpage if (properties[lastColumn].title) { <- Removed the length statement here this is updated version as well. const name: string = properties[lastColumn].title[0]?.text?.content || NotionObjectTypes.UNTITLED; return this.returnPage(name, pageId, emoji); } return null; }

So when we get the page from the api: we need to make sure we get all the pages, removing that length statement solves the Untitled pages not being shown:

parent page

sub pages

database row (this are also the pages)

page without titled should get Untitled title

If you currently will request there are 140 pages, in your workspace you shared

Nabhag8848 · 2024-03-09T18:28:05Z

src/lib/NotionSDK.ts

+    private getNotionApiHeaders(token: string): Record<string, string> {
+        return {
+            Authorization: `Bearer ${token}`,
+            "Content-Type": NotionApi.CONTENT_TYPE,
+            "User-Agent": NotionApi.USER_AGENT,
+            "Notion-Version": this.NotionVersion,
+        };
+    }
+


As you have Introduce this method, Lets change in entire file, where we are passing this exact same header and lets make the lines of code less and call this method instead of hard coding

INCLUDING the code changes i shared.

Eg: const response = await this.http.post(NotionApi.SEARCH, { headers: this.getNotionApiHeaders(token) });

Nabhag8848 · 2024-03-09T18:30:38Z

src/lib/NotionSDK.ts

-    public async searchDatabases(
-        token: string
-    ): Promise<Array<IDatabase> | Error> {
+    public async recursiveSearchDatabases(token: string, startCursor?: string): Promise<Array<IDatabase>> {
        try {
-            const response = await this.http.post(NotionApi.SEARCH, {
-                data: {
-                    filter: {
-                        value: NotionObjectTypes.DATABASE,
-                        property: NotionObjectTypes.PROPERTY,
+            let response;
+
+            if (startCursor) {
+                response = await this.http.post(NotionApi.SEARCH, {
+                    data: {
+                        filter: {
+                            value: NotionObjectTypes.DATABASE,
+                            property: NotionObjectTypes.PROPERTY,
+                        },
+                        start_cursor: startCursor,
                    },
-                },
-                headers: {
-                    Authorization: `Bearer ${token}`,
-                    "Content-Type": NotionApi.CONTENT_TYPE,
-                    "User-Agent": NotionApi.USER_AGENT,
-                    "Notion-Version": this.NotionVersion,
-                },
-            });
+                    headers: this.getNotionApiHeaders(token),
+                });
+            } else {
+                response = await this.http.post(NotionApi.SEARCH, {
+                    data: {
+                        filter: {
+                            value: NotionObjectTypes.DATABASE,
+                            property: NotionObjectTypes.PROPERTY,
+                        },
+                    },
+                    headers: this.getNotionApiHeaders(token),
+                });
+            }

            if (!response.statusCode.toString().startsWith("2")) {
-                return this.handleErrorResponse(
-                    response.statusCode,
-                    `Error While Searching Databases: `,
-                    response.content
-                );
+                throw new AppsEngineException(`Error While Searching Databases: ${response.content}`);
            }
-
-            const { results } = response.data;
-
+    
+            const { results, has_more, next_cursor } = response.data;
+    
            const result: Array<IDatabase> = [];
+
            results.forEach(async (item) => {
-                const objectType: string = item?.[NotionObjectTypes.OBJECT];
-                const databaseObject = await this.getDatabaseObjectFromResults(
-                    item
-                );
-
+                const databaseObject = await this.getDatabaseObjectFromResults(item);
                result.push(databaseObject);
            });
+
+            if (has_more) {
+                const recursiveResults = await this.recursiveSearchDatabases(token, next_cursor);
+                result.push(...recursiveResults);
+            }
+
            return result;
        } catch (err) {
            throw new AppsEngineException(err as string);
        }
    }
+
+    public async searchDatabases(token: string): Promise<Array<IDatabase> | Error> {
+        try {
+            const results = await this.recursiveSearchDatabases(token);
+            return results;
+        } catch (err) {
+            throw new AppsEngineException(err as string);
+        }
+    }
+


Implement making sure We have just one method and don't have extra helper method to avoid duplicacy as approached shared above.

Also make sure about this case what if during the paginated requests one of the requests get error from server in that case we need to assume that whole set of request is Error. above example solves that.

Applies to all three: searchDatabases, searchPages, searchPagesAndDatabases

Nabhag8848 · 2024-03-09T18:32:37Z

src/lib/NotionSDK.ts

@@ -492,31 +533,34 @@ export class NotionSDK implements INotionSDK {
        }
    }

-    public async searchPagesAndDatabases(


Change according to approach similar to shared.

Also make sure about this case what if during the paginated requests one of the requests get error from server in that case we need to assume that whole request is Error. above example solves that.

Nabhag8848 · 2024-03-09T18:33:37Z

src/lib/NotionSDK.ts

+            if (startCursor) {
+                response = await this.http.post(NotionApi.SEARCH, {
+                    data: {
+                        start_cursor: startCursor,
+                    },
+                    headers: this.getNotionApiHeaders(token),
+                });
+            } else {
+                response = await this.http.post(NotionApi.SEARCH, {
+                    headers: this.getNotionApiHeaders(token),
+                });
+            }
+


You can use conditional statments in object instead of making if else statement for request. implement in new implementation take reference from example code shared.

Nabhag8848 · 2024-03-09T18:40:03Z

src/lib/NotionSDK.ts

-                    `Error While Searching Databases: `,
-                    response.content
-                );
+                throw new AppsEngineException(`Error While Searching Databases: ${response.content}`);


Suggested change

throw new AppsEngineException(`Error While Searching Databases: ${response.content}`);

return this.handleErrorResponse(

response.statusCode,

`Error While Searching Databases: `,

response.content

);

never throw an error for an usual error response, we are handling this error to later don't open a modal if we don't get the database

brf153 · 2024-04-03T04:58:17Z

@Nabhag8848 working on this. Sorry for the delay

Nabhag8848 · 2024-04-04T17:17:41Z

@Nabhag8848 working on this. Sorry for the delay

No Worries, Thanks ! @brf153 Looking forward 🚀

… in the original function as recommended Signed-off-by: brf153 <153hsb@gmail.com>

brf153 · 2024-04-06T03:23:33Z

@Nabhag8848 I have made the changes, and your suggestions have greatly helped me. Thank you for the recommendations.

Nabhag8848 · 2024-04-06T15:39:13Z

@Nabhag8848 I have made the changes, and your suggestions have greatly helped me. Thank you for the recommendations.

@brf153 🙏 will review it, by tomorrow thanks!
There are Api Calls within NotionSDK which wtill doesn't use getAPiHeaders methods. i believe you can use that.

Try checking all the features once changing that and see if nothing is getting break.

Nabhag8848

Hey @brf153 Added some comments, few more Changes, we can merge this after. Lemme know once you are done.

Thanks !

Nabhag8848 · 2024-04-08T06:43:35Z

src/lib/NotionSDK.ts

+
+            const response = await this.http.post(NotionApi.SEARCH, {
+                data,
+headers: this.getNotionApiHeaders(token),


Indent this header

Nabhag8848 · 2024-04-08T06:46:39Z

src/lib/NotionSDK.ts

+            return this.handleErrorResponse(
+                err.statusCode,
+                `Error While Searching Databases: `,
+                err.content
+            );


If you will have a look on previous comments again I meant Usual Errors, Errors which will go into Catch are not usual which means those errors are not known and weren't expected anytime.

Suggested change

return this.handleErrorResponse(

err.statusCode,

`Error While Searching Databases: `,

err.content

);

throw new AppsEngineException(err as string);

Nabhag8848 · 2024-04-08T06:47:04Z

src/lib/NotionSDK.ts

-                    "User-Agent": NotionApi.USER_AGENT,
-                    "Notion-Version": this.NotionVersion,
-                },
+headers: this.getNotionApiHeaders(token),


Nabhag8848 · 2024-04-08T06:47:10Z

src/lib/NotionSDK.ts

-                    "User-Agent": NotionApi.USER_AGENT,
-                    "Notion-Version": this.NotionVersion,
-                },
+headers: this.getNotionApiHeaders(token),


Nabhag8848 · 2024-04-08T06:47:25Z

src/lib/NotionSDK.ts

-                    "Notion-Version": this.NotionVersion,
-                },
+                data,
+headers: this.getNotionApiHeaders(token),


Nabhag8848 · 2024-04-08T06:47:36Z

src/lib/NotionSDK.ts

-                    "User-Agent": NotionApi.USER_AGENT,
-                    "Notion-Version": this.NotionVersion,
-                },
+headers: this.getNotionApiHeaders(token),


Nabhag8848 · 2024-04-08T06:47:44Z

src/lib/NotionSDK.ts

-                    "User-Agent": NotionApi.USER_AGENT,
-                    "Notion-Version": this.NotionVersion,
-                },
+headers: this.getNotionApiHeaders(token),


Nabhag8848 · 2024-04-08T06:48:32Z

src/lib/NotionSDK.ts

+            return this.handleErrorResponse(
+                err.statusCode,
+                `Error While Searching Databases: `,
+                err.content
+            );


Suggested change

return this.handleErrorResponse(

err.statusCode,

`Error While Searching Databases: `,

err.content

);

throw new AppsEngineException(err as string);

Reason mentioned above.

Nabhag8848 · 2024-04-08T06:49:27Z

src/lib/NotionSDK.ts

+            return this.handleErrorResponse(
+                err.statusCode,
+                `Error While Searching Databases: `,
+                err.content
+            );


Suggested change

return this.handleErrorResponse(

err.statusCode,

`Error While Searching Databases: `,

err.content

);

throw new AppsEngineException(err as string);

Reason mentiond above.

Nabhag8848 · 2024-04-08T06:55:18Z

src/lib/NotionSDK.ts

+
+
+


Remove vertical spaces. 🙏🏻

…in catch errors Signed-off-by: brf153 <153hsb@gmail.com>

brf153 · 2024-04-08T09:41:50Z

@Nabhag8848 made the changes

Nabhag8848

LGTM 🔥 @brf153 Very Thanks, Do you have any feature in mind to built ? / or have some suggestion on improvement around this app feel free to open issue.

brf153 · 2024-04-08T16:33:23Z

LGTM 🔥 @brf153 Very Thanks, Do you have any feature in mind to built ? / or have some suggestion on improvement around this app feel free to open issue.

Sure I will open an issue as soon I get some idea for a feature

brf153 added 7 commits January 21, 2024 21:31

feat:added recursive function for searchPages

2a5141d

Signed-off-by: brf153 <153hsb@gmail.com>

feat:added recursive function for searchPagesandDatabases

781afd9

Signed-off-by: brf153 <153hsb@gmail.com>

feat:added pagination for retrieve database

3b19353

Signed-off-by: brf153 <153hsb@gmail.com>

cleaning code

3041e6c

Signed-off-by: brf153 <153hsb@gmail.com>

fix:fixed the title error

6289a79

Signed-off-by: brf153 <153hsb@gmail.com>

code cleaned

f20b168

Signed-off-by: brf153 <153hsb@gmail.com>

fixing merge conflicts

c14ce21

Signed-off-by: brf153 <153hsb@gmail.com>

Nabhag8848 added this to Pending Review in Notion App - Community Dashboard Feb 4, 2024

Nabhag8848 added the enhancement New feature or request label Feb 29, 2024

Nabhag8848 assigned brf153 Feb 29, 2024

Nabhag8848 self-requested a review March 9, 2024 08:18

Nabhag8848 requested changes Mar 9, 2024

View reviewed changes

Nabhag8848 mentioned this pull request Mar 18, 2024

[Enhance] Improve readability of comment created time #73

Merged

1 task

remove additional recursive functions and implement the functionality…

09f1f4e

… in the original function as recommended Signed-off-by: brf153 <153hsb@gmail.com>

Nabhag8848 requested changes Apr 8, 2024

View reviewed changes

JeffreytheCoder mentioned this pull request Apr 8, 2024

[Bug] Comments don't support pagination #75

Closed

Nabhag8848 moved this from Pending Review to Changes Requested in Notion App - Community Dashboard Apr 8, 2024

proper indentation, remove blank lines and throw AppsEngineException …

7f4e2c8

…in catch errors Signed-off-by: brf153 <153hsb@gmail.com>

brf153 requested a review from Nabhag8848 April 8, 2024 09:41

Nabhag8848 approved these changes Apr 8, 2024

View reviewed changes

Nabhag8848 changed the title ~~[Feat]: Introducing Pagination Support for Notion API in Page and Database Responses~~ [Fix] Introducing Pagination Support for Notion API in Page and Database Responses Apr 8, 2024

Nabhag8848 merged commit f28506d into RocketChat:main Apr 8, 2024
1 check passed

Nabhag8848 moved this from Changes Requested to Done in Notion App - Community Dashboard Apr 10, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Fix] Introducing Pagination Support for Notion API in Page and Database Responses #59

[Fix] Introducing Pagination Support for Notion API in Page and Database Responses #59

brf153 commented Jan 31, 2024

brf153 commented Jan 31, 2024 •

edited

Loading

brf153 commented Jan 31, 2024

Nabhag8848 commented Mar 9, 2024

brf153 commented Mar 9, 2024

Nabhag8848 left a comment •

edited

Loading

Nabhag8848 Mar 9, 2024 •

edited

Loading

Nabhag8848 Mar 9, 2024

Nabhag8848 Mar 9, 2024 •

edited

Loading

Nabhag8848 Mar 9, 2024

Nabhag8848 Mar 9, 2024 •

edited

Loading

Nabhag8848 Mar 9, 2024

brf153 commented Apr 3, 2024

Nabhag8848 commented Apr 4, 2024 •

edited

Loading

brf153 commented Apr 6, 2024

Nabhag8848 commented Apr 6, 2024 •

edited

Loading

Nabhag8848 left a comment

Nabhag8848 Apr 8, 2024

Nabhag8848 Apr 8, 2024 •

edited

Loading

Nabhag8848 Apr 8, 2024

Nabhag8848 Apr 8, 2024

Nabhag8848 Apr 8, 2024

Nabhag8848 Apr 8, 2024

Nabhag8848 Apr 8, 2024

Nabhag8848 Apr 8, 2024

Nabhag8848 Apr 8, 2024

Nabhag8848 Apr 8, 2024

brf153 commented Apr 8, 2024

Nabhag8848 left a comment •

edited

Loading

brf153 commented Apr 8, 2024

[Fix] Introducing Pagination Support for Notion API in Page and Database Responses #59

[Fix] Introducing Pagination Support for Notion API in Page and Database Responses #59

Conversation

brf153 commented Jan 31, 2024

Issue(s)

Acceptance Criteria fulfillment

Proposed changes (including videos or screenshots):

brf153 commented Jan 31, 2024 • edited Loading

brf153 commented Jan 31, 2024

The Changes

Nabhag8848 commented Mar 9, 2024

brf153 commented Mar 9, 2024

Nabhag8848 left a comment • edited Loading

Choose a reason for hiding this comment

Nabhag8848 Mar 9, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nabhag8848 Mar 9, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nabhag8848 Mar 9, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brf153 commented Apr 3, 2024

Nabhag8848 commented Apr 4, 2024 • edited Loading

brf153 commented Apr 6, 2024

Nabhag8848 commented Apr 6, 2024 • edited Loading

Nabhag8848 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nabhag8848 Apr 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brf153 commented Apr 8, 2024

Nabhag8848 left a comment • edited Loading

Choose a reason for hiding this comment

brf153 commented Apr 8, 2024

brf153 commented Jan 31, 2024 •

edited

Loading

Nabhag8848 left a comment •

edited

Loading

Nabhag8848 Mar 9, 2024 •

edited

Loading

Nabhag8848 Mar 9, 2024 •

edited

Loading

Nabhag8848 Mar 9, 2024 •

edited

Loading

Nabhag8848 commented Apr 4, 2024 •

edited

Loading

Nabhag8848 commented Apr 6, 2024 •

edited

Loading

Nabhag8848 Apr 8, 2024 •

edited

Loading

Nabhag8848 left a comment •

edited

Loading