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

Add header=colId option for the table-schema API #719 #749

Conversation

fflorent
Copy link
Collaborator

@fflorent fflorent commented Nov 14, 2023

Resolves #719

Introduce header parameter so we can either retrieve the colId or the label properties for the headers.

The documentation is provided here: gristlabs/grist-help#287

@fflorent fflorent force-pushed the issue-719-header-param-table-schema branch from 166bfb6 to 083ef80 Compare November 14, 2023 18:28
@fflorent fflorent force-pushed the issue-719-header-param-table-schema branch from 083ef80 to 870bf3a Compare November 14, 2023 18:29
@fflorent fflorent requested a review from alexmojaki November 16, 2023 09:47
@@ -51,21 +51,21 @@ export async function collectTableSchemaInFrictionlessFormat(
}

const data = await exportTable(activeDoc, tableRef, req);
const tableSchema = columnsToTableSchema(tableId, data, settings.locale);
const tableSchema = columnsToTableSchema(tableId, data, {locale: settings.locale, header: options.header});
Copy link
Contributor

Choose a reason for hiding this comment

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

This is the only place calling columnsToTableSchema, and having it as a separate function which is just one statement doesn't feel like it improves readability. Here's a simplified rewrite of this whole function:

  const {tableId, header} = options;
  if (!activeDoc.docData) {
    throw new Error('No docData in active document');
  }

  // Look up the table to make a CSV from.
  const settings = activeDoc.docData.docSettings();
  const tables = activeDoc.docData.getMetaTable('_grist_Tables');
  const tableRef = tables.findRow('tableId', tableId);

  if (tableRef === 0) {
    throw new ApiError(`Table ${tableId} not found.`, 404);
  }

  const {tableName, columns} = await exportTable(activeDoc, tableRef, req);
  return {
    name: tableId.toLowerCase().replace(/_/g, '-'),
    title: tableName,
    schema: {
      fields: columns.map(col => ({
        name: col[header || "label"],
        ...(col.description ? {description: col.description} : {}),
        ...buildTypeField(col, settings.locale),
      })),
    }
  };
}

Copy link
Collaborator Author

@fflorent fflorent Nov 17, 2023

Choose a reason for hiding this comment

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

Fair point, done :)

Copy link
Contributor

@alexmojaki alexmojaki left a comment

Choose a reason for hiding this comment

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

Thanks!

@alexmojaki alexmojaki merged commit 7bc862f into gristlabs:main Nov 17, 2023
13 checks passed
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.

Add header=colId option to thedownload/table-schema route
2 participants