Added support for various endpoints to Paper Badget widget.

[Alicole]

Changes for multiple type endpoint support.

Need to update documentation for:

i). article-doi only, use:

showBadges({"article-doi" : "10.1186/2047-217X-3-18", "container-class" : "badge-container"});
or showBadgeFurniture({"article-doi" : "10.1186/2047-217X-3-18", "furniture-class" : "paper-badges-hidden"});

ii). Orcid only

showBadges({"orcid" : "0000-0001-5979-8713", "container-class" : "badge-container"});
or showBadgeFurniture({"orcid" : "0000-0001-5979-8713", "furniture-class" : "paper-badges-hidden"});

iii). article-doi for specified user:

showBadges({"article-doi" : "10.1186/2047-217X-3-18", "orcid" : "0000-0001-5979-8713", "container-class" : "badge-container"});
or showBadgeFurniture({"article-doi" : "10.1186/2047-217X-3-18", "orcid" : "0000-0001-5979-8713", "furniture-class" : "paper-badges-hidden"});

[abbycabs]

Thanks! Will take a look later.

Is anyone from ORCID is up for reviewing this? Since you're the primary audience :) @rcpeters @wjrsimpson Liz

[rcpeters]

@acabunoc does CI breaking matter?

[abbycabs]

No, we never got that working properly on forks

On Sep 24, 2015, at 2:37 PM, Robert Peters [email protected] wrote:

@acabunoc does CI breaking matter?

—
Reply to this email directly or view it on GitHub.

[rcpeters]

Code wise this works. Style wise I find having both + and join('/') confusing:

    function getEndpoint(confIn, count) {
        var endPoint=[];
        endPoint.push("https://badges.mozillascience.org");
        if (confIn["article-doi"]) endPoint.push("papers/" + confIn["article-doi"]);
        if (confIn["orcid"]) endPoint.push("users/" + confIn["orcid"]);
        endPoint.push("/badges");
        if (count) endPoint.push("/count" + count)
        return endPoint.join("/");
    }

    function getEndpoint(confIn, count) {
        var endPoint = "https://badges.mozillascience.org";
        if (confIn["article-doi"]) endPoint += "papers/" + confIn["article-doi"];
        if (confIn["orcid"]) endPoint += "users/" + confIn["orcid"];
        endPoint += "/badges";
        if (count) endPoint += "/count" + count;
        return endPoint;
    }

Would either of those be clearer? I'm not sure.

[Alicole]

Hi Rob,

Please don’t look at this from a semantic perspective.

Every function was chosen carefully and because of the function they provided.

“Push” in this case, ensures the correct order is maintained for every part of each possible endpoint supported.

“Join” collects and renders each part of the composition, without a need for loop and ensures they are separated by “/” – all this encapsulated in a single function, simple and elegant – keeping the code clean, free from confusing clutter.

And the last part acts as a test, so “/count” is only added as a suffix when required.

Kindest regards,

Al.

From: Robert Peters [mailto:[email protected]]
Sent: 24 September 2015 19:42
To: mozillascience/PaperBadger
Cc: Coleman, Alistair
Subject: Re: [PaperBadger] Added support for various endpoints to Paper Badget widget. (#131)

In public/widgets/paper-badger-widget.jshttps://github.com//pull/131#discussion_r40356064:

@@ -87,22 +87,44 @@ function splitArray(arr, lookGroup, taxonomyClass)

  return ttt;

}

+function getEndpoint(confIn, count)

Code wise this works. Style wise I find having both + and join('/') confusing:

function getEndpoint(confIn, count) {

    var endPoint=[];

    endPoint.push("https://badges.mozillascience.org");

    if (confIn["article-doi"]) endPoint.push("papers/" + confIn["article-doi"]);

    if (confIn["orcid"]) endPoint.push("users/" + confIn["orcid"]);

    endPoint.push("/badges");

    if (count) endPoint.push("/count" + count)

    return endPoint.join("/");

}



function getEndpoint(confIn, count) {

    var endPoint = "https://badges.mozillascience.org";

    if (confIn["article-doi"]) endPoint += "papers/" + confIn["article-doi"];

    if (confIn["orcid"]) endPoint += "users/" + confIn["orcid"];

    endPoint += "/badges";

    if (count) endPoint += "/count" + count;

    return endPoint;

}

Would either of those be clearer? I'm not sure.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/131/files#r40356064.

[abbycabs]

Hi both!

I'm going to r+ and merge this since it works and it's a great feature.

However, I would prefer clearer semantics help onboard new contributors (clever semantics can make it hard for new ppl to join in 😄)