Field Deletion Warning when editing Connections #41144
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
boring-cyborg
bot
added
area:UI
RNHTTR
reviewed
Jul 31, 2024
|
@lh5844 can you also add a unit test please? |
Because I've kept the general warning and that change is made in the connection_form.js, there don't seem to be any unit tests for any javascript files. So, I'll leave it at that. Correct if I'm wrong though |
lh5844
requested review from
ryanahamilton,
ashb,
bbovenzi and
pierrejeambrun
as code owners
bbovenzi
reviewed
Aug 8, 2024
…irflow into edit_connection_message
bbovenzi
reviewed
Aug 12, 2024
|
One last comment. Otherwise lgtm! |
Co-authored-by: Brent Bovenzi <brent.bovenzi@gmail.com>
RNHTTR
approved these changes
Aug 13, 2024
|
Thank you for your suggestion! It definitely made it more concise. With that, it should be ready to be merged! |
bbovenzi
approved these changes
Aug 14, 2024
bbovenzi
added a commit
to astronomer/airflow
that referenced
this pull request
Aug 14, 2024
* able to change the 'Changed Row' display message after edit * added message in connection form to warn of empty fields * attempt to warn the specific fields cannot be empty * revert change because need to check fields before save is clicked * issues warning for specific fields that can't be deleted after save * removed the individual warnings * changed status to concise string * added more concise suggestion Co-authored-by: Brent Bovenzi <brent.bovenzi@gmail.com> --------- Co-authored-by: Brent Bovenzi <brent.bovenzi@gmail.com>
uranusjr
pushed a commit
to astronomer/airflow
that referenced
this pull request
Aug 15, 2024
* able to change the 'Changed Row' display message after edit * added message in connection form to warn of empty fields * attempt to warn the specific fields cannot be empty * revert change because need to check fields before save is clicked * issues warning for specific fields that can't be deleted after save * removed the individual warnings * changed status to concise string * added more concise suggestion Co-authored-by: Brent Bovenzi <brent.bovenzi@gmail.com> --------- Co-authored-by: Brent Bovenzi <brent.bovenzi@gmail.com>
bbovenzi
added a commit
to astronomer/airflow
that referenced
this pull request
Aug 15, 2024
* able to change the 'Changed Row' display message after edit * added message in connection form to warn of empty fields * attempt to warn the specific fields cannot be empty * revert change because need to check fields before save is clicked * issues warning for specific fields that can't be deleted after save * removed the individual warnings * changed status to concise string * added more concise suggestion Co-authored-by: Brent Bovenzi <brent.bovenzi@gmail.com> --------- Co-authored-by: Brent Bovenzi <brent.bovenzi@gmail.com>
Artuz37
pushed a commit
to Artuz37/airflow
that referenced
this pull request
Aug 19, 2024
* able to change the 'Changed Row' display message after edit * added message in connection form to warn of empty fields * attempt to warn the specific fields cannot be empty * revert change because need to check fields before save is clicked * issues warning for specific fields that can't be deleted after save * removed the individual warnings * changed status to concise string * added more concise suggestion Co-authored-by: Brent Bovenzi <brent.bovenzi@gmail.com> --------- Co-authored-by: Brent Bovenzi <brent.bovenzi@gmail.com>
bbovenzi
added a commit
that referenced
this pull request
Aug 19, 2024
* able to change the 'Changed Row' display message after edit * added message in connection form to warn of empty fields * attempt to warn the specific fields cannot be empty * revert change because need to check fields before save is clicked * issues warning for specific fields that can't be deleted after save * removed the individual warnings * changed status to concise string * added more concise suggestion --------- Co-authored-by: Lucy Hu <90779522+lh5844@users.noreply.github.com>
molcay
pushed a commit
to VladaZakharova/airflow
that referenced
this pull request
Aug 19, 2024
* able to change the 'Changed Row' display message after edit * added message in connection form to warn of empty fields * attempt to warn the specific fields cannot be empty * revert change because need to check fields before save is clicked * issues warning for specific fields that can't be deleted after save * removed the individual warnings * changed status to concise string * added more concise suggestion Co-authored-by: Brent Bovenzi <brent.bovenzi@gmail.com> --------- Co-authored-by: Brent Bovenzi <brent.bovenzi@gmail.com>
romsharon98
pushed a commit
to romsharon98/airflow
that referenced
this pull request
Aug 20, 2024
* able to change the 'Changed Row' display message after edit * added message in connection form to warn of empty fields * attempt to warn the specific fields cannot be empty * revert change because need to check fields before save is clicked * issues warning for specific fields that can't be deleted after save * removed the individual warnings * changed status to concise string * added more concise suggestion Co-authored-by: Brent Bovenzi <brent.bovenzi@gmail.com> --------- Co-authored-by: Brent Bovenzi <brent.bovenzi@gmail.com>
utkarsharma2
added
type:improvement
utkarsharma2
pushed a commit
that referenced
this pull request
Sep 2, 2024
* able to change the 'Changed Row' display message after edit * added message in connection form to warn of empty fields * attempt to warn the specific fields cannot be empty * revert change because need to check fields before save is clicked * issues warning for specific fields that can't be deleted after save * removed the individual warnings * changed status to concise string * added more concise suggestion --------- Co-authored-by: Lucy Hu <90779522+lh5844@users.noreply.github.com>
23 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The issue mentions that fields backed by the BS3TextFieldWidget cannot be deleted once populated. To delete those fields, the user must delete the connection and create a new one. The "Changed Row" message displayed after a row is edited is misleading.
Therefore, as a temporary fix, I have created a warning that always shows when the user is editing a connection form as shown below.
What I tried to do was get the specific fields that were populated and issue a warning with those field names when the user tries to delete them. However, I'm having trouble figuring out how to do that check when the user clicks the 'Save' button. So, I have a warning of the currently populated fields that can be modified but not deleted after the user clicks 'Save' shown below.
related: #40105
^ 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.rstor{issue_number}.significant.rst, in newsfragments.