Feature: Refactor stats calculation

action.yml

@@ -23,6 +23,10 @@ inputs:
     description: 'The maximum number of rows to display in the table. A value of `0` means unlimited.'
     required: false
     default: 0
+  stats:
+    description: 'A comma separated list of the stats to be calculated and displayed.'
+    required: false
+    default: 'REVIEWS,TIME,COMMENTS'
   charts:
     description: 'Whether to add charts to the stats or not. Possible values: "true" or "false"'
     required: false

src/index.js

@@ -1,53 +1,17 @@
-const get = require('lodash.get');
 const core = require('@actions/core');
 const github = require('@actions/github');
 const execute = require('./execute');
+const { parseInputs } = require('./parsers');
 const { t } = require('./i18n');
 
-const parseArray = (value) => value.split(',');
-
-const getPeriod = () => {
-  const MAX_PERIOD_DATE = 365;
-  const value = parseInt(core.getInput('period'), 10);
-  return Math.min(value, MAX_PERIOD_DATE);
-};
-
-const getRepositories = (currentRepo) => {
-  const input = core.getInput('repositories');
-  return input ? parseArray(input) : [currentRepo];
-};
-
-const getPrId = () => get(github, 'context.payload.pull_request.node_id');
-
 const getParams = () => {
   const currentRepo = process.env.GITHUB_REPOSITORY;
-  const githubToken = core.getInput('githubToken');
-  const personalToken = core.getInput('token') || githubToken;
 
-  return {
+  return parseInputs({
+    core,
+    github,
     currentRepo,
-    githubToken,
-    personalToken,
-    org: core.getInput('organization'),
-    repos: getRepositories(currentRepo),
-    sortBy: core.getInput('sortBy') || core.getInput('sort-by'),
-    publishAs: core.getInput('publishAs') || core.getInput('publish-as'),
-    periodLength: getPeriod(),
-    displayCharts: core.getBooleanInput('charts'),
-    disableLinks: core.getBooleanInput('disableLinks') || core.getBooleanInput('disable-links'),
-    pullRequestId: getPrId(),
-    limit: parseInt(core.getInput('limit'), 10),
-    excludeStr: core.getInput('exclude'),
-    telemetry: core.getBooleanInput('telemetry'),
-    webhook: core.getInput('webhook'),
-    slack: {
-      webhook: core.getInput('slackWebhook') || core.getInput('slack-webhook'),
-      channel: core.getInput('slackChannel') || core.getInput('slack-channel'),
-    },
-    teams: {
-      webhook: core.getInput('teamsWebhook') || core.getInput('teams-webhook'),
-    },
-  };
+  });
 };
 
 const run = async () => {

src/interactors/__tests__/mergeStats.test.js

@@ -0,0 +1,56 @@
+const mergeStats = require('../mergeStats');
+const { VALID_STATS } = require('../../config/stats');
+const { authors, reviewStats, pullRequestStats } = require('../../../tests/mocks');
+
+describe('Interactors | .mergeStats', () => {
+  const baseParams = {
+    authors,
+    reviewStats,
+    pullRequestStats,
+  };
+
+  it('returns an array with all the stats for each author', async () => {
+    const results = mergeStats(baseParams);
+    expect(results.length).toEqual(authors.length);
+
+    results.forEach((result) => {
+      expect(result).toHaveProperty('author');
+      expect(result.author).toHaveProperty('login');
+      expect(result).toHaveProperty('stats');
+      VALID_STATS.forEach((stat) => {
+        expect(result.stats).toHaveProperty(stat);
+      });
+    });
+  });
+
+  it('returns all the stats for authors with data', async () => {
+    const results = mergeStats(baseParams)
+      .find(({ author }) => author.login === 'user1');
+
+    expect(results.stats).toEqual({
+      totalReviews: 4,
+      totalComments: 1,
+      timeToReview: 2052500,
+      commentsPerReview: 0.25,
+      openedPullRequests: 17,
+    });
+  });
+
+  it('returns empty stats for authors with no data', async () => {
+    const results = mergeStats(baseParams)
+      .find(({ author }) => author.login === 'user4');
+
+    expect(results.stats).toEqual({
+      timeToReview: null,
+      totalReviews: null,
+      totalComments: null,
+      commentsPerReview: null,
+      openedPullRequests: null,
+    });
+  });
+
+  it('returns empty array when no authors passed', async () => {
+    const results = mergeStats({ ...baseParams, authors: [] });
+    expect(results).toEqual([]);
+  });
+});

src/interactors/getPullRequestStats/__tests__/calculateReviewsStats.test.js

@@ -0,0 +1,10 @@
+const { reviews: input } = require('../../../../tests/mocks');
+const calculatePullRequestStats = require('../calculatePullRequestStats');
+
+describe('Interactors | getPullRequestStats | .calculatePullRequestStats', () => {
+  const result = calculatePullRequestStats(input);
+
+  it('calculates the openedPullRequests', () => {
+    expect(result.openedPullRequests).toBe(3);
+  });
+});

src/interactors/getPullRequestStats/__tests__/groupPullRequests.test.js

@@ -0,0 +1,28 @@
+const { pullRequests: input } = require('../../../../tests/mocks');
+const groupPullRequests = require('../groupPullRequests');
+
+const getPRsByAuthorId = (data, authorId) => {
+  const { pullRequests } = data.find((review) => review.authorId === authorId);
+  return pullRequests.map(({ id }) => id);
+};
+
+describe('Interactors | getPullRequestStats | .groupPullRequests', () => {
+  it('groups pull requests by author', () => {
+    const result = groupPullRequests(input);
+    expect(result.length).toEqual(2);
+    const authorIds = result.map((pr) => pr.authorId);
+    expect(authorIds).toContain('1031639', '2009676');
+    expect(getPRsByAuthorId(result, '1031639')).toEqual([12345]);
+    expect(getPRsByAuthorId(result, '2009676').sort()).toEqual([12346].sort());
+  });
+
+  it('keeps only the required properties', () => {
+    const result = groupPullRequests(input);
+    result.forEach(({ pullRequests }) => pullRequests.forEach((pullRequest) => {
+      expect(pullRequest).toHaveProperty('id');
+      expect(pullRequest).toHaveProperty('submittedAt');
+      expect(pullRequest).not.toHaveProperty('cursor');
+      expect(pullRequest).not.toHaveProperty('reviews');
+    }));
+  });
+});

src/interactors/getPullRequestStats/__tests__/index.test.js

@@ -0,0 +1,19 @@
+const { pullRequests: input } = require('../../../../tests/mocks');
+const getPullRequestStats = require('../index');
+
+const getAuthorIds = (reviewers) => reviewers.map((r) => r.authorId);
+
+describe('Interactors | getPullRequestStats', () => {
+  it('groups pull requests by author and calculate its stats', () => {
+    const result = getPullRequestStats(input);
+    expect(result.length).toEqual(2);
+    expect(getAuthorIds(result)).toContain('1031639', '8755542');
+
+    result.forEach((author) => {
+      expect(author).toHaveProperty('authorId');
+
+      expect(author).toHaveProperty('stats');
+      expect(author.stats).toHaveProperty('openedPullRequests');
+    });
+  });
+});

src/interactors/getPullRequestStats/calculatePullRequestStats.js

@@ -0,0 +1,7 @@
+module.exports = (pulls) => {
+  const openedPullRequests = pulls.length;
+
+  return {
+    openedPullRequests,
+  };
+};

src/interactors/getPullRequestStats/groupPullRequests.js

@@ -0,0 +1,15 @@
+module.exports = (pulls) => {
+  const byAuthor = pulls.reduce((acc, pull) => {
+    const authorId = pull.author.id;
+
+    if (!acc[authorId]) acc[authorId] = { authorId, pullRequests: [] };
+
+    acc[authorId].pullRequests.push({
+      id: pull.id,
+      submittedAt: pull.submittedAt,
+    });
+    return acc;
+  }, {});
+
+  return Object.values(byAuthor);
+};

src/interactors/getPullRequestStats/index.js

@@ -0,0 +1,8 @@
+const calculatePullRequestStats = require('./calculatePullRequestStats');
+const groupPullRequests = require('./groupPullRequests');
+
+module.exports = (pulls) => groupPullRequests(pulls)
+  .map(({ authorId, pullRequests }) => {
+    const stats = calculatePullRequestStats(pullRequests);
+    return { authorId, pullRequests, stats };
+  });

src/interactors/getReviewers/__tests__/calculateReviewsStats.test.js → src/interactors/getReviewStats/__tests__/calculateReviewsStats.test.js

@@ -1,7 +1,7 @@
-const input = require('./mocks/reviews');
+const { reviews: input } = require('../../../../tests/mocks');
 const calculateReviewsStats = require('../calculateReviewsStats');
 
-describe('Interactors | getReviewers | .calculateReviewsStats', () => {
+describe('Interactors | getReviewStats | .calculateReviewsStats', () => {
   const result = calculateReviewsStats(input);
 
   it('calculates the totalReviews', () => {

src/interactors/getReviewers/__tests__/groupReviews.test.js → src/interactors/getReviewStats/__tests__/groupReviews.test.js

@@ -1,24 +1,24 @@
-const input = require('./mocks/pullRequests');
+const { pullRequests: input } = require('../../../../tests/mocks');
 const groupReviews = require('../groupReviews');
 
-const getReviewsByAuthor = (data, login) => {
-  const { reviews } = data.find(({ author }) => author.login === login);
+const getReviewsByAuthorId = (data, authorId) => {
+  const { reviews } = data.find((review) => review.authorId === authorId);
   return reviews.map(({ id }) => id);
 };
 
-describe('Interactors | getReviewers | .groupReviews', () => {
+describe('Interactors | getReviewStats | .groupReviews', () => {
   it('groups reviews by author', () => {
     const result = groupReviews(input);
     expect(result.length).toEqual(2);
-    const authors = result.map((r) => r.author.login);
-    expect(authors).toContain('manuelmhtr', 'jartmez');
-    expect(getReviewsByAuthor(result, 'manuelmhtr')).toEqual([9876]);
-    expect(getReviewsByAuthor(result, 'jartmez').sort()).toEqual([5679, 9877].sort());
+    const authorIds = result.map((r) => r.authorId);
+    expect(authorIds).toContain('1031639', '8755542');
+    expect(getReviewsByAuthorId(result, '1031639')).toEqual([9876]);
+    expect(getReviewsByAuthorId(result, '8755542').sort()).toEqual([5679, 9877].sort());
   });
 
   it('removes reviews marked as own pull request', () => {
     const result = groupReviews(input);
-    expect(getReviewsByAuthor(result, 'manuelmhtr')).not.toContain([5678]);
+    expect(getReviewsByAuthorId(result, '1031639')).not.toContain([5678]);
   });
 
   it('keeps only the required properties', () => {

src/interactors/getReviewStats/__tests__/index.test.js

@@ -0,0 +1,22 @@
+const { pullRequests: input } = require('../../../../tests/mocks');
+const getReviewers = require('../index');
+
+const getAuthorIds = (reviewers) => reviewers.map((r) => r.authorId);
+
+describe('Interactors | getReviewStats', () => {
+  it('groups reviews by author and calculate its stats', () => {
+    const result = getReviewers(input);
+    expect(result.length).toEqual(2);
+    expect(getAuthorIds(result)).toContain('1031639', '8755542');
+
+    result.forEach((reviewer) => {
+      expect(reviewer).toHaveProperty('authorId');
+
+      expect(reviewer).toHaveProperty('reviews');
+      expect(reviewer.reviews.length > 0).toBe(true);
+
+      expect(reviewer).toHaveProperty('stats');
+      expect(reviewer.stats).toHaveProperty('timeToReview');
+    });
+  });
+});

src/interactors/getReviewers/groupReviews.js → src/interactors/getReviewStats/groupReviews.js

@@ -13,11 +13,11 @@ module.exports = (pulls) => {
 
   const byAuthor = all.reduce((acc, review) => {
     const { author, isOwnPull, ...other } = review;
-    const key = author.id;
+    const authorId = author.id;
 
-    if (!acc[key]) acc[key] = { author, reviews: [] };
+    if (!acc[authorId]) acc[authorId] = { authorId, reviews: [] };
 
-    acc[key].reviews.push(other);
+    acc[authorId].reviews.push(other);
     return acc;
   }, {});
 

src/interactors/getReviewStats/index.js

@@ -0,0 +1,8 @@
+const calculateReviewsStats = require('./calculateReviewsStats');
+const groupReviews = require('./groupReviews');
+
+module.exports = (pulls) => groupReviews(pulls)
+  .map(({ authorId, reviews }) => {
+    const stats = calculateReviewsStats(reviews);
+    return { authorId, reviews, stats };
+  });

src/interactors/getReviewers/__tests__/filterReviewer.test.js → src/interactors/getUsers/__tests__/filterUser.test.js

@@ -1,6 +1,6 @@
-const filterReviewer = require('../filterReviewer');
+const filterUser = require('../filterUser');
 
-describe('Interactors | getReviewers | .filterReviewer', () => {
+describe('Interactors | getUsers | .filterUser', () => {
   const reviewers = [
     'manuelmhtr',
     'jartmez',
@@ -10,7 +10,7 @@ describe('Interactors | getReviewers | .filterReviewer', () => {
 
   it('filters out reviewers by a list of usernames', () => {
     const exclude = ['manuelmhtr', 'jartmez'];
-    const results = reviewers.filter((reviewer) => filterReviewer(exclude, reviewer));
+    const results = reviewers.filter((reviewer) => filterUser(exclude, reviewer));
     expect(results.length).toEqual(2);
     expect(results).toEqual([
       'bot1',
@@ -20,7 +20,7 @@ describe('Interactors | getReviewers | .filterReviewer', () => {
 
   it('filters out reviewers by a regular expression', () => {
     const exclude = /bot/;
-    const results = reviewers.filter((reviewer) => filterReviewer(exclude, reviewer));
+    const results = reviewers.filter((reviewer) => filterUser(exclude, reviewer));
     expect(results.length).toEqual(2);
     expect(results).toEqual([
       'manuelmhtr',