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

Make AirflowDateTimePickerWidget a required field #18602

Merged
merged 1 commit into from
Sep 30, 2021

Conversation

ephraimbuddy
Copy link
Contributor

@ephraimbuddy ephraimbuddy commented Sep 29, 2021

The UI breaks when a search field sends a null datetime. This fixes it.

Closes #15976

@boring-cyborg boring-cyborg bot added the area:webserver Webserver related Issues label Sep 29, 2021
@ephraimbuddy
Copy link
Contributor Author

Thanks to @bbovenzi we now have a default value in the UI for date fields

Copy link
Member

@ashb ashb left a comment

Choose a reason for hiding this comment

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

What is the path to trigger this?

Your fix looks suspect to me, and instead of using "now" as the value I think we want to just return the query un-altered in that case.

An even better fix might be to change the field validation so that null is not allowed/is never passed to the filter!

@ephraimbuddy
Copy link
Contributor Author

What is the path to trigger this?

Your fix looks suspect to me, and instead of using "now" as the value I think we want to just return the query un-altered in that case.

An even better fix might be to change the field validation so that null is not allowed/is never passed to the filter!

The trigger is to go to browse >> task instances and select start_date, end_date or queued_dttm on the search, then click the search. The date should be null before you search

@ashb
Copy link
Member

ashb commented Sep 29, 2021

What is the path to trigger this?
Your fix looks suspect to me, and instead of using "now" as the value I think we want to just return the query un-altered in that case.
An even better fix might be to change the field validation so that null is not allowed/is never passed to the filter!

The trigger is to go to browse >> task instances and select start_date, end_date or queued_dttm on the search, then click the search. The date should be null before you search

Yeah, that is improper validation then -- we should fix that instead

@ephraimbuddy
Copy link
Contributor Author

Disabling the input when the values are null disables the DateTime picker

Copy link
Contributor

@bbovenzi bbovenzi left a comment

Choose a reason for hiding this comment

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

It would be preferable to just disable the submit button on the search form but it is built by FAB. This is a decent solution that requires less changes I believe.

@github-actions
Copy link

The PR is likely OK to be merged with just subset of tests for default Python and Database versions without running the full matrix of tests, because it does not modify the core of Airflow. If the committers decide that the full tests matrix is needed, they will add the label 'full tests needed'. Then you should rebase to the latest main or amend the last commit of the PR, and push it with --force-with-lease.

@github-actions github-actions bot added the okay to merge It's ok to merge this PR as it does not require more tests label Sep 29, 2021
@bbovenzi
Copy link
Contributor

Related issue: #15976

The UI breaks when a search field sends a null datetime. This fixes it
@ephraimbuddy ephraimbuddy changed the title Default UtcAwareFilterMixin datetime to utcnow on valueError Make AirflowDateTimePickerWidget a required field Sep 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:webserver Webserver related Issues okay to merge It's ok to merge this PR as it does not require more tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error when querying on the Browse view with empty date picker
3 participants