Skip to content

Commit

Permalink
Feature: Refactor message builder on Post to Slack interactor
Browse files Browse the repository at this point in the history
  • Loading branch information
manuelmhtr committed Dec 5, 2024
1 parent 96bf29c commit 1951d35
Show file tree
Hide file tree
Showing 18 changed files with 409 additions and 243 deletions.
25 changes: 13 additions & 12 deletions src/interactors/__tests__/mergeStats.test.js
Original file line number Diff line number Diff line change
@@ -1,31 +1,32 @@
const mergeStats = require('../mergeStats');
const { VALID_STATS } = require('../../config/stats');
const { authors, reviewStats, pullRequestStats } = require('../../../tests/mocks');
const { users, reviewStats, pullRequestStats } = require('../../../tests/mocks');

describe('Interactors | .mergeStats', () => {
const baseParams = {
authors,
users,
reviewStats,
pullRequestStats,
};

it('returns an array with all the stats for each author', async () => {
it('returns an array with all the stats for each user', async () => {
const results = mergeStats(baseParams);
expect(results.length).toEqual(authors.length);
expect(results.length).toEqual(users.length);

results.forEach((result) => {
expect(result).toHaveProperty('author');
expect(result.author).toHaveProperty('login');
expect(result).toHaveProperty('user');
expect(result.user).toHaveProperty('login');
expect(result).toHaveProperty('reviews');
expect(result).toHaveProperty('stats');
VALID_STATS.forEach((stat) => {
expect(result.stats).toHaveProperty(stat);
});
});
});

it('returns all the stats for authors with data', async () => {
it('returns all the stats for users with data', async () => {
const results = mergeStats(baseParams)
.find(({ author }) => author.login === 'user1');
.find(({ user }) => user.login === 'user1');

expect(results.stats).toEqual({
totalReviews: 4,
Expand All @@ -36,9 +37,9 @@ describe('Interactors | .mergeStats', () => {
});
});

it('returns empty stats for authors with no data', async () => {
it('returns empty stats for users with no data', async () => {
const results = mergeStats(baseParams)
.find(({ author }) => author.login === 'user4');
.find(({ user }) => user.login === 'user4');

expect(results.stats).toEqual({
timeToReview: null,
Expand All @@ -49,8 +50,8 @@ describe('Interactors | .mergeStats', () => {
});
});

it('returns empty array when no authors passed', async () => {
const results = mergeStats({ ...baseParams, authors: [] });
it('returns empty array when no users passed', async () => {
const results = mergeStats({ ...baseParams, users: [] });
expect(results).toEqual([]);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
const { entries } = require('../../../../tests/mocks');
const buildReviewTimeLink = require('../buildReviewTimeLink');

const [entry] = entries;

const SUCCESSFUL_LINK = "https://app.flowwer.dev/charts/review-time/~(u~(i~'user1~n~'user1)~p~30~r~(~(d~'qprzn9~t~'84)~(d~'qpvagu~t~'a3)~(d~'qpvn25~t~'3lu)~(d~'qqqtu5~t~'2vy)))";

const EMPTY_LINK = "https://app.flowwer.dev/charts/review-time/~(u~(i~'user1~n~'user1)~p~30~r~(~))";

const MAX_LENGTH = 1024;

const buildReview = (submittedAt) => ({
timeToReview: 1000,
submittedAt: new Date(submittedAt).toISOString(),
});

describe('Interactors | .buildTable | .buildReviewTimeLink', () => {
const period = 30;

it('builds the link correctly', () => {
const response = buildReviewTimeLink(entry, period);

expect(response).toEqual(SUCCESSFUL_LINK);
});

it('builds a link event with empty reviews', () => {
const emptyReviewer = { ...entry, reviews: null };
const response = buildReviewTimeLink(emptyReviewer, period);

expect(response).toEqual(EMPTY_LINK);
});

it('limits the url to less than 1,024 characters', () => {
const reviews = Array(100).fill().map((_e, index) => buildReview(index * 1000));
const response = buildReviewTimeLink({ ...entry, reviews }, period);
expect(response.length <= MAX_LENGTH).toEqual(true);
expect(MAX_LENGTH - response.length < 16).toEqual(true);
});
});
37 changes: 37 additions & 0 deletions src/interactors/fulfillEntries/__tests__/calculateTotals.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
const stats = require('../../__tests__/mocks/stats.json');
const statsSum = require('../../__tests__/mocks/statsSum.json');
const calculateTotals = require('../calculateTotals');

describe('Interactors | .buildTable | .calculateTotals', () => {
it('sums all the stats in a array', () => {
const response = calculateTotals(stats);
expect(response).toEqual(statsSum);
});

it('returns the correct sum event when data contains nulls', () => {
const withNulls = {
totalReviews: undefined,
totalComments: null,
commentsPerReview: null,
timeToReview: 0,
};
const response = calculateTotals([...stats, withNulls]);
expect(response).toEqual(statsSum);
});

it('returns the correct sum event when data contains an empty object', () => {
const empty = {};
const response = calculateTotals([...stats, empty]);
expect(response).toEqual(statsSum);
});

it('returns all stats in zeros when receiving an empty one', () => {
const response = calculateTotals([]);
expect(response).toEqual({
totalReviews: 0,
totalComments: 0,
commentsPerReview: 0,
timeToReview: 0,
});
});
});
16 changes: 16 additions & 0 deletions src/interactors/fulfillEntries/__tests__/getContributions.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
const statsSum = require('../../__tests__/mocks/statsSum.json');
const reviewers = require('../../__tests__/mocks/reviewers.json');
const getContributions = require('../getContributions');

const [reviewer] = reviewers;

describe('Interactors | .buildTable | .getContributions', () => {
it('adds the percentage of each stat vs the total', () => {
const response = getContributions(reviewer, statsSum);
expect(response).toMatchObject({
commentsPerReview: 0.004222972972972973,
totalComments: 0.008695652173913044,
totalReviews: 0.8695652173913043,
});
});
});
25 changes: 25 additions & 0 deletions src/interactors/fulfillEntries/__tests__/index.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
const { entries } = require('../../../../tests/mocks');
const fulfillEntries = require('../index');

describe('Interactors | .fulfillEntries', () => {
const periodLength = 30;

it('adds contributions to each reviewer', () => {
const response = fulfillEntries(entries, { periodLength });
expect(response.length).toEqual(entries.length);

response.forEach((reviewer) => {
expect(reviewer).toHaveProperty('contributions');
});
});

it('adds urls to each reviewer', () => {
const response = fulfillEntries(entries, { periodLength });
expect(response.length).toEqual(entries.length);

response.forEach((reviewer) => {
expect(reviewer).toHaveProperty('urls');
expect(reviewer.urls).toHaveProperty('timeToReview');
});
});
});
49 changes: 49 additions & 0 deletions src/interactors/fulfillEntries/buildReviewTimeLink.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
const JSURL = require('jsurl');

const URL = 'https://app.flowwer.dev/charts/review-time/';
const MAX_URI_LENGTH = 1024;
const CHARS_PER_REVIEW = 16;

const toSeconds = (ms) => Math.round(ms / 1000);

const compressInt = (int) => int.toString(36);

const compressDate = (date) => compressInt(Math.round(date.getTime() / 1000));

const parseReview = ({ submittedAt, timeToReview }) => ({
d: compressDate(submittedAt),
t: compressInt(toSeconds(timeToReview)),
});

const buildUri = ({ user, period, reviews }) => {
const data = JSURL.stringify({
u: {
i: `${user.id}`,
n: user.login,
},
p: period,
r: reviews,
});

const uri = `${URL}${data}`;
const exceededLength = uri.length - MAX_URI_LENGTH;
if (exceededLength <= 0) return uri;

// Remove at least one, but trying to guess exactly how many to remove.
const reviewsToRemove = Math.max(1, Math.ceil(exceededLength / CHARS_PER_REVIEW));
return buildUri({ user, period, reviews: reviews.slice(reviewsToRemove) });
};

module.exports = (entry, period) => {
const { user, reviews } = entry || {};
const parsedReviews = (reviews || [])
.map((r) => ({ ...r, submittedAt: new Date(r.submittedAt) }))
.sort((a, b) => a.submittedAt - b.submittedAt)
.map(parseReview);

return buildUri({
user,
period,
reviews: parsedReviews,
});
};
10 changes: 10 additions & 0 deletions src/interactors/fulfillEntries/calculateTotals.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
const { STATS } = require('../../constants');

const sumStat = (stats, statName) => stats.reduce((a, values) => a + (values[statName] || 0), 0);

const calculateTotals = (allStats) => STATS.reduce((prev, statName) => ({
...prev,
[statName]: sumStat(allStats, statName),
}), {});

module.exports = calculateTotals;
13 changes: 13 additions & 0 deletions src/interactors/fulfillEntries/getContributions.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
const { STATS } = require('../../constants');

const calculatePercentage = (value, total) => {
if (!total) return 0;
return Math.min(1, Math.max(0, value / total));
};

const getContributions = (reviewer, totals) => STATS.reduce((prev, statsName) => {
const percentage = calculatePercentage(reviewer.stats[statsName], totals[statsName]);
return { ...prev, [statsName]: percentage };
}, {});

module.exports = getContributions;
18 changes: 18 additions & 0 deletions src/interactors/fulfillEntries/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
const buildReviewTimeLink = require('./buildReviewTimeLink');
const getContributions = require('./getContributions');
const calculateTotals = require('./calculateTotals');

const getUrls = ({ entry, periodLength }) => ({
timeToReview: buildReviewTimeLink(entry, periodLength),
});

module.exports = (entries, { periodLength }) => {
const allStats = entries.map(({ stats }) => stats);
const totals = calculateTotals(allStats);

return entries.map((entry) => ({
...entry,
contributions: getContributions(entry, totals),
urls: getUrls({ entry, periodLength }),
}));
};
19 changes: 10 additions & 9 deletions src/interactors/mergeStats.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,26 +4,27 @@ const EMPTY_STATS = VALID_STATS
.reduce((acc, stat) => ({ ...acc, [stat]: null }), {});

module.exports = ({
authors,
users,
reviewStats,
pullRequestStats,
}) => {
const reviewStatsByAuthorId = reviewStats.reduce((acc, reviewStat) => ({
const reviewsByUserId = reviewStats.reduce((acc, reviewsData) => ({
...acc,
[reviewStat.authorId]: reviewStat.stats,
[reviewsData.userId]: reviewsData,
}), {});

const prStatsByAuthorId = pullRequestStats.reduce((acc, prStat) => ({
const pullRequestsByUserId = pullRequestStats.reduce((acc, prsData) => ({
...acc,
[prStat.authorId]: prStat.stats,
[prsData.userId]: prsData,
}), {});

return authors.map((author) => ({
author,
return users.map((user) => ({
user,
reviews: reviewsByUserId[user.id]?.reviews || [],
stats: {
...EMPTY_STATS,
...(reviewStatsByAuthorId[author.id] || {}),
...(prStatsByAuthorId[author.id] || {}),
...(reviewsByUserId[user.id]?.stats || {}),
...(pullRequestsByUserId[user.id]?.stats || {}),
},
}));
};
8 changes: 2 additions & 6 deletions src/interactors/postSlackMessage/__tests__/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,9 @@ describe('Interactors | .postSlackMessage', () => {
const defaultOptions = {
core,
isSponsor: true,
reviewers: 'REVIEWERS',
table: 'TABLE',
pullRequest: 'PULl REQUEST',
periodLength: 'PERIOD LENGTH',
disableLinks: 'DISPLAY LINKS',
displayCharts: 'DISPLAY CHARTS',
slack: {
webhook: 'https://slack.com/webhook',
channel: '#my-channel',
Expand Down Expand Up @@ -81,11 +79,9 @@ describe('Interactors | .postSlackMessage', () => {
await postSlackMessage({ ...defaultOptions });
expect(error).not.toHaveBeenCalled();
expect(buildMessage).toBeCalledWith({
reviewers: defaultOptions.reviewers,
table: defaultOptions.table,
pullRequest: defaultOptions.pullRequest,
periodLength: defaultOptions.periodLength,
disableLinks: defaultOptions.disableLinks,
displayCharts: defaultOptions.displayCharts,
});
expect(Fetchers.postToSlack).toBeCalledTimes(1);
expect(Fetchers.postToSlack).toBeCalledWith({
Expand Down
Loading

0 comments on commit 1951d35

Please sign in to comment.