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 Task Instance clear for mapped task summaries #48584

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bbovenzi
Copy link
Contributor

On the Mapped Task Summary details page, add a Clear button to clear all the mapped tasks at once.

This PR also updated the Mark As button too, but there seems to be an API issue that prevents bulk marking state from working so the button is commented out with a TODO


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in airflow-core/newsfragments.

@boring-cyborg boring-cyborg bot added the area:UI Related to UI/UX. For Frontend Developers. label Mar 31, 2025
Copy link
Contributor

@jscheffl jscheffl left a comment

Choose a reason for hiding this comment

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

Looks good in my view! LGTM!

<ActionAccordion affectedTasks={affectedTasks} note={note} setNote={setNote} />
<ActionAccordion
affectedTasks={affectedTasks}
note={taskNote === undefined ? undefined : note}
Copy link
Contributor

Choose a reason for hiding this comment

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

I briefly thought about if it would make sense to keep the notes fied and if all are cleared then the note is applied to all mapped tasks?
But not critical / blocking. Can also be adjusted in future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mark state as and setting a note are the same patch endpoint which doesn't work right now.

Copy link
Member

Choose a reason for hiding this comment

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

If I remember correctly you need to pass a tuple somewhere.

I’ll double check tomorrow. Otherwise I think we should fix the backend

Copy link
Member

Choose a reason for hiding this comment

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

Oh that’s the bulk patch ? Anyway I need to check the code tomorrow

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cool. I'll hold off. It would be great to have this all at once instead of refactoring again later.

@bbovenzi bbovenzi force-pushed the allow-clearing-all-mapped-tis-at-once branch from 374c4fe to 662351c Compare April 1, 2025 20:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:UI Related to UI/UX. For Frontend Developers.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants