[REF] Pull out anonymous javascript function to make it testable

[demeritcowboy]

Overview

I want to write a test for this function but can't because it's anonymous and it's part of an ajax event handler but that doesn't work with the jasmine/karma tests.

Technical Details

There's a tradeoff with making any private function public, but I don't think this was written originally with hiding variables/functions in mind, just that the "standard" way to write event handlers is with anonymous functions. It just makes them hard to test if they do stuff that would be nice to unit test on their own.

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[colemanw]

I'm happy with this change but I think it could be further improved by separating the business-logic-y part of the function from the ajax-y part of the function. What about leaving it as an anonymous function and keeping the stuff like var newType = _.values(data.relationshipType)[0]; and scope.digest() inside that anonymous function but break out the business logic into a separate function that takes newType as a param.

[demeritcowboy]

That makes sense.
I'm not an angular expert but I think that separation from the $scope.$digest would then require one slight change. By making this separation, the caller of the new function can no longer make assumptions about what the called function is doing. So either I should keep the $scope.$digest in the new function where it knows what it just did, or the caller needs to call $scope.$apply to make sure it catches everything the called function might do, which might include changes outside the current scope - it doesn't currently but if decoupled then it can't assume that. I've gone with the latter.

[colemanw]

Yea that's even better.