feat: #174 As a 'Developer', I want to see a graph on my analytics page showing API calls #221
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Update market place api schema - Add AppUsageStats component
- Update market place api schema - Add AppUsageStats component
nphivu414
requested review from
willmcvay,
trankhacvy,
duong-se,
vuhuucuong,
phmngocnghia and
dannd4
willmcvay
approved these changes
Feb 10, 2020
| @@ -188,7 +198,7 @@ export const AnalyticsPage: React.FC<AnalyticsPageProps> = ({ installations, dev | |||
| handleMapAppNameToInstallation(installationAppDataArray, developerDataArray), | |||
| [installationAppDataArray, developerDataArray], | |||
| ) | |||
|
|
|||
| console.log('appUsageStats', appUsageStats) | |||
This was
linked to
issues
Feb 11, 2020
vuhuucuong
reviewed
Feb 11, 2020
Comment on lines
29
to
34
| ...{ | ||
| [developerAppId]: { | ||
| appName: developerAppName, | ||
| requests, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
will this work without ...{} ?
[developerAppId]: {
appName: developerAppName,
requests,
},
There was a problem hiding this comment.
It wont work without the spread operator here because I need to merge 2 objects into 1.
For example:
a = {
appId1: {
appName: 'App 1',
request: 5
}
}
b = {
appId2: {
appName: 'App 2',
request: 10
}
}
Expected result:
{
appId1: {
appName: 'App 1',
request: 5
},
appId2: {
appName: 'App 2',
request: 10
}
}
Comment on lines
70
to
73
| return Object.values(appUsage) | ||
| .filter((app: any) => app.appName) | ||
| .map((app: any) => { | ||
| return `${app.appName}: ${app.requests}` |
There was a problem hiding this comment.
it's nice if you can replace any with a correct type
| const { appId, usage } = currentValue || {} | ||
| const developerApp = developerApps?.find(app => app.id === appId) | ||
| if (developerApp && developerApp.id) { | ||
| const { id: developerAppId, name: developerAppName } = developerApp |
There was a problem hiding this comment.
Could this be changed to
if (!developerApp?.id){|
return accumulator
}
const { id: developerAppId, name: developerAppName } = developerApp
...
| } | ||
|
|
||
| describe('DeveloperTrafficChart', () => { | ||
| describe('getAppUsageStatsChartData', () => { |
There was a problem hiding this comment.
make sure to test this function with more cases since it looks quite complicated
There was a problem hiding this comment.
Hi @vuhuucuong , i've already added another test case for multiple app usages
vuhuucuong
approved these changes
Feb 11, 2020
nphivu414
deleted the
feat/174-add-a-graph-showing-api-calls-in-analytics-page
branch
nphivu414
pushed a commit
that referenced
this pull request
Apr 29, 2020
* [CLD-598] Searching by app name or company name on browse apps * Change auze-pipelines
nphivu414
pushed a commit
that referenced
this pull request
Apr 29, 2020
…-component [CLD-696] change style help guide component
nphivu414
added a commit
that referenced
this pull request
Apr 29, 2020
…ge showing API calls (#221) * feat: #174 - Add AppUsageStats apis + redux store - Update market place api schema - Add AppUsageStats component * feat: #174 - Add AppUsageStats apis + redux store - Update market place api schema - Add AppUsageStats component * feat: #173 create traffic table * feat: #174 - Build app usages graph * Revert package.json * feat: #174 - Update test for developer traffic chart * feat: #174 - Add util test, fix created date format in traffic table * feat: #174 - Remove developerId param from appUsageStatsRequest * feat: #174 Remove unused logs - Update appId params * feat: #174 - Update usage stats test for multiple apps * feat: #174 - Fix app usage stats test issues * feat: #174 - Update analytic page rendering flow while loading * feat: #174 - Add type for app trafic tooltip label * feat: #174 - Update marketplace-api-schema.ts file
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.