[O2B-1194] Refactor export functionality #1462
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1462 +/- ##
==========================================
- Coverage 48.78% 45.20% -3.58%
==========================================
Files 528 768 +240
Lines 7968 12452 +4484
Branches 1477 2220 +743
==========================================
+ Hits 3887 5629 +1742
- Misses 4081 6823 +2742 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
I would suggest a bit of rework on top of my comments:
- I propose to extract something like
ExportConfigurationModelto store the configuration of the export, like selected columns and export type - I would create two classes (and maybe a common interface, but not sure it makes sense without ts): something like
CsvExportGeneratorandJsonExportGeneratorthat both expose anexportfunction that takes as parameter a list of items, a list of properties, a file name and generate the corresponding file.
| @@ -51,6 +58,25 @@ export class OverviewPageModel extends Observable { | |||
|
|
|||
| this._observableItems = ObservableData.builder().initialValue(RemoteData.loading()).build(); | |||
| this._observableItems.bubbleTo(this); | |||
|
|
|||
| this._exportDataSource = new PaginatedRemoteDataSource(); | |||
There was a problem hiding this comment.
Exports are not paginated
| } | ||
|
|
||
| /** | ||
| * Get the field values that will be exported |
There was a problem hiding this comment.
I think this comment is misleading, because it seems like you return the value of the fields.
I would rather use Get the list of properties that will be exported for each items
| } | ||
|
|
||
| /** | ||
| * Set the selected fields to be exported |
There was a problem hiding this comment.
Set the list of properties that will be exported for each items
| @@ -136,8 +251,30 @@ export class OverviewPageModel extends Observable { | |||
| } | |||
|
|
|||
| /** | |||
| * Return the current items remote data | |||
| * States if the list of NOT paginated runs contains the full list of runs available under the given criteria | |||
| */ | ||
| get areExportItemsTruncated() { | ||
| return this.exportItems.match({ | ||
| Success: (payload) => this.items.match({ |
There was a problem hiding this comment.
Why do you need this second match?
| @@ -0,0 +1,143 @@ | |||
| /** | |||
There was a problem hiding this comment.
I would go further and move this away from Runs
I have a JIRA ticket
Notable changes for users:
Notable changes for developers:
OverviewPageModelexportTriggerAndModalto work with another overview viewsChanges made to the database: