Refactor feature page chart methods #1031
Conversation
There was a problem hiding this comment.
Thanks so much for this Daniel!!
Added comments for/to:
- Remove runtime conversions to any
- Ensure we still have the fix from Fix: Feature usage chart pagination #1013
- Other nits
frontend/src/static/js/components/test/webstatus-feature-page.test.ts
Outdated
Show resolved
Hide resolved
frontend/src/static/js/components/test/webstatus-feature-page.test.ts
Outdated
Show resolved
Hide resolved
frontend/src/static/js/components/test/webstatus-feature-page.test.ts
Outdated
Show resolved
Hide resolved
frontend/src/static/js/components/test/webstatus-feature-page.test.ts
Outdated
Show resolved
Hide resolved
|
Also: The playwright test is failing because it is showing only the last page of results.
This is fixed in #1013 but your current changes undo that (I think from a git conflict resolution). Once you revert, it should pass. |
|
@jcscottiii You might want to check this again and view the changes on the last commit 70e0599 - I was not able to find a way to ensure we can use the |
There was a problem hiding this comment.
I see why pageComponent was needed to fix the playwright test. But we could leverage the same thing we do for LoadingTaskType.
type LoadingTaskType = '_loadingMetricsTask' | '_loadingUsageTask';
+type FetchTaskType = '_fetchFeatureSupportData' | '_fetchFeatureUsageData';Then we can modify _startDataFetchingTask like the following
- private async _startDataFetchingTask<T extends LoadingTaskType>(
- manualRun: boolean,
- dataFetcher: (
- apiClient: APIClient,
- featureId: string,
- startDate: Date,
- endDate: Date,
- ) => Promise<void>,
- taskType: T,
- ) {
+ private async _startDataFetchingTask<
+ T extends LoadingTaskType,
+ F extends FetchTaskType,
+ >(manualRun: boolean, dataFetcher: F, taskType: T) {
...
- await dataFetcher(apiClient, featureId, this.startDate, this.endDate);
+ await this[dataFetcher](
+ apiClient,
+ featureId,
+ this.startDate,
+ this.endDate,
+ );
...And finally the two tasks would like this:
export class FeaturePage extends LitElement {
private async _startFeatureSupportTask(manualRun: boolean) {
await this._startDataFetchingTask(
manualRun,
- this._fetchFeatureSupportData,
+ '_fetchFeatureSupportData',
'_loadingMetricsTask',
);
}
export class FeaturePage extends LitElement {
private async _startFeatureUsageTask(manualRun: boolean) {
await this._startDataFetchingTask(
manualRun,
- this._fetchFeatureUsageData,
+ '_fetchFeatureUsageData',
'_loadingUsageTask',
);
}
DanielRyanSmith
force-pushed
the
964_refactor-chart-methods
branch
from
0fdb4e5 to
5b0cc78
Compare
Fixes #964
This change refactors some methods for generating the data used to populate the feature support and usage charts to maximize reusability, as well as adding new unit tests.