From f6e40a600e121ebfe64790d19cc85d877fcbcdf5 Mon Sep 17 00:00:00 2001 From: Joshua Newton Date: Wed, 6 Dec 2023 14:54:39 -0500 Subject: [PATCH] [Discourse] Update schema keys to use plural form (`topic_count` -> `topics_count`) (#9778) * Discourse: Update schema to use plural keys (e.g. topic_count -> topics_countN) * Revert "Discourse: Update schema to use plural keys" This reverts commit 4073a17aaaaa253ce9ef01754ede77ba5deaa7f5. * `discourse.service.js`: Add `Joi.alternatives` plural schema * `discourse.service.js`: Update func to be plural-agnostic Previously, for e.g. 'topic', the call to the `DiscourseMetricIntegrationFactory` function supplied both 'topics' and 'topic_count'. And, we now need to check for 'topics_count' as well. To cover all three string variations, why not supply only 'topic' to the function, then selectively add the 's' on a case-by-case basis. ((Note: I've preserved the old metricName variable as to minimize the diff here and make my changes clearer.)) * `discourse.tester.js`: Add second `data` case * Address Prettier linting warnings --- services/discourse/discourse.service.js | 31 +++-- services/discourse/discourse.tester.js | 150 ++++++++++++++---------- 2 files changed, 110 insertions(+), 71 deletions(-) diff --git a/services/discourse/discourse.service.js b/services/discourse/discourse.service.js index 210acde8ee7e9..475cb7978315f 100644 --- a/services/discourse/discourse.service.js +++ b/services/discourse/discourse.service.js @@ -4,13 +4,22 @@ import { metric } from '../text-formatters.js' import { nonNegativeInteger, optionalUrl } from '../validators.js' import { BaseJsonService } from '../index.js' -const schema = Joi.object({ +const schemaSingular = Joi.object({ topic_count: nonNegativeInteger, user_count: nonNegativeInteger, post_count: nonNegativeInteger, like_count: nonNegativeInteger, }).required() +const schemaPlural = Joi.object({ + topics_count: nonNegativeInteger, + users_count: nonNegativeInteger, + posts_count: nonNegativeInteger, + likes_count: nonNegativeInteger, +}) + +const schema = Joi.alternatives(schemaSingular, schemaPlural) + const queryParamSchema = Joi.object({ server: optionalUrl.required(), }).required() @@ -36,7 +45,10 @@ class DiscourseBase extends BaseJsonService { } } -function DiscourseMetricIntegrationFactory({ metricName, property }) { +function DiscourseMetricIntegrationFactory({ metricType }) { + // We supply the singular form to more easily check against both schemas. + // But, we use the plural form as the metric name for grammatical reasons. + const metricName = `${metricType}s` return class DiscourseMetric extends DiscourseBase { // The space is needed so we get 'DiscourseTopics' rather than // 'Discoursetopics'. `camelcase()` removes it. @@ -63,7 +75,12 @@ function DiscourseMetricIntegrationFactory({ metricName, property }) { async handle(_routeParams, { server }) { const data = await this.fetch({ server }) - return this.constructor.render({ stat: data[property] }) + // e.g. metricType == 'topic' --> try 'topic_count' then 'topics_count' + let stat = data[`${metricType}_count`] + if (stat === undefined) { + stat = data[`${metricType}s_count`] + } + return this.constructor.render({ stat }) } } } @@ -97,10 +114,10 @@ class DiscourseStatus extends DiscourseBase { } const metricIntegrations = [ - { metricName: 'topics', property: 'topic_count' }, - { metricName: 'users', property: 'user_count' }, - { metricName: 'posts', property: 'post_count' }, - { metricName: 'likes', property: 'like_count' }, + { metricType: 'topic' }, + { metricType: 'user' }, + { metricType: 'post' }, + { metricType: 'like' }, ].map(DiscourseMetricIntegrationFactory) export default [...metricIntegrations, DiscourseStatus] diff --git a/services/discourse/discourse.tester.js b/services/discourse/discourse.tester.js index bdaac3d56c572..763264a31255a 100644 --- a/services/discourse/discourse.tester.js +++ b/services/discourse/discourse.tester.js @@ -6,76 +6,98 @@ export const t = new ServiceTester({ title: 'Discourse', }) -const data = { - topic_count: 22513, - post_count: 337719, - user_count: 31220, - topics_7_days: 143, - topics_30_days: 551, - posts_7_days: 2679, - posts_30_days: 10445, - users_7_days: 204, - users_30_days: 803, - active_users_7_days: 762, - active_users_30_days: 1495, - like_count: 308833, - likes_7_days: 3633, - likes_30_days: 13397, -} +const dataCases = [ + { + // Singular form + topic_count: 22513, + post_count: 337719, + user_count: 31220, + topics_7_days: 143, + topics_30_days: 551, + posts_7_days: 2679, + posts_30_days: 10445, + users_7_days: 204, + users_30_days: 803, + active_users_7_days: 762, + active_users_30_days: 1495, + like_count: 308833, + likes_7_days: 3633, + likes_30_days: 13397, + }, + { + // Plural form + topics_count: 22513, + posts_count: 337719, + users_count: 31220, + topics_7_days: 143, + topics_30_days: 551, + posts_7_days: 2679, + posts_30_days: 10445, + users_7_days: 204, + users_30_days: 803, + active_users_7_days: 762, + active_users_30_days: 1495, + likes_count: 308833, + likes_7_days: 3633, + likes_30_days: 13397, + }, +] -t.create('Topics') - .get('/topics.json?server=https://meta.discourse.org') - .intercept(nock => - nock('https://meta.discourse.org') - .get('/site/statistics.json') - .reply(200, data), - ) - .expectBadge({ label: 'discourse', message: '23k topics' }) +dataCases.forEach(data => { + t.create('Topics') + .get('/topics.json?server=https://meta.discourse.org') + .intercept(nock => + nock('https://meta.discourse.org') + .get('/site/statistics.json') + .reply(200, data), + ) + .expectBadge({ label: 'discourse', message: '23k topics' }) -t.create('Posts') - .get('/posts.json?server=https://meta.discourse.org') - .intercept(nock => - nock('https://meta.discourse.org') - .get('/site/statistics.json') - .reply(200, data), - ) - .expectBadge({ label: 'discourse', message: '338k posts' }) + t.create('Posts') + .get('/posts.json?server=https://meta.discourse.org') + .intercept(nock => + nock('https://meta.discourse.org') + .get('/site/statistics.json') + .reply(200, data), + ) + .expectBadge({ label: 'discourse', message: '338k posts' }) -t.create('Users') - .get('/users.json?server=https://meta.discourse.org') - .intercept(nock => - nock('https://meta.discourse.org') - .get('/site/statistics.json') - .reply(200, data), - ) - .expectBadge({ label: 'discourse', message: '31k users' }) + t.create('Users') + .get('/users.json?server=https://meta.discourse.org') + .intercept(nock => + nock('https://meta.discourse.org') + .get('/site/statistics.json') + .reply(200, data), + ) + .expectBadge({ label: 'discourse', message: '31k users' }) -t.create('Likes') - .get('/likes.json?server=https://meta.discourse.org') - .intercept(nock => - nock('https://meta.discourse.org') - .get('/site/statistics.json') - .reply(200, data), - ) - .expectBadge({ label: 'discourse', message: '309k likes' }) + t.create('Likes') + .get('/likes.json?server=https://meta.discourse.org') + .intercept(nock => + nock('https://meta.discourse.org') + .get('/site/statistics.json') + .reply(200, data), + ) + .expectBadge({ label: 'discourse', message: '309k likes' }) -t.create('Status') - .get('/status.json?server=https://meta.discourse.org') - .intercept(nock => - nock('https://meta.discourse.org') - .get('/site/statistics.json') - .reply(200, data), - ) - .expectBadge({ label: 'discourse', message: 'online' }) + t.create('Status') + .get('/status.json?server=https://meta.discourse.org') + .intercept(nock => + nock('https://meta.discourse.org') + .get('/site/statistics.json') + .reply(200, data), + ) + .expectBadge({ label: 'discourse', message: 'online' }) -t.create('Status with http (not https)') - .get('/status.json?server=http://meta.discourse.org') - .intercept(nock => - nock('http://meta.discourse.org') - .get('/site/statistics.json') - .reply(200, data), - ) - .expectBadge({ label: 'discourse', message: 'online' }) + t.create('Status with http (not https)') + .get('/status.json?server=http://meta.discourse.org') + .intercept(nock => + nock('http://meta.discourse.org') + .get('/site/statistics.json') + .reply(200, data), + ) + .expectBadge({ label: 'discourse', message: 'online' }) +}) t.create('Invalid Host') .get('/status.json?server=https://some.host')