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
Show file tree
Hide file tree
Changes from 12 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
6 changes: 4 additions & 2 deletions app/directives/challenge-tile/challenge-tile.directive.jade
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
93 changes: 21 additions & 72 deletions app/directives/external-account/external-account.directive.js
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,32 @@
templateUrl: 'directives/external-account/external-account.directive.html',
scope: {
linkedAccounts: '=',
linksData: '=',
// linksData: '=',
Copy link
Contributor

Choose a reason for hiding this comment

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

delete the line if we dont need this anymore

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;
}
});

Expand Down Expand Up @@ -92,11 +95,11 @@
.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];
// delete $scope.linksData[provider.provider];
Copy link
Contributor

Choose a reason for hiding this comment

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

please remove this commented line

toaster.pop('success', "Success",
String.supplant(
"Your {provider} account has been unlinked.",
Expand All @@ -120,60 +123,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
120 changes: 19 additions & 101 deletions app/directives/external-account/external-account.directive.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,127 +14,45 @@ describe('External Accounts Directive', function() {
describe('Linked external accounts', function() {
var linkedAccounts = [
{
providerType: 'linkedin',
// don't care about other details
provider: 'linkedin',
data: {
// don't care about other details
}
},
{
providerType: 'github'
provider: 'github',
data: {
// don't care about other details
}
}
];
var linksData = {
'linkedin' : {provider: 'linkedin', name: 'name-linkedin'},
'github' : {provider: 'github', name: 'name-github'}
};
var externalAccounts;

beforeEach(function() {
scope.linkedAccounts = linkedAccounts;
scope.linksData = linksData;
element = angular.element('<external-accounts linked-accounts="linkedAccounts", links-data="linksData"></external-accounts>)');
element = angular.element('<external-accounts linked-accounts="linkedAccounts"></external-accounts>)');
externalAccounts = $compile(element)(scope);
scope.$digest();
// scope.$apply();
});

it('should have added account list to scope', function() {
expect(element.isolateScope().accountList).to.exist;

});

it('should have "linked" property set for github & linkedin', function() {
var githubAccount = _.find(element.isolateScope().accountList, function(a) { return a.provider === 'github'});
expect(githubAccount).to.have.property('status')
.that.equals('linked');

// var linkeindAccount = _.find(element.isolateScope().accountList, function(a) { return a.provider === 'linkedin'});
// expect(linkeindAccount).to.have.property('linked')
// .that.equals(true);
var githubAccount = _.find(element.isolateScope().accountList, function(a) {
return a.provider === 'github'
});
expect(githubAccount).to.have.property('status').that.equals('linked');
});
});
});

describe('External Links Data Directive', function() {
var scope;
var element;

beforeEach(function() {
bard.appModule('topcoder');
bard.inject(this, '$compile', '$rootScope');
scope = $rootScope.$new();
});

bard.verifyNoOutstandingHttpRequests();

describe('Linked external accounts', function() {
var externalLinks = [
{
providerType: 'linkedin',
// don't care about other details
},
{
providerType: 'github'
},
{
providerType: 'behance'
},
{
providerType: 'dribbble'
},
{
providerType: 'bitbucket'
}
];
var linkedAccounts = {
github: {
handle: "github-handle",
followers: 1,
publicRepos: 1
},
stackoverflow: {
handle: 'so-handle',
reputation: 2,
answers: 2
},
behance: {
name: 'behance name',
projectViews: 3,
projectAppreciations: 3
},
dribbble: {
handle: 'dribble-handle',
followers: 4,
likes: 4
},
bitbucket: {
username: 'bitbucket-username',
followers: 5,
repositories: 5
},
twitter: {
handle: 'twitter-handle',
noOfTweets: 6,
followers: 6
},
linkedin: {
name: 'linkedin name',
title: 'linkedin title'
}
};
var externalLinksData;

beforeEach(function() {
scope.linkedAccounts = linkedAccounts;
scope.externalLinks = externalLinks;
element = angular.element('<external-links-data linked-accounts-data="linkedAccounts" external-links="externalLinks"></external-links-data>)');
externalLinksData = $compile(element)(scope);
it('should have pending status for stackoverflow ', function() {
scope.linkedAccounts.push({provider: 'stackoverflow', data: {status: 'PENDING'}});
scope.$digest();
var soAccount = _.find(element.isolateScope().accountList, function(a) {
return a.provider === 'stackoverflow'
});
expect(soAccount).to.have.property('status').that.equals('pending');
});

it('should have added linkedAccounts to scope', function() {
expect(element.isolateScope().linkedAccounts).to.exist;
// linkedAccounts should have 5 entries because externalLinks contains only 5 records
expect(element.isolateScope().linkedAccounts).to.have.length(5);
});

});
});
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
.external-link-list
div.external-link-tile(ng-repeat="account in linkedAccounts")
div.external-link-tile(ng-repeat="account in linkedAccountsData")
.top
div.logo
i.fa(ng-class="(account|providerData:'className') || 'fa-globe'")
Expand Down Expand Up @@ -97,3 +97,8 @@
.handle {{account.data.name}}

.title {{account.data.title}}

div(ng-switch-when="weblink")
p.link-title(data-ellipsis, ng-bind="account.title")

a.link-url(ng-href="{{account.URL}}", ng-bind="account.URL")
22 changes: 22 additions & 0 deletions app/directives/external-account/external-links-data.directive.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
(function() {
'use strict';

/**
* @desc links data card directive
* @example <external-links-data></external-links-data>
*/
angular
.module('tcUIComponents')
.directive('externalLinksData', externalLinksData);

function externalLinksData() {
var directive = {
restrict: 'E',
templateUrl: 'directives/external-account/external-link-data.directive.html',
scope: {
linkedAccountsData: '='
}
};
return directive;
}
})();
Loading