Skip to content

[v1 API - tables] Use default title for tables #11526

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Mar 21, 2025
Merged

[v1 API - tables] Use default title for tables #11526

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions front/lib/api/content_nodes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ export const FOLDERS_SELECTION_PREVENTED_MIME_TYPES = [
MIME_TYPES.NOTION.SYNCING_FOLDER,
] as readonly string[];

export const UNTITLED_TITLE = "Untitled Document";

export function getContentNodeInternalIdFromTableId(
dataSourceView: DataSourceViewResource | DataSourceViewType,
tableId: string
Expand Down Expand Up @@ -93,9 +95,7 @@ export function getContentNodeFromCoreNode(
parentInternalId: coreNode.parent_id ?? null,
// TODO(2025-01-27 aubin): remove this once the corresponding titles are backfilled.
title:
coreNode.title === "Untitled document"
? coreNode.node_id
: coreNode.title,
coreNode.title === UNTITLED_TITLE ? coreNode.node_id : coreNode.title,
sourceUrl: coreNode.source_url ?? null,
permission: "read",
lastUpdatedAt: coreNode.timestamp,
Expand Down
18 changes: 8 additions & 10 deletions front/lib/api/data_sources.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import type { Transaction } from "sequelize";

import { getConversationWithoutContent } from "@app/lib/api/assistant/conversation/without_content";
import { default as apiConfig, default as config } from "@app/lib/api/config";
import { UNTITLED_TITLE } from "@app/lib/api/content_nodes";
import { sendGitHubDeletionEmail } from "@app/lib/api/email";
import { upsertTableFromCsv } from "@app/lib/api/tables";
import { getMembers } from "@app/lib/api/workspace";
Expand Down Expand Up @@ -635,18 +636,13 @@ export async function upsertTable({

const titleEmpty = params.title.trim().length === 0;
// Enforce a max size on the title: since these will be synced in ES we don't support arbitrarily large titles.
if (
params.title &&
(params.title.length > MAX_NODE_TITLE_LENGTH || titleEmpty)
) {
if (params.title && params.title.length > MAX_NODE_TITLE_LENGTH) {
return new Err({
name: "dust_error",
code: titleEmpty ? "title_is_empty" : "title_too_long",
code: "title_too_long",
message:
"Invalid title:" +
(titleEmpty
? "title cannot be empty"
: `title too long (max ${MAX_NODE_TITLE_LENGTH} characters).`),
`title too long (max ${MAX_NODE_TITLE_LENGTH} characters).`,
});
}

Expand Down Expand Up @@ -687,6 +683,8 @@ export async function upsertTable({
standardizedSourceUrl = standardized;
}

const title = titleEmpty ? UNTITLED_TITLE : params.title;
Copy link
Contributor

Choose a reason for hiding this comment

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

here it would be "Untitled Table" no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought about it and IMO better to have one unified "UNTITLED_TITLE"

Copy link
Contributor

Choose a reason for hiding this comment

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

we could still have UNTITLED_DOCUMENT_TITLE, UNTITLED_TABLE_TITLE and UNTITLED_FOLDER_TITLE defined at the same place? but ofc this is only a fallback since each connector should provide better names

Copy link
Contributor

Choose a reason for hiding this comment

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

so totally up to you, i'm fine with anything 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks :) I think it's better to have only one, despite the downside of it being a bit unaccurate. An option would be to rename "[untitled]", will do that if we ever hear about this again


if (async) {
if (fileId) {
const file = await FileResource.fetchById(auth, fileId);
Expand Down Expand Up @@ -746,7 +744,7 @@ export async function upsertTable({
csv: null,
fileId: fileId ?? null,
truncate,
title: params.title,
title,
mimeType: params.mimeType,
sourceUrl: standardizedSourceUrl,
},
Expand Down Expand Up @@ -788,7 +786,7 @@ export async function upsertTable({
tableParents,
fileId: fileId ?? null,
truncate,
title: params.title,
title,
mimeType: params.mimeType,
sourceUrl: standardizedSourceUrl,
});
Expand Down
3 changes: 2 additions & 1 deletion ...pages/api/v1/w/[wId]/spaces/[spaceId]/data_sources/[dsId]/documents/[documentId]/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { fromError } from "zod-validation-error";

import { withPublicAPIAuthentication } from "@app/lib/api/auth_wrappers";
import apiConfig from "@app/lib/api/config";
import { UNTITLED_TITLE } from "@app/lib/api/content_nodes";
import type { Authenticator } from "@app/lib/auth";
import { MAX_NODE_TITLE_LENGTH } from "@app/lib/content_nodes";
import { runDocumentUpsertHooks } from "@app/lib/document_upsert_hooks/hooks";
Expand Down Expand Up @@ -569,7 +570,7 @@ async function handler(
?.trim();

// Use titleInTags if no title is provided.
const title = r.data.title?.trim() || titleInTags || "Untitled Document";
const title = r.data.title?.trim() || titleInTags || UNTITLED_TITLE;

if (!titleInTags) {
tags.push(`title:${title}`);
Expand Down
Loading