Skip to content
This repository was archived by the owner on Mar 4, 2025. It is now read-only.

SUP-2481 intergrate web links api #556

Merged
merged 18 commits into from
Nov 30, 2015
Merged

SUP-2481 intergrate web links api #556

Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
4e4fa38
SUP-1611, Settings: Support adding an external web link.
Oct 6, 2015
dbaaab5
Merge branch 'dev' into feature/sup-1611-web-link-support-ui
Oct 24, 2015
c84af59
major refactoring, moving data processing to services
Oct 26, 2015
5a62e35
more fixes
Oct 27, 2015
c254046
SUP-2481, intergrate-web-links-api
Nov 4, 2015
13b4020
SUP-2481, intergrate-web-links-api
Nov 6, 2015
6c57b2c
Merge branch 'dev' into feature/sup-2481-intergrate-web-links-api
Nov 10, 2015
a1df4e4
SUP-2481, intergrate-web-links-api
Nov 10, 2015
11583ef
Merge branch 'dev' into feature/sup-2481-intergrate-web-links-api
Nov 16, 2015
34eb382
SUP-2481, Integrate w/ API for External web links
Nov 16, 2015
8e7941b
SUP-2481, intergrate-web-links-api
Nov 16, 2015
39c198a
SUP-2481, intergrate-web-links-api
Nov 16, 2015
92d84b3
SUP-2481, intergrate-web-links-api
Nov 17, 2015
9692e5f
SUP-2481, intergrate-web-links-api
Nov 17, 2015
48af033
Merge branch 'dev' into feature/sup-2481-intergrate-web-links-api
Nov 18, 2015
6167b0b
SUP-2481, intergrate-web-links-api
Nov 18, 2015
fda6af1
SUP-2481, intergrate-web-links-api
Nov 24, 2015
0b3e95d
Merge branch 'dev' into feature/sup-2481-intergrate-web-links-api
Nov 24, 2015
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
6 changes: 4 additions & 2 deletions app/directives/challenge-tile/challenge-tile.directive.jade
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@
// Only show if not data science track
p.roles
span(ng-hide="challenge.track === 'DATA_SCIENCE'")
#[span Role: ] #[span {{challenge.userDetails.roles | listRoles}}]
span Role:  
span {{challenge.userDetails.roles | listRoles}}

.completed-challenge(
ng-show="challenge.status === 'COMPLETED' || challenge.status === 'PAST'",
Expand Down Expand Up @@ -65,7 +66,8 @@
// Only show if not data science track
p.roles
span(ng-hide="challenge.track === 'DATA_SCIENCE'")
#[span Role: ] #[span {{challenge.userDetails.roles | listRoles}}]
span Role:  
span {{challenge.userDetails.roles | listRoles}}

.challenge.list-view(ng-show="view=='list'", ng-class="challenge.track")
.active-challenge(ng-show="challenge.status === 'ACTIVE'")
Expand Down
104 changes: 29 additions & 75 deletions app/directives/external-account/external-account.directive.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,10 @@
{ provider: "linkedin", className: "fa-linkedin", displayName: "LinkedIn", disabled: true, order: 5, colorClass: 'el-linkedin', featured: true},
{ provider: "stackoverflow", className: "fa-stack-overflow", displayName: "Stack Overflow", disabled: false, order: 3, colorClass: 'el-stackoverflow'},
{ provider: "behance", className: "fa-behance", displayName: "Behance", disabled: true, order: 2, colorClass: 'el-behance'},
// { provider: "google-oauth2", className: "fa-google-plus", displayName: "Google+", disabled: true, order: }, colorClass: 'el-dribble',
{ provider: "github", className: "fa-github", displayName: "Github", disabled: false, order: 1, colorClass: 'el-github', featured: true},
{ provider: "bitbucket", className: "fa-bitbucket", displayName: "Bitbucket", disabled: false, order: 7, colorClass: 'el-bitbucket'},
{ provider: "twitter", className: "fa-twitter", displayName: "Twitter", disabled: true, order: 4, colorClass: 'el-twitter'},
{ provider: "weblinks", className: "fa-globe", displayName: "Web Links", disabled: true, order: 8, colorClass: 'el-weblinks'}
{ provider: "weblink", className: "fa-globe", displayName: "Web Links", disabled: true, order: -1, colorClass: 'el-weblinks'}
// TODO add more
];

Expand All @@ -20,28 +19,36 @@
templateUrl: 'directives/external-account/external-account.directive.html',
scope: {
linkedAccounts: '=',
linksData: '=',
readOnly: '='
},
controller: ['$log', '$scope', 'ExternalAccountService', 'toaster',
function($log, $scope, ExternalAccountService, toaster) {

$log = $log.getInstance("ExtAccountDirectiveCtrl")
$scope.accountList = _.clone(_supportedAccounts, true);

var _accountList = _.clone(_supportedAccounts, true);
_.remove(_accountList, function(al) { return al.order < 0});
$scope.$watchCollection('linkedAccounts', function(newValue, oldValue) {
for (var i=0;i<$scope.accountList.length;i++) {
var _idx = _.findIndex(newValue, function(a) {
return $scope.accountList[i].provider === a.providerType;
});
if (_idx == -1) {
$scope.accountList[i].status = 'unlinked';
} else {
// check if data
if ($scope.linksData[$scope.accountList[i].provider]) {
$scope.accountList[i].status = 'linked';
if (newValue) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@parthshah please let me know if we really need to compare newValue against oldValue for this check. Earlier, it was doing the check newValue != oldValue which was causing the wrong states of the accounts when we change the values in existing linkedAccount variable. This was also causing unit tests(external-account.directive.spec.js) to not work as expected and I saw some workarounds ($inject and $timeout) there. I have removed those as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

In general, it's my understanding that new and old values should always be compared before performing any intensive work to reduce unnecessary updates. Do you think that check needs to be altered since we are watching a collection?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think $watchCollection checks one layer deep already by default. So unless we're checking nested elements, we should only see a newValue if Angular detected changes already. Although, we should test this to make sure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree with Nick, specially when we are watching objects/arrays in comparison to primitive values. Further, the equality operator checks for the references which is always same if alter (push/pop) the existing array.

angular.forEach(_accountList, function(account) {
var _linkedAccount = _.find(newValue, function(p) {
return p.provider === account.provider
});
var accountStatus = _.get(_linkedAccount, 'data.status', null);
if (!_linkedAccount) {
account.status = 'unlinked';
} else if(accountStatus && accountStatus.toLowerCase() === 'pending') {
account.status = 'pending';
} else {
$scope.accountList[i].status = 'pending';
account.status = 'linked';
}
}
});
$scope.accountList = _accountList;
} else {
// reset the status for all accounts
angular.forEach(_accountList, function(account) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@parthshah do you think we should do this? I did that while writing unit tests for the directive. The use case is that whenever we set the linkedAccounts to null values, ideally it should reset the accountList state to indicate that no account is linked yet. Correct? Test is titled with "should reset accountList when linkedAccounts set to null"

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this makes sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

delete account.status;
});
}
});

Expand All @@ -62,7 +69,7 @@
ExternalAccountService.linkExternalAccount(provider.provider, null)
.then(function(resp) {
$log.debug("Social account linked: " + JSON.stringify(resp));
$scope.linkedAccounts.push(resp.profile);
$scope.linkedAccounts.push(resp.linkedAccount);
toaster.pop('success', "Success",
String.supplant(
"Your {provider} account has been linked. Data from your linked account will be visible on your profile shortly.",
Expand Down Expand Up @@ -92,11 +99,12 @@
.then(function(resp) {
$log.debug("Social account unlinked: " + JSON.stringify(resp));
var toRemove = _.findIndex($scope.linkedAccounts, function(la) {
return la.providerType === provider.provider;
return la.provider === provider.provider;
});
// remove from both links array and links data array
$scope.linkedAccounts.splice(toRemove, 1);
delete $scope.linksData[provider.provider];
if (toRemove > -1) {
// remove from the linkedAccounts array
$scope.linkedAccounts.splice(toRemove, 1);
}
toaster.pop('success', "Success",
String.supplant(
"Your {provider} account has been unlinked.",
Expand All @@ -120,60 +128,6 @@
]
};
})
.directive('externalLinksData', function() {
return {
restrict: 'E',
templateUrl: 'directives/external-account/external-link-data.directive.html',
scope: {
linkedAccountsData: '=',
externalLinks: '='
},
controller: ['$log', '$scope', 'ExternalAccountService',
function($log, $scope, ExternalAccountService) {
$log = $log.getInstance('ExternalLinksDataDirective');
var validProviders = _.pluck(_supportedAccounts, 'provider');
function reCalcData(links, data) {
$scope.linkedAccounts = [];
angular.forEach(links, function(link) {

var provider = link.providerType;
var isValidProviderIdx = _.findIndex(validProviders, function(p) {
return p === provider;
});
// skip if we dont care about this provider
if (isValidProviderIdx == -1)
return;

if (!data[provider]) {
$scope.linkedAccounts.push({
provider: provider,
data: {
handle: link.name,
status: 'PENDING'
}
});
} else {
// add data
$scope.linkedAccounts.push({
provider: provider,
data: data[provider]
});
}
});
}


$scope.$watch('linkedAccountsData', function(newValue, oldValue) {
reCalcData($scope.externalLinks, newValue);
});

$scope.$watchCollection('externalLinks', function(newValue, oldValue) {
reCalcData(newValue, $scope.linkedAccountsData);
});
}
]
}
})
.filter('providerData', function() {
return function(input, field) {
return _.result(_.find(_supportedAccounts, function(s) {
Expand Down
Loading