[ACHIEVEMENT] The Gladiator - achievement for getting all the reaction to a comment

[dunaevsky]

The commenter gets the achievement.

-It still does not work for inline comments

-You can also add reactions to the PR itself, I wanted to give an achievement to the creator also, but I didn't see it being audited. @thatkookooguy LMKWYT

-I am uncertain about the name, so would like suggestions

-Big up and much 👍 to @ortichon who helped me de-bug this shit

[thatkookooguy]

@dunaevsky opened PR #52 so you'll have the data you want. you can review that one first so you'll have the data available to work in this branch

[thatkookooguy]

Also added a data example in PullRequestExample.MD but it's pretty much the same as reactions on comments :-)

[ortichon]

@dunaevsky let me know when you finish all your changes and I'll start reviewing.

[dunaevsky]

@ortichon @thatkookooguy I'ma put a pin on it for tonight and pick up tomorrow sometime, Since there's 2 features to add.

If you have suggestions for the "short" and "description" it will be great.
I like it being called the Gladiator.

[thatkookooguy]

@dunaevsky Cool. Just add the help wanted label so we'll know this is not up for review yet and just brainstorming

[dunaevsky]

@thatkookooguy Already have 3 labels since this was my goal here from the get go

[thatkookooguy]

Here's how I made a function that gives back all comments (or pull request) creators that got all 6 reactions:

function getCommentAuthorsWithAllReactions(pullRequest) {
    var allComments = _.concat(pullRequest.comments, pullRequest.inlineComments);
    var authors = _.map(allComments, 'author.username');
    var AllCommentsReactions = _.map(allComments, 'reactions');

    // also add pull request description reactions
    authors.push(pullRequest.creator.username);
    AllCommentsReactions.push(pullRequest.reactions);

    getOnlyUniqueReactionsWithoutAuthors(AllCommentsReactions, authors);

    return onlyUsersWithAllReactions(authors, AllCommentsReactions);
}

function reactionsWithoutAuthor(reactions, author) {
    return _.map(_.reject(reactions, ['user.username', author]), 'reaction');
}

function getOnlyUniqueReactionsWithoutAuthors(AllCommentsReactions, authors) {
    _.forEach(AllCommentsReactions, function(reactions, index) {
        AllCommentsReactions[index] =
            _.uniq(reactionsWithoutAuthor(reactions, authors[index]));

    });
}

function onlyUsersWithAllReactions(authors, AllCommentsReactions) {
    var commentAuthorsWithAllReactions = [];

    _.forEach(authors, function(author, index) {
        if (AllCommentsReactions[index].length === 6) {
            commentAuthorsWithAllReactions.push(author);
        }
    });

    return commentAuthorsWithAllReactions;
}

I try and remove all the data that I don't need so that data would be easier to understand.

1. get all comments (of all types)
2. from that, create two connected arrays of authors and reactions to that author's comment
3. add the pull request creator as another author, and the pull request reactions to the reactions array

   Now, the PR data and the comments data are the same. from here, on, the code doesn't know it came from different sources

4. for each reactions array, remove any reaction by the text author, and remove all the data that doesn't interest us.

   This is done with _map to change every reaction object to the reaction string. At this point, we have all reactions not by the author in string format (example [ "laugh", "+1", "+1" ] ). Finally, we run _.uniq on that array to get only unique values (example: [ "laugh", "+1" ] )

5. Now, we have exactly the data we want to test. A list of authors we want to test for the achievements, and a list of unique reactions (not by the author). If all reactions were added, it means the unique reactions would be equal to 6 ('+1', '-1', 'laugh', 'confused', 'heart', 'hooray'). So, the final function, just creates an array of the authors that had 6 exactly reactions

So, when you call getCommentAuthorsWithAllReactions(pullRequest), you'll get all the usernames of the users who had something with all reactions but they weren't included in the calculation

getCommentAuthorsWithAllReactions(pullRequest)
// ["dunaevsky", "Thatkookooguy"]

[thatkookooguy]

Notice that I don't use _.isEmpty at all. lodash have lots of little tricks to save code from edge cases. For example, you checked that !_.isEmpty(comment.reactions), but if you just did the _.forEach with an empty array or even undefined, lodash will just return undefined

_.forEach(undefined, function(dog) { return console.log('woof') });
_.forEach([], function(dog) { return console.log('woof') });
// both print nothing

Since all lodash function do that, you will just get an empty array if getCommentAuthorsWithAllReactions didn't find reactions at all.

[thatkookooguy]

@dunaevsky - So, make the function work and let's start the review.

Please test the achievement (if you already got the achievement in the DB, delete it to test you get it again). Also, test it after you took the latest changes from master into your work
please lint your code with our new eslint file.

you can use eslint inside atom by installing the following packages:

• https://atom.io/packages/linter
• https://atom.io/packages/linter-eslint

Then, make sure that inside the linter-eslint settings, you check the box .

This will fix all common mistakes automatically for you when you save the file, and show you errors for things you need to fix manually.

restart atom, open achievibit, and the linter should recognize our settings file automatically and start showing you lint errors

[dunaevsky]

@thatkookooguy cool. I'll go over it. But gonna have to postpone it again

[coveralls]

Coverage decreased (-24.5%) to 18.939% when pulling c6c1ea5 on achiev-all-reactions into f6765b2 on master.

[dunaevsky]

I did it for the memberberry :*

[coveralls]

Coverage increased (+1.4%) to 44.854% when pulling b9e31d6 on achiev-all-reactions into f6765b2 on master.

[thatkookooguy]

E is uppercase

I think a ?! can work here

[thatkookooguy]

do you like this description? seems kind of too non-descriptive to me

[coveralls]

Coverage increased (+1.4%) to 44.854% when pulling 808304d on achiev-all-reactions into f6765b2 on master.