Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tracking system, number of content types and components per user/project #14900

Merged
merged 7 commits into from
Feb 1, 2023

Conversation

christiancp100
Copy link
Contributor

What does it do?

It tracks the number of content types, the number of components, and the number of Dynamic Zones that a project has

Why is it needed?

To get more insights about what the final user uses.

How to test it?

On Amplitude

Related issue(s)/PR(s)

@codecov
Copy link

codecov bot commented Nov 16, 2022

Codecov Report

Base: 59.12% // Head: 47.61% // Decreases project coverage by -11.52% ⚠️

Coverage data is based on head (c557588) compared to base (786e476).
Patch coverage: 50.00% of modified lines in pull request are covered.

Additional details and impacted files
@@             Coverage Diff             @@
##             main   #14900       +/-   ##
===========================================
- Coverage   59.12%   47.61%   -11.52%     
===========================================
  Files        1502      448     -1054     
  Lines       38340    15756    -22584     
  Branches     7383     3371     -4012     
===========================================
- Hits        22669     7502    -15167     
+ Misses      13401     6849     -6552     
+ Partials     2270     1405      -865     
Flag Coverage Δ
back 47.39% <50.00%> (+<0.01%) ⬆️
front ?
unit_back 47.39% <50.00%> (+<0.01%) ⬆️
unit_front ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
packages/plugins/i18n/server/services/metrics.js 100.00% <ø> (ø)
packages/core/admin/server/controllers/admin.js 35.86% <20.00%> (-0.92%) ⬇️
...ackages/core/strapi/lib/services/metrics/sender.js 100.00% <100.00%> (ø)
...in/src/content-manager/components/Wysiwyg/index.js
packages/core/upload/admin/src/hooks/useUpload.js
...gsPage/pages/Users/components/SelectRoles/index.js
...re/admin/admin/src/components/GlobalStyle/index.js
packages/core/upload/admin/src/index.js
...anager/pages/ListView/utils/createPluginsFilter.js
.../core/admin/admin/src/components/LeftMenu/index.js
... and 1047 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Contributor

@innerdvations innerdvations left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but we'll wait on someone who knows more about how to tracking system works (and can check in Amplitude) in case there's an issue I'm missing.

@christiancp100 christiancp100 added the pr: enhancement This PR adds or updates some part of the codebase or features label Dec 13, 2022
Copy link
Member

@Convly Convly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe you can simplify your sum operation (or at least make it more readable) by chaining fp operations. Here's an example

const countDynamicZoneAttributes = pipe(
  map('attributes'),
  flatMap(values),
  sumBy(propEq('type', 'dynamiczone'))
);

@Convly Convly added the source: core:strapi Source is core/strapi package label Dec 28, 2022
@Convly Convly added this to the 4.5.6 milestone Dec 28, 2022
@Convly Convly modified the milestones: 4.5.6, 4.6.0 Jan 11, 2023
@Convly Convly modified the milestones: 4.6.0, 4.6.1 Jan 25, 2023
@Convly Convly added the flag: don't merge This PR should not be merged at the moment label Jan 25, 2023
return {
data: {
useTypescriptOnServer,
useTypescriptOnAdmin,
isHostedOnStrapiCloud,
numberOfAllContentTypes, // TODO: Rename this in Strapi v5
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you mind adding a bit more context in case someone else should handle this in V5?
Maybe a short sentence about what it should be renamed to & why?

example:
(it could probably be phrased better though)
// TODO: V5: This event should be renamed numberOfContentTypes in V5 as the name is already taken to describe the number of content types using i18n.

Comment on lines 104 to 109
const getNumberOfDynamicZones = () =>
pipe(
map('attributes'),
flatMap(values),
sumBy(propEq('type', 'dynamiczone'))
)(strapi.contentTypes);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be a matter of personal preference (feel free to ignore), but I would suggest that for arrow functions of more than one line, consider using {} and a return statement to better delimit the boundaries of the function 🙂

Suggested change
const getNumberOfDynamicZones = () =>
pipe(
map('attributes'),
flatMap(values),
sumBy(propEq('type', 'dynamiczone'))
)(strapi.contentTypes);
const getNumberOfDynamicZones = () => {
return pipe(
map('attributes'),
flatMap(values),
sumBy(propEq('type', 'dynamiczone'))
)(strapi.contentTypes)
};

@@ -60,6 +68,9 @@ module.exports = (strapi) => {
useTypescriptOnAdmin: isUsingTypeScriptSync(adminRootPath),
projectId: uuid,
isHostedOnStrapiCloud: env('STRAPI_HOSTING', null) === 'strapi.cloud',
numberOfAllContentTypes: _.size(strapi.contentTypes), // TODO: Rename this in Strapi v5
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above 🙂

@@ -42,6 +43,13 @@ module.exports = (strapi) => {
const serverRootPath = strapi.dirs.app.root;
const adminRootPath = path.join(strapi.dirs.app.root, 'src', 'admin');

const getNumberOfDynamicZones = () =>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above 🙂

@@ -6,6 +6,7 @@ const { getService } = require('../utils');
const sendDidInitializeEvent = async () => {
const { isLocalizedContentType } = getService('content-types');

// TODO: Rename this in Strapi v5
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙂

@christiancp100 christiancp100 removed the flag: don't merge This PR should not be merged at the moment label Feb 1, 2023
@christiancp100 christiancp100 merged commit b71eccf into main Feb 1, 2023
@alexandrebodin alexandrebodin deleted the tracking-system-ct-components-dz branch February 27, 2023 20:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: enhancement This PR adds or updates some part of the codebase or features source: core:strapi Source is core/strapi package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants