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

feat: applicaton usage endpoint #4548

Merged
merged 5 commits into from
Aug 23, 2023
Merged

feat: applicaton usage endpoint #4548

merged 5 commits into from
Aug 23, 2023

Conversation

sjaanus
Copy link
Contributor

@sjaanus sjaanus commented Aug 23, 2023

No description provided.

@vercel
Copy link

vercel bot commented Aug 23, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
unleash-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 23, 2023 8:50am
unleash-monorepo-frontend ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 23, 2023 8:50am

});

const reduceRows = (rows: any[]): IClientApplication[] => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since we are joining usage table, there are duplicate entries, which we are reducing into usage object.

const existingApp = acc[row.app_name];

if (existingApp) {
let project = existingApp.usage.find(
Copy link
Contributor

Choose a reason for hiding this comment

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

could be const

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Comment on lines 69 to 70
project: joi.string().allow('').optional(),
environment: joi.string().allow('').optional(),
Copy link
Contributor

Choose a reason for hiding this comment

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

Is joi still in use? I thought OpenAPI was handling validation now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, we have around 63 usages of this kind of validation. If we are creating anything new, we go with OpenAPI, but older endpoints use it.

Copy link
Contributor

Choose a reason for hiding this comment

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

But we do have an application-schema.ts. Does this not handle the validation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is used for response, but joi is doing for request.

Comment on lines 49 to 73
(usage) => usage.project === row.project,
);

if (project) {
project.environments.push(row.environment);
} else {
existingApp.usage.push({
project: row.project,
environments: [row.environment],
});
}
} else {
const mappedRow = mapRow(row);
acc[row.app_name] = {
...mappedRow,
usage:
row.project && row.environment
? [
{
project: row.project,
environments: [row.environment],
},
]
: [],
};
Copy link
Contributor

Choose a reason for hiding this comment

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

Spent a fair bit of time looking at this function to figure out what it does. Could we simplify it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Refactored a little, but cant do much more.

Copy link
Contributor

@FredrikOseberg FredrikOseberg left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

2 participants