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

Connector: implement retrieve content nodes for Gdrive #4125

Merged
merged 2 commits into from
Mar 4, 2024

Conversation

PopDaph
Copy link
Contributor

@PopDaph PopDaph commented Mar 4, 2024

Description

We're implementing a "retrieve content nodes" for each connector.
It is meant to replace "retrieve resource titles".
More context here: #4044

This is the PR for Gdrive.

Risk

Low as not used yet.

Deploy Plan

Nothing special.

@PopDaph PopDaph requested a review from lasryaric March 4, 2024 17:26
@PopDaph PopDaph force-pushed the retrieve-content-node-gdrive branch from 477a262 to 252e344 Compare March 4, 2024 17:57
Copy link
Contributor

@lasryaric lasryaric left a comment

Choose a reason for hiding this comment

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

Left one comment but if you have a good reason for sourceURL to be empty, you can merge.

title: f.name || "",
dustDocumentId: getGoogleDriveEntityDocumentId(f),
lastUpdatedAt: f.lastUpsertedTs?.getTime() || null,
sourceUrl: null,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is source URL empty?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because it's empty when we retrieve the read permissions and out of consistency I kept the exact same behavior!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually I think it's better to have it and the consistency should be with all connector on this screen instead of per connector. I added something, looks like it's working but let me know if you think it's better to fetch the API to retrieve the url from the API

@PopDaph PopDaph merged commit 74ae1a0 into main Mar 4, 2024
5 checks passed
@PopDaph PopDaph deleted the retrieve-content-node-gdrive branch March 4, 2024 19:07
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