[UI Side] Changes add/remove member request to contain a body

[JorWo]

Ticket Link

Groupings-1214

List of squashed commits

• Changed all add and remove member functions in general.controller.js to make a PUT request with an ArrayList instead of a comma-separated string of members
• Added makeApiRequestWithBody and makeApiRequestWithBodyAndTimeoutModal to groupings.service.js
• Changed GroupingsRestController to expect a PUT request with a body instead of a header
• Added sanitizeList helper method with 100% Jacoco test coverage to GroupingsRestController to sanitize a list of strings
• Added another makeApiRequestBody in HttpRequestService that takes a list of data instead of string in the parameter
• Added JsonUtil and JsonUtilTest from the API side to use in unit tests (Convert list into json when performing mockMvc.content())
• Updated Unit and Jasmine tests

Test Checklist

• Unit Tests Passed:
• Jasmine Tests Passed:
• General Visual Inspection:

[mhodgesatuh]

Codacy has issues with the PR. I note that for the injection sync it doesn't have advice. Found this, https://stackoverflow.com/questions/51715616/how-to-fix-codacy-alert-generic-object-injection-sink, which reminds me of a conversation we had a while back, so it might not be the best of advice.

[JorWo]

Codacy has issues with the PR. I note that for the injection sync it doesn't have advice. Found this, https://stackoverflow.com/questions/51715616/how-to-fix-codacy-alert-generic-object-injection-sink, which reminds me of a conversation we had a while back, so it might not be the best of advice.

I think we should disable the generic object injection sink because it gives an error anywhere a variable is passed into square brackets. I found that the author of the rule said that the rule is too noisy and it was designed to assist with manual checks: eslint-community/eslint-plugin-security#21 (comment)

[mhodgesatuh]

I'm fine with disabling this check. If you have time, can you locate it for me? Otherwise it might be next week before I get to it. This week is a very busy one from hereon. There are other Codacy issues though that should be addressed.

[JorWo]

I'm fine with disabling this check. If you have time, can you locate it for me? Otherwise it might be next week before I get to it. This week is a very busy one from hereon. There are other Codacy issues though that should be addressed.

Here is the link, scroll down to "Detect object injection": https://app.codacy.com/p/383092/patterns/list?engine=f8b29663-2cb2-498d-b923-a10c6a8c05cd&category=2&status=true

[mhodgesatuh]

Not sure why the following. Why is removing 1 handled differently than 2+? Necessary? If so the inline comments should explain since not intuitive.

$scope.membersToModify = []; if (membersToRemove.length > 1) { removeMembers(membersToRemove, listName); } else { $scope.userInput = membersToRemove[0];

[devgav]

As far as the codacy error goes i think they want you to do something like

res[parseInt(i)].result;
source: https://stackoverflow.com/questions/51715616/how-to-fix-codacy-alert-generic-object-injection-sink

obviously that looks confusing. An alternative you could try is to do a for....of loop.

More on for...of loops => https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/for...of

[mhodgesatuh]

Given that Codacy doesn't feel like this check meets their standards, maybe to ignore the particular warning for now until we know better what the issue is and how to address it. I'm fine with that strategy for now.

[devgav]

I think we should try that for...of loop since it seems like codacy is just complaining about accessing the elements in our array with the brackets and our index. The for...of loop essentially does the same thing without needing the index or the brackets and needing to access our elements with [i].

[devgav]

Perfect!