bbovenzi commented on a change in pull request #15795:
URL: https://github.com/apache/airflow/pull/15795#discussion_r637101562
##########
File path: airflow/www/static/js/connection_form.js
##########
@@ -140,6 +140,55 @@ $(document).ready(function () {
}
}
+ $.fn.serializeObject = function () {
+ const o = {};
+ const a = this.serializeArray();
+ $.each(a, function () {
+ if (this.name === 'csrf_token') {
+ // dont send csrf token
+ } else if (this.name === 'conn_id') {
+ o.connection_id = this.value;
+ } else if (this.value !== '') {
+ o[this.name] = this.value;
+ }
+ });
+ return o;
+ };
+
+ function displayAlert(status, message){
+ const alertClass = status ? 'alert-success' : 'alert-error';
+ let alertBox = $('.container .row .alert');
+ if (alertBox.length) {
+ alertBox.removeClass('alert-success').removeClass('alert-error');
+ alertBox.addClass(alertClass);
+ alertBox.text(message);
+ alertBox.show();
+ } else {
+ alertBox = $('<div class="alert ' + alertClass + '">\n' +
+ '<button type="button" class="close"
data-dismiss="alert">×</button>\n' + message + '</div>');
+
+ $('.container .row').prepend(alertBox).show();
+ }
+ }
+
+ function testConnection() {
+ $.ajax({
+ url: '/api/v1/connections/test',
+ type: 'post',
+ contentType: 'application/json',
+ dataType: 'json',
+ data: JSON.stringify($('form#model_form').serializeObject()),
+ success: function(data) {
+ displayAlert(data.status, data.message);
+ },
+ });
+ }
+
+ $('#test-connection').on('click', (e) => {
+ e.preventDefault();
+ testConnection();
Review comment:
I think we're nesting the testConnection functions too much. `click ->
testConnection() -> .serializeObject()`. Since each function is just used once
lets just have all of it live inside of here for code readability.
##########
File path: airflow/www/static/js/connection_form.js
##########
@@ -140,6 +140,55 @@ $(document).ready(function () {
}
}
+ $.fn.serializeObject = function () {
+ const o = {};
+ const a = this.serializeArray();
+ $.each(a, function () {
+ if (this.name === 'csrf_token') {
+ // dont send csrf token
+ } else if (this.name === 'conn_id') {
+ o.connection_id = this.value;
+ } else if (this.value !== '') {
Review comment:
```suggestion
} else if (this.value !== '' && this.name !== 'csrf_token') {
```
We shouldn't have an if statement that does nothing.
##########
File path: airflow/www/static/js/connection_form.js
##########
@@ -140,6 +140,55 @@ $(document).ready(function () {
}
}
+ $.fn.serializeObject = function () {
Review comment:
We shouldn't add a prototype to jquery. Let's just move all of it to the
`on('click'` handler.
##########
File path: airflow/www/static/js/connection_form.js
##########
@@ -140,6 +140,55 @@ $(document).ready(function () {
}
}
+ $.fn.serializeObject = function () {
+ const o = {};
+ const a = this.serializeArray();
Review comment:
We need better variable names here. Single letters are only for common
items event and id.
##########
File path: airflow/www/static/js/connection_form.js
##########
@@ -140,6 +140,55 @@ $(document).ready(function () {
}
}
+ $.fn.serializeObject = function () {
+ const o = {};
+ const a = this.serializeArray();
+ $.each(a, function () {
+ if (this.name === 'csrf_token') {
+ // dont send csrf token
+ } else if (this.name === 'conn_id') {
+ o.connection_id = this.value;
+ } else if (this.value !== '') {
+ o[this.name] = this.value;
+ }
+ });
+ return o;
+ };
+
+ function displayAlert(status, message){
+ const alertClass = status ? 'alert-success' : 'alert-error';
+ let alertBox = $('.container .row .alert');
+ if (alertBox.length) {
+ alertBox.removeClass('alert-success').removeClass('alert-error');
+ alertBox.addClass(alertClass);
+ alertBox.text(message);
+ alertBox.show();
+ } else {
+ alertBox = $('<div class="alert ' + alertClass + '">\n' +
+ '<button type="button" class="close"
data-dismiss="alert">×</button>\n' + message + '</div>');
+
+ $('.container .row').prepend(alertBox).show();
+ }
+ }
+
+ function testConnection() {
+ $.ajax({
+ url: '/api/v1/connections/test',
+ type: 'post',
+ contentType: 'application/json',
+ dataType: 'json',
+ data: JSON.stringify($('form#model_form').serializeObject()),
Review comment:
We should process the data all at once and not inline of the ajax call.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
