Skip to content

Improve ExecuteSql endpoint wrapper #1813

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

Merged
merged 17 commits into from
Jun 25, 2025
Merged

Improve ExecuteSql endpoint wrapper #1813

merged 17 commits into from
Jun 25, 2025

Conversation

labkey-nicka
Copy link
Contributor

@labkey-nicka labkey-nicka commented Jun 23, 2025

Rationale

Introduce a standalone executeSql() that is akin to selectRows(). We have a number of usages of Query.executeSql() that are not going through our wrappers (namely selectRowsDeprecated()). This streamlines all of the executeSql calls, gets rid of the awkward patterns in selectRowsDeprecated(), and allows for reduction of a lot of boilerplate.

Related Pull Requests

Changes

  • Introduces executeSql() and adds it to our QueryAPIWrapper.
  • Refactors all usages of Query.executeSql() in our apps to use the new executeSql().
  • Refactors all usages of selectRowsDeprecated() that are using the sql parameter to use executeSql().
  • Removes supports for calling Query.executeSql() from selectRowsDeprecated().
  • Add typings to selectRowsDeprecated() (better late then never).
  • Update usages of Query.Command interface

@labkey-nicka labkey-nicka self-assigned this Jun 23, 2025
@labkey-nicka labkey-nicka marked this pull request as ready for review June 23, 2025 16:06
@@ -777,7 +773,7 @@ export function getCrossFolderSelectionResult(

return new Promise((resolve, reject) => {
return Ajax.request({
url: buildURL('experiment', 'getCrossFolderDataSelection.api'),
url: ActionURL.buildURL('experiment', 'getCrossFolderDataSelection.api'),
Copy link
Contributor

Choose a reason for hiding this comment

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

just curious: is there a specific reason for moving these to ActionURL.buildURL? should we be deprecating or removing the buildURL in ui-components?

Copy link
Contributor Author

@labkey-nicka labkey-nicka Jun 24, 2025

Choose a reason for hiding this comment

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

When linking to LKS I recommend using ActionURL.buildURL() as it was purpose-built for that. The other buildURL() still has utility but mainly just for it's support generation of navigation links where we are linking from within the apps to LKS pages (which is not very often) for user navigation purposes. We could rename the latter but I don't think we should deprecate it since there is not 100% functional overlap.

@labkey-nicka labkey-nicka merged commit 617e313 into develop Jun 25, 2025
3 checks passed
@labkey-nicka labkey-nicka deleted the fb_execute_sql branch June 25, 2025 04:58
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