Skip to content

SCAL-257337: Added sortOptions to getUnderlyingDataForPoint - #678

Open
tushardeepakts wants to merge 5 commits into
mainfrom
SCAL-257337_answer_sort
Open

tushardeepakts wants to merge 5 commits into
mainfrom
SCAL-257337_answer_sort

Conversation

@tushardeepakts

@tushardeepakts tushardeepakts commented Sep 18, 2026

Copy link
Copy Markdown

Summary

Adds an optional sortOptions parameter so customers can sort the raw underlying data behind a clicked chart point.

Changes

  • Added a new public SortOptions type ({ columnName; ascending }), exported from index.ts.
  • Added an optional sortOptions parameter to getUnderlyingDataForPoint(cols, points?, sortOptions?).
    • Applies sorting to the new session before returning it.
  • Added a new updateSort mutation.
  • Added a private applySort() helper.
    • Maps column names to GUIDs using the parent source detail.
    • Avoids an extra round-trip.

Video:

Screen.Recording.2026-09-21.at.2.53.20.PM.mov

@tushardeepakts
tushardeepakts requested a review from a team as a code owner September 18, 2026 08:03
@tushardeepakts tushardeepakts self-assigned this Sep 18, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces sorting capabilities to the getUnderlyingDataForPoint method in AnswerService by adding a new SortOptions interface, an updateSort GraphQL mutation, and the applySort helper method. Corresponding unit tests have also been added to verify the sorting behavior. The review feedback suggests adding defensive checks within applySort to handle missing or invalid inputs (such as undefined source columns or unrecognized column names) to prevent runtime TypeErrors and improve robustness.

Comment thread src/utils/graphql/answerService/answerService.ts
@pkg-pr-new

pkg-pr-new Bot commented Sep 18, 2026

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/@thoughtspot/visual-embed-sdk@678

commit: 8410290

@tushardeepakts tushardeepakts changed the title SCAL-257337: Add sort to getUnderlyingDataForPoint SCAL-257337: Added sortOptions to getUnderlyingDataForPoint Sep 18, 2026
/**
* Describes how to sort a column when fetching data. Pass an array of these to
* sort by multiple columns, in priority order.
* @version SDK: 1.52.0 | ThoughtSpot Cloud: 26.9.0.cl

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

fix the version

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The SDK version I have taken from package.json, and TS cloud version I have calculated from the formula mentioned in Claude.md file :

For SDK 1.N.x the Cloud version is 26.(N-43).0.cl

private async applySort(sortOptions: SortOptions[], sourceDetail: any) {
const sortDetails = sortOptions.map((sort) => ({
columnId: getGuidsFromColumnNames(sourceDetail, [sort.columnName]).values().next().value,
sortType: sort.ascending ? 'ASCENDING' : 'DESCENDING',

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

make a enum for this

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

These are internal wire-contract values used once in a ternary. An enum here would be unnecessary. The public API only exposes ascending: boolean; ASCENDING/DESCENDING are just internal GraphQL values.

This branch has not been deployed

No deployments
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