Implement multisort in DAGs table #47849

mariana-marcal-santana · 2025-03-17T09:21:18Z

Implement multisort for DAGs when displayed in a table; The URL displays extra sorting attributes when Shift+click is used on the column to sort by.

This implements the UI portion of #46383 and is dependent on the backend part implemented in #47440.

cc @pierrejeambrun @bbovenzi

boring-cyborg · 2025-03-17T09:21:22Z

Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contributors' Guide (https://github.com/apache/airflow/blob/main/contributing-docs/README.rst)
Here are some useful points:

Pay attention to the quality of your code (ruff, mypy and type annotations). Our pre-commits will help you with that.
In case of a new feature add useful documentation (in docstrings or in docs/ directory). Adding a new operator? Check this short guide Consider adding an example DAG that shows how users should use it.
Consider using Breeze environment for testing locally, it's a heavy docker but it ships with a working Airflow and a lot of integrations.
Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
Please follow ASF Code of Conduct for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
Be sure to read the Airflow Coding style.
Always keep your Pull Requests rebased, otherwise your build might fail due to changes not related to your commits.
Apache Airflow is a community-driven project and together we are making it better 🚀.
In case of doubts contact the developers at:
Mailing List: dev@airflow.apache.org
Slack: https://s.apache.org/airflow-slack

pierrejeambrun

Thanks for your contribution.

Were you able to test this with a working backend ? (What I mean by that is you should be able to test that when you try sorting on multiple criteria the table should re-order itself appropriately.)

For this I believe you would need to rebase your branch on top of #47440

mariana-marcal-santana · 2025-03-19T10:29:24Z

I tested this with #47440's backend and the part related to the frontend (the setting of the URL) worked correctly as shown in the picture in #46383 (comment). The actual sorting wasn't perfect in some cases as I also mentioned in the comment above which is why I asked if the backend had been tested for examples like the one I shared.

pierrejeambrun

You need to modify components so that the request that is send to the backend passes the appropriateorder_by list of query parameters.

For now the url is correctly set and updated via the table component, but the requests send to the backend do not specify the multisort, this is why you do not have the expected response.

I think the backend implementation is functional and you should observe the appropriate behavior based on #47440

pierrejeambrun · 2025-03-19T11:03:12Z

An exemple here, multiple sort are specified, but the backend request only specify the first one dag_id

mariana-marcal-santana · 2025-03-19T23:06:53Z

Thanks for the insights! I'll check on that and update on my progress

mariana-marcal-santana · 2025-03-21T21:47:19Z

I've corrected the code to also pass the sort parameters to the backend URL. When tested with the backend everything seemed to work as expected, including the previously mentioned example.

cc @pierrejeambrun @bbovenzi

Thank you!

pierrejeambrun

Thanks @mariana-marcal-santana,

Looking good, will do a final testing and review once the backend is merged.

A few things need to be adjusted.

pierrejeambrun · 2025-03-31T08:45:01Z

airflow/ui/src/pages/DagsList/DagsList.tsx

@@ -166,8 +166,7 @@ export const DagsList = () => {
    searchParams.get(NAME_PATTERN_PARAM) ?? undefined,
  );

-  const [sort] = sorting;
-  const orderBy = sort ? `${sort.desc ? "-" : ""}${sort.id}` : "-last_run_start_date";
+  const orderBy = new URLSearchParams(globalThis.location.search).getAll("sort");


You are missing the default sorting value.

By default if sort wasn't defined it would default to -last_run_start_date, that's lost with the new implementation

Also there are plenty of other places where we do sorting like that and that need to be updated to work with a list:
AssetList, Variables, Connections etc.... (you can search for const [sort] = sorting in the code base)

Thanks for the feedback, I'll start working on this asap!

bbovenzi

One quick comment and could you rebase when you have a chance?

After rebasing I'll do some more manual usability testing

bbovenzi · 2025-04-24T16:41:50Z

airflow/ui/src/pages/Asset/Asset.tsx

+  const orderBy = new URLSearchParams(globalThis.location.search).getAll("sort").length === 0 ?
+    ["-timestamp"] : new URLSearchParams(globalThis.location.search).getAll("sort");


Suggested change

const orderBy = new URLSearchParams(globalThis.location.search).getAll("sort").length === 0 ?

["-timestamp"] : new URLSearchParams(globalThis.location.search).getAll("sort");

const sortParam = new URLSearchParams(globalThis.location.search).getAll("sort")

const orderBy = sortParam.length === 0 ?

["-timestamp"] : sortParam;

I won't comment on every single occurrence but let's try to keep our code DRY.

Since this is so common, maybe we can even pull it into its own util function or hook?

Hello @bbovenzi @pierrejeambrun !

I've created the function and used it in all places mentioned in #47849 (comment).

I've also rebased the repo (yesterday) as requested.

I would appreciate any feedback on the implementation! Thanks!

Frontend: Implement multisort for DAGs when displayed in a table; The URL displays extra sorting attributes when Shift+click is used on the column to sort by. This implements the UI portion of apache#46383

Frontend: Implement multisort for DAGs when displayed in a table; The browser and backend URLs display extra sorting attributes when Shift+click is used on the column to sort by. This implements the UI portion of apache#46383

Frontend: Implement multisort for DAGs when displayed in a table; Add default to the sorting parameters; Add support for multiple sort parameters in more lists This implements the UI portion of apache#46383

Frontend: Implement multisort for DAGs when displayed in a table; Add function to get default sorting parameters This implements the UI portion of apache#46383

Frontend: Implement multisort for DAGs when displayed in a table; Add support for multiple sort parameters in new lists This implements the UI portion of apache#46383

bbovenzi

Looking good.

Do you mind running pnpm lint && pnpm format inside your local airflow-core/src/airflow/ui directory?

bbovenzi · 2025-04-28T15:03:36Z

airflow-core/src/airflow/ui/src/utils/query.ts

+export const getOrderBy = (pre_defined?: string): string[] => {
+  const sortParam = new URLSearchParams(globalThis.location.search).getAll("sort");
+  return (sortParam.length === 0 && pre_defined != undefined) ? [pre_defined] : sortParam;
+};
+


Suggested change

export const getOrderBy = (pre_defined?: string): string[] => {

const sortParam = new URLSearchParams(globalThis.location.search).getAll("sort");

return (sortParam.length === 0 && pre_defined != undefined) ? [pre_defined] : sortParam;

};

export const getOrderBy = (defaultOrder?: string): string[] => {

const sortParam = new URLSearchParams(globalThis.location.search).getAll("sort");

return (sortParam.length === 0 && defaultOrder !== undefined) ? [defaultOrder] : sortParam;

};

We generally don't want to use snake_case in our typescript code. The only ones we do have are generated from the python API.

Hello @bbovenzi !

In my last commit I fixed the casing, ran the afore mentioned commands and fixed the things marked in the lint command (that were from my commits).

Anything else I should change?

Also, I noticed that the code failed these tests: https://github.com/apache/airflow/actions/runs/14682302749/job/41221555783. How should I fix it?
I noticed that it fails where I changed orderBy?: string to orderBy?: string[]. From what I saw, I think it's because it clashes with the definition on the file airflow-core/src/airflow/api_fastapi/core_api/openapi/v1-rest-api-generated.yaml (that I changed but I believe it got overwritten).

Could you confirm this please?

Thank you!

pnpm format should help solve most of those static checks

We may also need to make sure we generate the API spec docs before we run pnpm format

Hello @bbovenzi !

I ran pnpm format but the static checks still failed...

When you say API spec docs do you mean this file: airflow-core/src/airflow/api_fastapi/core_api/openapi/v1-rest-api-generated.yaml? And if so how can I generate it? I changed it locally, adapted the code that depended on it and everything seems to be working on my machine. What do I need to do to pass the static checks?

Thank you!

Oh I think this is an order of operations with pre-commit

pre-commit run generate-openapi-spec and then pre-commit run ts-compile-format-lint-ui

Hello @bbovenzi !

I had previously ran these operations and noticed that the first one changes the v1-rest-api-generated.yaml file like so:

This is what is causing the errors in the CI because the Order by argument now has to be an array and it keeps changing to string (which doesn't go along with the changes in the code for the multisort).
Given this, what I meant was if there was a way to define the file and the pre-commit to allow the Order By to be an array? From what I know, it seems like this could fix the CI errors.

Thank you!

Implement multisort for DAGs when displayed in a table; Fix casing and formatting This implements the UI portion of apache#46383

pierrejeambrun · 2025-05-15T14:42:52Z

airflow-core/src/airflow/api_fastapi/core_api/openapi/v1-rest-api-generated.yaml

+          type: array
+          items:
+            type: string


You can't modify this manually, the file is auto generated based on the backend, and currently the backend specify a string not an array. You'll need to wait for the backend PR to be done and rebase on top of that, this way the order_by will have the appropriate array type.

Unfortunately the backend PR is not progressing. I trying pinging the contributor but we might want to take it at some point.

Hello @pierrejeambrun @bbovenzi !

Thanks for clarifying! Given this, should I fix the editions in this file now or only after the backend PR is merged to have a better sense of the necessary changes?

Once the backend PR is merged, rebasing on latest main will automatically give you that change. (The updated openapi spec).

We can't merge this PR until the backend is done, until then I think we can keep things like that (manually edited spec), this way from a frontend point of view the backend interface is correct and you can work without disrupting your workflow (eslint errors etc), but the CI static check won't be able to succeed. Once the backend is done we remove the manual change to the openapi spec and rebase on main to get the updated spec.

mariana-marcal-santana requested review from bbovenzi, pierrejeambrun, ryanahamilton and jscheffl as code owners March 17, 2025 09:21

boring-cyborg bot added the area:UI Related to UI/UX. For Frontend Developers. label Mar 17, 2025

pierrejeambrun reviewed Mar 18, 2025

View reviewed changes

pierrejeambrun reviewed Mar 19, 2025

View reviewed changes

mariana-marcal-santana force-pushed the bugfix branch from bbc5fe5 to d19d25a Compare March 21, 2025 21:43

pierrejeambrun reviewed Mar 31, 2025

View reviewed changes

bbovenzi added this to the Airflow 3.1.0 milestone Apr 24, 2025

bbovenzi reviewed Apr 24, 2025

View reviewed changes

mariana-marcal-santana added 4 commits April 25, 2025 17:25

Implement multisort in DAGs table

1b4fb62

Frontend: Implement multisort for DAGs when displayed in a table; The URL displays extra sorting attributes when Shift+click is used on the column to sort by. This implements the UI portion of apache#46383

Implement multisort in DAGs table

3c1c6aa

Frontend: Implement multisort for DAGs when displayed in a table; The browser and backend URLs display extra sorting attributes when Shift+click is used on the column to sort by. This implements the UI portion of apache#46383

Implement multisort in DAGs table

58ba197

Frontend: Implement multisort for DAGs when displayed in a table; Add default to the sorting parameters; Add support for multiple sort parameters in more lists This implements the UI portion of apache#46383

Implement multisort in DAGs table

6827b2e

Frontend: Implement multisort for DAGs when displayed in a table; Add function to get default sorting parameters This implements the UI portion of apache#46383

mariana-marcal-santana force-pushed the bugfix branch from 2bd2e69 to 6827b2e Compare April 25, 2025 17:23

mariana-marcal-santana requested review from shubhamraj-git, ephraimbuddy, rawwar, jason810496 and bugraoz93 as code owners April 25, 2025 17:23

Implement multisort in DAGs table

2e96649

Frontend: Implement multisort for DAGs when displayed in a table; Add support for multiple sort parameters in new lists This implements the UI portion of apache#46383

bbovenzi reviewed Apr 28, 2025

View reviewed changes

Implement multisort in DAGs table Frontend:

7b9160b

Implement multisort for DAGs when displayed in a table; Fix casing and formatting This implements the UI portion of apache#46383

pierrejeambrun reviewed May 15, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement multisort in DAGs table #47849

Implement multisort in DAGs table #47849

mariana-marcal-santana commented Mar 17, 2025

boring-cyborg bot commented Mar 17, 2025

pierrejeambrun left a comment •

edited

Loading

mariana-marcal-santana commented Mar 19, 2025

pierrejeambrun left a comment •

edited

Loading

pierrejeambrun commented Mar 19, 2025

mariana-marcal-santana commented Mar 19, 2025

mariana-marcal-santana commented Mar 21, 2025

pierrejeambrun left a comment

pierrejeambrun Mar 31, 2025 •

edited

Loading

pierrejeambrun Mar 31, 2025

mariana-marcal-santana Apr 1, 2025

bbovenzi left a comment

bbovenzi Apr 24, 2025

bbovenzi Apr 24, 2025

mariana-marcal-santana Apr 26, 2025

bbovenzi left a comment

bbovenzi Apr 28, 2025

mariana-marcal-santana Apr 29, 2025

bbovenzi Apr 30, 2025

bbovenzi Apr 30, 2025

mariana-marcal-santana May 3, 2025

bbovenzi May 6, 2025

mariana-marcal-santana May 12, 2025

pierrejeambrun May 15, 2025

mariana-marcal-santana May 19, 2025

pierrejeambrun May 20, 2025

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier! Saves Data!

		const orderBy = new URLSearchParams(globalThis.location.search).getAll("sort").length === 0 ?
		["-timestamp"] : new URLSearchParams(globalThis.location.search).getAll("sort");

Implement multisort in DAGs table #47849

Are you sure you want to change the base?

Implement multisort in DAGs table #47849

Conversation

mariana-marcal-santana commented Mar 17, 2025

boring-cyborg bot commented Mar 17, 2025

pierrejeambrun left a comment • edited Loading

Choose a reason for hiding this comment

mariana-marcal-santana commented Mar 19, 2025

pierrejeambrun left a comment • edited Loading

Choose a reason for hiding this comment

pierrejeambrun commented Mar 19, 2025

mariana-marcal-santana commented Mar 19, 2025

mariana-marcal-santana commented Mar 21, 2025

pierrejeambrun left a comment

Choose a reason for hiding this comment

pierrejeambrun Mar 31, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bbovenzi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bbovenzi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pierrejeambrun left a comment •

edited

Loading

pierrejeambrun left a comment •

edited

Loading

pierrejeambrun Mar 31, 2025 •

edited

Loading