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

DATAUP-737: Second set of deepscan.io fixes #2889

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 15 additions & 21 deletions kbase-extension/static/kbase/js/kbwidget.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ return KBWidget(
version : "1.0.0", //future proofing, but completely unused

_accessors : [ //optional. A list of values to automatically create setter/getters for.
'foo' //you'll now be able to store something at $widget.foo('newValue') and access it via var foo = $widget.foo();
Copy link
Collaborator Author

@ialarmedalien ialarmedalien Apr 14, 2022

Choose a reason for hiding this comment

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

Generally I have tried to add comments whenever there's a change that isn't var -> let/const, removing an unused argument, renaming a var that is used in the upper scope, or something else that's similarly trivial.

'foo' //you'll now be able to store something at $widget.foo('newValue') and access it via const foo = $widget.foo();
{name : 'bar', setter : 'setBar', getter : 'getBar'} // the setter/getter can also be overridden If these functions are not
// defined in your object, you'll get default implementations. Add new methods
// in the object body for specific behavior. See the caveat about bindings and notifications
Expand Down Expand Up @@ -59,14 +59,14 @@ return KBWidget(

Instantiate as follows:

var $fancy = $('some-jquery-selector').myFancyNewWidget(
const $fancy = $('some-jquery-selector').myFancyNewWidget(
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Did a big search for var and replaced them all with let, and then ran eslint --fix, which caught some of the lets that could be changed to consts. Fixed the rest by hand.

{
bar : 7
}
);

var foo = $fancy.foo();
var $element = $fancy.$elem; //is the same as $('some-jquery-selector');
const foo = $fancy.foo();
const $element = $fancy.$elem; //is the same as $('some-jquery-selector');

You get a lot of methods availabe by default:

Expand Down Expand Up @@ -198,7 +198,7 @@ define(['jquery', 'handlebars'], ($, Handlebars) => {
};

const makeBindingBlurCallback = function (elem, $target, attribute, transformers, accessors) {
return $.proxy(function (e, vals) {
return $.proxy(function (e) {
if (e.type === 'keypress' && e.which !== 13) {
return;
}
Expand Down Expand Up @@ -410,15 +410,15 @@ define(['jquery', 'handlebars'], ($, Handlebars) => {
return $(tag);
};

const KBWidget = function (def) {
var KBWidget = function (def) {
def = def || {};

var Widget = function ($elem) {
const Widget = function ($elem) {
let self = this;

//XXX THIS IS FOR BACKWARDS COMPATIBILITY WITH JQUERY PLUGIN SYNTAX __ONLY__
if (!(this instanceof Widget)) {
var args = $elem;
const args = $elem;
self = new Widget(this, args);
$elem = this;
if (window.console) {
Expand All @@ -443,7 +443,7 @@ define(['jquery', 'handlebars'], ($, Handlebars) => {
$elem.get(0).kb_obj = self;
}

var args = Array.prototype.slice.call(arguments, 1);
const args = Array.prototype.slice.call(arguments, 1);

$elem[def.name] = function (method) {
if (window.console) {
Expand Down Expand Up @@ -471,7 +471,6 @@ define(['jquery', 'handlebars'], ($, Handlebars) => {
Widget.prototype.__attributes = {};

if (defCopy._accessors !== undefined) {
//for (var accessor in defCopy._accessors) {
$.each(
defCopy._accessors,
$.proxy((idx, accessor) => {
Expand Down Expand Up @@ -615,15 +614,13 @@ define(['jquery', 'handlebars'], ($, Handlebars) => {
const opts = $.extend(true, {}, this.options);
this.options = $.extend(true, {}, opts, args);

let arg;
for (arg in args) {
Comment on lines -618 to -619
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

combined

for (const arg in args) {
if (args[arg] === undefined && this.options[arg] !== undefined) {
delete this.options[arg];
}
}

let attribute;
for (attribute in this.__attributes) {
for (const attribute in this.__attributes) {
if (this.options[attribute] !== undefined) {
const setter = this.__attributes[attribute].setter;
this[setter](this.options[attribute]);
Expand All @@ -645,12 +642,12 @@ define(['jquery', 'handlebars'], ($, Handlebars) => {
if (this.options.template) {
$.ajax(this.options.template)
.done(
$.proxy(function (res) {
$.proxy(function () {
this.templateSuccess.apply(this, arguments);
}, this)
)
.fail(
$.proxy(function (res) {
$.proxy(function () {
this.templateFailure.apply(this, arguments);
}, this)
);
Expand All @@ -662,10 +659,7 @@ define(['jquery', 'handlebars'], ($, Handlebars) => {
templateSuccess: function (templateString) {
const template = Handlebars.compile(templateString);

const html = template();

const res = template(this.templateContent());

const $res = $.jqElem('span').append(res);
this._rewireIds($res, this);

Expand Down Expand Up @@ -727,7 +721,7 @@ define(['jquery', 'handlebars'], ($, Handlebars) => {
setValuesForKeys: function setValuesForKeys(obj) {
const objCopy = $.extend({}, obj);

for (attribute in this.__attributes) {
for (const attribute in this.__attributes) {
if (objCopy[attribute] !== undefined) {
const setter = this.__attributes[attribute].setter;
this[setter](objCopy[attribute]);
Expand Down Expand Up @@ -769,7 +763,7 @@ define(['jquery', 'handlebars'], ($, Handlebars) => {
$elem.removeAttr('id');
}

$.each($elem.find('[id]'), function (idx) {
$.each($elem.find('[id]'), function () {
$target.data($(this).attr('id'), $(this));
$(this).attr('data-id', $(this).attr('id'));
$(this).removeAttr('id');
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/*
/*
Listselect Renderer

Provides a select list that allows the selection of one or multiple data items that can be filtered by their attributes. The attribute to be filtered will be displayed as the label in the selection list. Filters can be chained by pressing the enter key in the filter box.
Expand Down Expand Up @@ -140,7 +140,7 @@

render: function (index) {
const renderer = rendererListselect[index];

let button_span, result_list;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

need to be declared at this level as the previous code took advantage of variable hoisting

if (renderer.settings.navigation_url) {
renderer.settings.navigation_callback = renderer.update_data;
}
Expand Down Expand Up @@ -231,14 +231,9 @@
renderer.settings.filter_attribute + ' <span class="caret"></span>';
const filter_list = document.createElement('ul');
filter_list.setAttribute('class', 'dropdown-menu');
filter_list.setAttribute(
'style',
renderer.settings.extra_wide
? 'max-height: 200px; overflow: auto;'
: 'max-height: 200px; overflow: auto;'
Comment on lines -237 to -238
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I love me some ternaries, but this one seems a little unnecessary...

);
filter_list.setAttribute('style', 'max-height: 200px; overflow: auto;');
let filter_string = '';
for (var i = 0; i < renderer.settings.filter.length; i++) {
for (let i = 0; i < renderer.settings.filter.length; i++) {
filter_string +=
'<li><a onclick="rendererListselect[' +
renderer.index +
Expand All @@ -265,7 +260,7 @@
'style',
'font-size: 9px; position: relative; top: -5px;'
);
for (var i = 0; i < renderer.settings.filter_breadcrumbs.length; i++) {
for (let i = 0; i < renderer.settings.filter_breadcrumbs.length; i++) {
const bc_button = document.createElement('button');
bc_button.setAttribute('class', 'btn btn-mini');
bc_button.setAttribute('style', 'margin-right: 3px;');
Expand All @@ -276,7 +271,7 @@
': ' +
renderer.settings.filter_breadcrumbs[i][1] +
' <span style="font-size: 11px; color: gray;">x</span>';
bc_button.addEventListener('click', function (event) {
bc_button.addEventListener('click', function () {
rendererListselect[index].removeBreadcrumb(this, index);
});
filter_breadcrumbs.appendChild(bc_button);
Expand All @@ -285,7 +280,7 @@
// check for multi-select vs single select
if (renderer.settings.multiple) {
// create the result list
var result_list = document.createElement('select');
const result_list = document.createElement('select');
result_list.setAttribute('multiple', '');
result_list.setAttribute('style', 'width: 415px');
result_list.setAttribute('size', renderer.settings.rows);
Expand All @@ -294,7 +289,7 @@
renderer.redrawResultlist(result_list, index);

// create the action buttons
var button_span = document.createElement('span');
button_span = document.createElement('span');
button_span.setAttribute('style', 'position: relative; bottom: 100px;');
const button_left = document.createElement('a');
button_left.setAttribute('class', 'btn btn-small btn-default');
Expand Down Expand Up @@ -411,12 +406,12 @@
? renderer.settings.button.icon
: '<i class="fa fa-check"></i>');
if (typeof renderer.settings.callback == 'function') {
var index = renderer.index;
const index = renderer.index;
if (renderer.settings.multiple) {
submit_button.addEventListener('click', () => {
const selection_result = [];
if (renderer.settings.return_object) {
for (var x = 0; x < result_list.options.length; x++) {
for (let x = 0; x < result_list.options.length; x++) {
for (let y = 0; y < renderer.settings.data.length; y++) {
if (
result_list.options[x].value ==
Expand All @@ -428,7 +423,7 @@
}
}
} else {
for (var x = 0; x < result_list.options.length; x++) {
for (let x = 0; x < result_list.options.length; x++) {
selection_result.push(result_list.options[x].value);
}
}
Expand Down Expand Up @@ -550,7 +545,7 @@
redrawResultlist: function (result_list, index) {
const renderer = rendererListselect[index];
const result_list_array = [];
for (var i = 0; i < renderer.settings.selection_data.length; i++) {
for (let i = 0; i < renderer.settings.selection_data.length; i++) {
result_list_array.push([
renderer.settings.selection_data[i][renderer.settings.value],
'<option value="' +
Expand All @@ -566,7 +561,7 @@
result_list_array.sort(renderer.listsort);
}
let result_list_string = '';
for (var i = 0; i < result_list_array.length; i++) {
for (let i = 0; i < result_list_array.length; i++) {
result_list_string += result_list_array[i][1];
}
result_list.innerHTML = result_list_string;
Expand All @@ -579,7 +574,7 @@
renderer.settings.filtered_data = renderer.settings.data;

// apply all filter breadcrumbs
for (var i = 0; i < renderer.settings.filter_breadcrumbs.length; i++) {
for (let i = 0; i < renderer.settings.filter_breadcrumbs.length; i++) {
renderer.settings.filtered_data = renderer.filter(
{
data: renderer.settings.filtered_data,
Expand Down Expand Up @@ -609,7 +604,7 @@

// create the selection list
let settings_string = '';
for (var i = 0; i < renderer.settings.filtered_data.length; i++) {
for (let i = 0; i < renderer.settings.filtered_data.length; i++) {
if (
!renderer.settings.selection[
renderer.settings.filtered_data[i][renderer.settings.value]
Expand Down Expand Up @@ -708,7 +703,7 @@
}
},
update_data: function (params, index) {
const renderer = rendererListselect[index];
let renderer = rendererListselect[index];

if (typeof params == 'string' && params == 'more') {
renderer.settings.offset = renderer.settings.data.length;
Expand Down Expand Up @@ -777,7 +772,7 @@
headers: headers,
dataType: 'json',
success: function (data) {
const renderer = rendererListselect[index];
renderer = rendererListselect[index];
renderer.settings.total_count = data.total_count;
if (typeof params == 'string' && params == 'more') {
renderer.settings.data = renderer.settings.data.concat(data.data);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ define(['kbwidget', 'bootstrap', 'jquery', 'narrativeConfig', 'kbaseNarrativeInp
Config,
kbaseNarrativeInput
) => {
'use strict';
return KBWidget({
name: 'kbaseDefaultNarrativeInput',
parent: kbaseNarrativeInput,
Expand Down Expand Up @@ -48,7 +49,7 @@ define(['kbwidget', 'bootstrap', 'jquery', 'narrativeConfig', 'kbaseNarrativeInp
p.default !== '' && p.default !== undefined
? " placeholder='" + p.default + "'"
: '';
input =
const input =
"<input class='form-control' style='width: 95%' name='" +
pid +
"'" +
Expand Down Expand Up @@ -93,7 +94,7 @@ define(['kbwidget', 'bootstrap', 'jquery', 'narrativeConfig', 'kbaseNarrativeInp
$(this.$elem)
.find('[name^=param]')
.filter(':input')
.each((key, field) => {
.each((_key, field) => {
let value = field.value;
if (!value) value = field.placeholder;
paramList.push(value.trim());
Expand All @@ -117,7 +118,7 @@ define(['kbwidget', 'bootstrap', 'jquery', 'narrativeConfig', 'kbaseNarrativeInp
$(this.$elem)
.find('[name^=param]')
.filter(':input')
.each((key, field) => {
.each((_key, field) => {
state[field.name] = field.value;
});

Expand All @@ -135,18 +136,16 @@ define(['kbwidget', 'bootstrap', 'jquery', 'narrativeConfig', 'kbaseNarrativeInp
$(this.$elem)
.find('[name^=param]')
.filter(':input')
.each((key, field) => {
.each((_key, field) => {
const $field = $(field);
const fieldName = $field.attr('name');

// If it's a text field, just dump the value in there.
if ($field.is('input') && $field.attr('type') === 'text') {
$field.val(state[fieldName]);
}

// If it's a select field, do the same... we'll have comboboxen or something,
// eventually, so I'm just leaving this open for that.
else if ($field.is('select')) {
Comment on lines -143 to -149
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

these do the same thing

// If it's a select field, do the same.
if (
($field.is('input') && $field.attr('type') === 'text') ||
$field.is('select')
) {
$field.val(state[fieldName]);
}
});
Expand Down Expand Up @@ -223,7 +222,7 @@ define(['kbwidget', 'bootstrap', 'jquery', 'narrativeConfig', 'kbaseNarrativeInp
// case 3 - data, need new datalist
// case 4 - data, need to update existing datalist
else if (objList.length > 0) {
var $datalist;
let $datalist;
if (!datalistID) {
datalistID = this.genUUID();
$input.attr('list', datalistID);
Expand All @@ -233,9 +232,9 @@ define(['kbwidget', 'bootstrap', 'jquery', 'narrativeConfig', 'kbaseNarrativeInp
$datalist = $(this.$elem.find('#' + datalistID));
}
$datalist.empty();
for (let j = 0; j < objList.length; j++) {
for (const element of objList) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

sonarcloud substitution

$datalist.append(
$('<option>').attr('value', objList[j][1]).append(objList[j][1])
$('<option>').attr('value', element[1]).append(element[1])
);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -258,10 +258,8 @@ define(['kbwidget', 'bootstrap', 'jquery', 'narrativeConfig', 'kbaseNarrativePar
// handle case with multiple fields
if (this.$elem.find('#' + this.spec.id).prop('checked')) {
return this.checkedValue;
} else {
return this.uncheckedValue;
}
return '';
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

never reached

return this.uncheckedValue;
},
});
});
Loading