Remove class hierarchy for groups of changes with same result or configs
This simplifies the code, and lets us filter changes down to the lowest level
Change-Id: Id1d999d8fe8acecc1a7286507037ea612ca17453
Reviewed-on: https://dart-review.googlesource.com/c/dart_ci/+/118021
Reviewed-by: Jonas Termansen <sortie@google.com>
diff --git a/results_feed/lib/src/commit.dart b/results_feed/lib/src/commit.dart
index a86dcbc..527f472 100644
--- a/results_feed/lib/src/commit.dart
+++ b/results_feed/lib/src/commit.dart
@@ -92,16 +92,6 @@
/// Sort in reverse chronological order.
int compareTo(other) => (other as ChangeGroup).range.compareTo(range);
- void addChanges(Iterable<Change> newChanges) {
- newChanges.forEach(changes.add);
- computeFilteredChanges(_filter);
- }
-
- void addLatestChanges(Iterable<Change> newChanges) {
- newChanges.forEach(latestChanges.add);
- computeFilteredChanges(_filter);
- }
-
bool show(Filter filter) =>
filter.showAllCommits || filteredChanges(filter).isNotEmpty;
@@ -133,6 +123,7 @@
summaries = _summarize(configurations);
factory Configurations(List configs) {
+ if (configs.isEmpty) return null;
configs.sort();
return _instances.putIfAbsent(
configs.join(' '), () => Configurations._(configs.toList()));
@@ -216,72 +207,73 @@
String get resultStyle => result == expected ? 'success' : 'failure';
}
-class ResultGroup with IterableMixin<Change> {
- final String changesText;
- final Map<String, Change> _map = SplayTreeMap();
+class Changes with IterableMixin<List<List<Change>>> {
+ final List<List<List<Change>>> changes;
- ResultGroup(this.changesText);
+ Changes(Iterable<Change> newChanges) : this.grouped(group(newChanges));
- get iterator => _map.values.iterator;
+ Changes.grouped(List<List<List<Change>>> this.changes);
- void operator []=(Object resultText, Object change) {
- _map[resultText] = change;
- }
+ get iterator => changes.iterator;
- bool show(Filter filter) =>
- !filter.showLatestFailures || first.resultStyle == 'failure';
-}
-
-class ConfigGroup with IterableMixin<ResultGroup> {
- final Configurations configurations;
- final Map<String, ResultGroup> _map;
-
- ConfigGroup(this.configurations, {Map<String, ResultGroup> map})
- : _map = map ?? SplayTreeMap();
-
- ResultGroup operator [](String resultText) =>
- _map.putIfAbsent(resultText, () => ResultGroup(resultText));
-
- get iterator => _map.values.iterator;
-
- ConfigGroup filtered(Filter filter) {
- if (!configurations.show(filter)) {
- return ConfigGroup(configurations, map: {});
+ /// The changes, grouped first by Configurations object, then by
+ /// changesText (the change in the results). Should not be modified
+ /// after it is created by this call.
+ static List<List<List<Change>>> group(Iterable<Change> newChanges) {
+ final map = SplayTreeMap<String, SplayTreeMap<String, List<Change>>>();
+ for (final change in newChanges) {
+ map
+ .putIfAbsent(change.configurations.text,
+ () => SplayTreeMap<String, List<Change>>())
+ .putIfAbsent(change.changesText, () => List<Change>())
+ .add(change);
}
- final newMap =
- Map.fromEntries(_map.entries.where((e) => e.value.show(filter)));
- return newMap.length == _map.length
- ? this
- : ConfigGroup(configurations, map: newMap);
- }
-}
-
-class Changes with IterableMixin<ConfigGroup> {
- final Map<String, ConfigGroup> _map;
-
- Changes(Iterable<Change> changes, {Map map}) : _map = map ?? SplayTreeMap() {
- changes.forEach(add);
+ return [
+ for (final configuration in map.keys)
+ [
+ for (final change in map[configuration].keys)
+ map[configuration][change]
+ ]
+ ];
}
- get iterator => _map.values.iterator;
-
- ConfigGroup operator [](Configurations configuration) =>
- _map.putIfAbsent(configuration.text, () => ConfigGroup(configuration));
-
- void add(Change change) {
- this[change.configurations][change.changesText][change.name] = change;
- }
+ static bool empty(x, f) => x.isEmpty;
Changes filtered(Filter filter) {
- final newMap = <String, ConfigGroup>{};
+ final newChanges =
+ applyFilter<List<List<Change>>>(configurationFilter, changes, filter);
+ return newChanges == changes ? this : Changes.grouped(newChanges);
+ }
+
+ /// Process a list by applying elementFilter to its elements, then throw
+ /// out empty or otherwise filtered elements with the emptyTest filter.
+ /// If no elements are modified or dropped, returns the input list object.
+ static applyFilter<T>(
+ T elementFilter(T t, Filter f), List<T> list, Filter filter,
+ [bool emptyTest(dynamic, Filter) = empty]) {
+ final newList = <T>[];
bool changed = false;
- for (final key in _map.keys) {
- final newValue = _map[key].filtered(filter);
- changed = changed || newValue != _map[key];
- if (!newValue.isEmpty) {
- newMap[key] = newValue;
- }
+ for (final element in list) {
+ final T newElement = elementFilter(element, filter);
+ changed = changed || newElement != element;
+ if (!emptyTest(newElement, filter)) newList.add(newElement);
}
- return changed ? Changes([], map: newMap) : this;
+ return changed ? newList : list;
+ }
+
+ List<List<Change>> configurationFilter(
+ List<List<Change>> list, Filter filter) {
+ if (list.first.first.configurations.show(filter)) {
+ return applyFilter<List<Change>>(resultFilter, list, filter);
+ }
+ return [];
+ }
+
+ List<Change> resultFilter(List<Change> list, Filter filter) {
+ if (filter.showLatestFailures && list.first.resultStyle != 'failure') {
+ return [];
+ }
+ return applyFilter<Change>((c, f) => c, list, filter,
+ (change, filter) => filter.showUnapprovedOnly && !change.approved);
}
}
diff --git a/results_feed/lib/src/filter_service.dart b/results_feed/lib/src/filter_service.dart
index 61a345d..160c877 100644
--- a/results_feed/lib/src/filter_service.dart
+++ b/results_feed/lib/src/filter_service.dart
@@ -8,24 +8,30 @@
final List<String> configurationGroups;
final bool showAllCommits;
final bool showLatestFailures;
+ final bool showUnapprovedOnly;
- const Filter._(
- this.configurationGroups, this.showAllCommits, this.showLatestFailures);
- Filter(this.configurationGroups, showAllCommits, this.showLatestFailures)
+ const Filter._(this.configurationGroups, this.showAllCommits,
+ this.showLatestFailures, this.showUnapprovedOnly);
+ Filter(this.configurationGroups, showAllCommits, this.showLatestFailures,
+ this.showUnapprovedOnly)
: showAllCommits = configurationGroups.isEmpty || showAllCommits;
static const defaultFilter = Filter._(
- allConfigurationGroups, defaultShowAllCommits, defaultShowLatestFailures);
- static const noneFilter = Filter._(allConfigurationGroups, true, false);
+ allConfigurationGroups,
+ defaultShowAllCommits,
+ defaultShowLatestFailures,
+ defaultShowUnapprovedOnly);
Filter copy(
{List<String> configurationGroups,
bool showAllCommits,
- bool showLatestFailures}) =>
+ bool showLatestFailures,
+ bool showUnapprovedOnly}) =>
Filter(
configurationGroups ?? this.configurationGroups,
showAllCommits ?? this.showAllCommits,
- showLatestFailures ?? this.showLatestFailures);
+ showLatestFailures ?? this.showLatestFailures,
+ showUnapprovedOnly ?? this.showUnapprovedOnly);
bool get allGroups =>
configurationGroups.length == allConfigurationGroups.length;
@@ -67,6 +73,7 @@
static const defaultShowAllCommits = true;
static const defaultShowLatestFailures = false;
+ static const defaultShowUnapprovedOnly = false;
static const allConfigurationGroups = <String>[
'analyzer',
'app_jitk',
diff --git a/results_feed/lib/src/results_panel.dart b/results_feed/lib/src/results_panel.dart
index 88259e4..25f45a2 100644
--- a/results_feed/lib/src/results_panel.dart
+++ b/results_feed/lib/src/results_panel.dart
@@ -45,6 +45,9 @@
@Input()
IntRange range;
+ Map<String, List<String>> summaries(List<List<Change>> group) =>
+ group.first.first.configurations.summaries;
+
int resultLimit = 10;
final preferredTooltipPositions = [
diff --git a/results_feed/lib/src/results_panel.html b/results_feed/lib/src/results_panel.html
index 578175e..9078f2b 100644
--- a/results_feed/lib/src/results_panel.html
+++ b/results_feed/lib/src/results_panel.html
@@ -3,7 +3,7 @@
<template
ngFor
let-configuration
- [ngForOf]="configurationGroup.configurations.summaries.keys">
+ [ngForOf]="summaries(configurationGroup).keys">
<material-chip
tooltipTarget
#chip="tooltipTarget"
@@ -13,7 +13,7 @@
</material-chip>
<material-tooltip-card [for]="chip">
<div *deferredContent>
- <span *ngFor="let singleConfiguration of configurationGroup.configurations.summaries[configuration]">
+ <span *ngFor="let singleConfiguration of summaries(configurationGroup)[configuration]">
{{singleConfiguration}}<br>
</span>
</div>
@@ -24,7 +24,7 @@
*ngFor="let resultGroup of configurationGroup"
style="margin-left: 16px">
<span [class]="resultGroup.first.resultStyle">
- {{resultGroup.changesText}}
+ {{resultGroup.first.changesText}}
</span><br>
<span *ngFor="let change of resultGroup.take(resultLimit)"
tooltipTarget #logs="tooltipTarget"
diff --git a/results_feed/lib/src/results_selector_panel.dart b/results_feed/lib/src/results_selector_panel.dart
index 8299599..d3a1fb6 100644
--- a/results_feed/lib/src/results_selector_panel.dart
+++ b/results_feed/lib/src/results_selector_panel.dart
@@ -48,9 +48,15 @@
@Input()
set changes(Changes changes) {
_changes = changes;
- selectableChanges = [
- for (ConfigGroup c in changes) SelectableConfigurationGroup(c, this)
- ];
+ for (final configurationGroup in changes) {
+ configurationCheckboxes[configurationGroup] = FixedMixedCheckbox();
+ for (final resultGroup in configurationGroup) {
+ resultCheckboxes[resultGroup] = FixedMixedCheckbox();
+ for (final change in resultGroup) {
+ checked[change] = true;
+ }
+ }
+ }
initializeSelected();
}
@@ -58,6 +64,9 @@
ChangeGroup commit;
@Input()
+ IntRange range;
+
+ @Input()
set selected(Set<Change> selected) {
_selected = selected;
initializeSelected();
@@ -65,7 +74,10 @@
Changes _changes;
Set<Change> _selected;
- List<SelectableConfigurationGroup> selectableChanges;
+
+ final checked = Map<Change, bool>();
+ final resultCheckboxes = Map<List<Change>, FixedMixedCheckbox>();
+ final configurationCheckboxes = Map<List<List<Change>>, FixedMixedCheckbox>();
int resultLimit = 10;
Changes get changes => _changes;
@@ -75,89 +87,58 @@
RelativePosition.OffsetTopLeft
];
+ Map<String, List<String>> summaries(List<List<Change>> group) =>
+ group.first.first.configurations.summaries;
+
void initializeSelected() {
if (_selected != null && _changes != null) {
- _selected.addAll([
- for (final c in selectableChanges)
- for (SelectableResultGroup r in c.resultGroups)
- for (SelectableChange h in r.changes) h.change
- ]);
+ _selected.addAll(checked.keys);
}
}
-}
-class SelectableChange {
- final Change change;
- bool selected = true;
- final SelectableResultGroup resultGroup;
- final SelectableConfigurationGroup configurationGroup;
- final ResultsSelectorPanel panel;
-
- SelectableChange(
- this.change, this.resultGroup, this.configurationGroup, this.panel);
-
- void onChange(bool event) {
- if (selected == event) return;
- selected = event;
+ bool setCheckbox(Change change, bool event) {
+ if (checked[change] == event) return false;
+ checked[change] = event;
if (event) {
- panel._selected.add(this.change);
+ _selected.add(change);
} else {
- panel._selected.remove(this.change);
+ _selected.remove(change);
}
- resultGroup.checkbox.setMixed();
- configurationGroup.checkbox.setMixed();
- }
-}
-
-class SelectableResultGroup {
- List<SelectableChange> changes;
- SelectableConfigurationGroup configurationGroup;
- FixedMixedCheckbox checkbox = FixedMixedCheckbox();
-
- SelectableResultGroup(
- ResultGroup group, this.configurationGroup, ResultsSelectorPanel panel) {
- changes = [
- for (final change in group)
- SelectableChange(change, this, configurationGroup, panel)
- ];
+ return true;
}
- void onChange(String event) {
+ void onChange(bool event, Change change, List<Change> resultGroup,
+ List<List<Change>> configurationGroup) {
+ if (setCheckbox(change, event)) {
+ configurationCheckboxes[configurationGroup].setMixed();
+ resultCheckboxes[resultGroup].setMixed();
+ }
+ }
+
+ void onResultChange(String event, List<Change> resultGroup,
+ List<List<Change>> configurationGroup) {
+ final checkbox = resultCheckboxes[resultGroup];
if (checkbox.eventMatchesState(event)) return;
assert(event != 'mixed');
bool newChecked = event == 'true';
checkbox.setState(newChecked, false);
- for (final change in changes) {
- change.selected = newChecked;
+ for (final change in resultGroup) {
+ setCheckbox(change, newChecked);
}
- configurationGroup.checkbox.setMixed();
- }
-}
-
-class SelectableConfigurationGroup {
- List<SelectableResultGroup> resultGroups;
- FixedMixedCheckbox checkbox = FixedMixedCheckbox();
-
- SelectableConfigurationGroup(
- ConfigGroup configGroup, ResultsSelectorPanel panel) {
- resultGroups = [
- for (final resultGroup in configGroup)
- SelectableResultGroup(resultGroup, this, panel)
- ];
+ configurationCheckboxes[configurationGroup].setMixed();
}
- get summaries =>
- resultGroups.first.changes.first.change.configurations.summaries;
-
- void onChange(String event) {
+ void onConfigurationChange(
+ String event, List<List<Change>> configurationGroup) {
+ final checkbox = configurationCheckboxes[configurationGroup];
if (checkbox.eventMatchesState(event)) return;
assert(event != 'mixed');
bool newChecked = event == 'true';
checkbox.setState(newChecked, false);
- for (final group in resultGroups) {
- group.checkbox.setState(newChecked, false);
- for (final change in group.changes) {
- change.selected = newChecked;
+ for (final subgroup in configurationGroup) {
+ resultCheckboxes[subgroup].setState(newChecked, false);
+ for (final change in subgroup) {
+ setCheckbox(change, newChecked);
}
}
}
@@ -166,6 +147,7 @@
class FixedMixedCheckbox {
bool checked = true;
bool indeterminate = false;
+
// Model change indeterminate <-> checked generates a bad 'unchecked' event.
// Workaround for issue https://github.com/dart-lang/angular_components/issues/434
bool expectBadEvent = false;
diff --git a/results_feed/lib/src/results_selector_panel.html b/results_feed/lib/src/results_selector_panel.html
index f3d8c14..c8daf2f 100644
--- a/results_feed/lib/src/results_selector_panel.html
+++ b/results_feed/lib/src/results_selector_panel.html
@@ -1,15 +1,15 @@
-<div *ngFor="let configurationGroup of selectableChanges">
+<div *ngFor="let configurationGroup of changes">
<material-chips>
<material-checkbox
class="result-checkbox"
- [checked]="configurationGroup.checkbox.checked"
- (change)="configurationGroup.onChange($event)"
- [indeterminate]="configurationGroup.checkbox.indeterminate">
+ [checked]="configurationCheckboxes[configurationGroup].checked"
+ [indeterminate]="configurationCheckboxes[configurationGroup].indeterminate"
+ (change)="onConfigurationChange($event, configurationGroup)">
</material-checkbox>
<template
ngFor
let-configuration
- [ngForOf]="configurationGroup.summaries.keys">
+ [ngForOf]="summaries(configurationGroup).keys">
<material-chip
tooltipTarget
#chip="tooltipTarget"
@@ -19,53 +19,53 @@
</material-chip>
<material-tooltip-card [for]="chip">
<div *deferredContent>
- <span *ngFor="let singleConfiguration of configurationGroup.summaries[configuration]">
+ <span *ngFor="let singleConfiguration of summaries(configurationGroup)[configuration]">
{{singleConfiguration}}<br>
</span>
</div>
</material-tooltip-card>
</template>
</material-chips>
- <span *ngFor="let resultGroup of configurationGroup.resultGroups"
+ <span *ngFor="let resultGroup of configurationGroup"
style="margin-left: 16px">
<material-checkbox
no-ink
- [checked]="resultGroup.checkbox.checked"
- (change)="resultGroup.onChange($event)"
- [indeterminate]="resultGroup.checkbox.indeterminate">
+ [checked]="resultCheckboxes[resultGroup].checked"
+ (change)="onResultChange($event, resultGroup, configurationGroup)"
+ [indeterminate]="resultCheckboxes[resultGroup].indeterminate">
</material-checkbox>
- <span [class]="resultGroup.changes.first.change.resultStyle">
- {{resultGroup.changes.first.change.changesText}}
+ <span [class]="resultGroup.first.resultStyle">
+ {{resultGroup.first.changesText}}
</span><br>
- <span class="nonbreaking" *ngFor="let change of resultGroup.changes.take(resultLimit)"
+ <span class="nonbreaking" *ngFor="let change of resultGroup.take(resultLimit)"
tooltipTarget
#logs="tooltipTarget"
style="cursor: pointer; margin-left: 16px; height: 16px">
<material-checkbox
- no-ink
- [checked]="change.selected"
- (checkedChange)="change.onChange($event)">
+ no-ink
+ [checked]="checked[change]"
+ (checkedChange)="onChange($event, change, resultGroup, configurationGroup)">
</material-checkbox>
- <span tooltipTarget #logs="tooltipTarget">{{change.change.name}}</span><br>
+ <span tooltipTarget #logs="tooltipTarget">{{change.name}}</span><br>
<material-tooltip-card
- [for]="logs"
- [preferredPositions]="preferredTooltipPositions">
+ [for]="logs"
+ [preferredPositions]="preferredTooltipPositions">
<div *deferredContent>
<h4>Logs</h4>
<dart-log
- *ngFor="let configuration of change.change.configurations.configurations"
- [configuration]="configuration"
- [index]="commit.range.end"
- [test]="change.change.name">
+ *ngFor="let configuration of change.configurations.configurations"
+ [configuration]="configuration"
+ [index]="range.end"
+ [test]="change.name">
</dart-log>
</div>
</material-tooltip-card>
</span>
<div
- *ngIf="resultGroup.changes.length > resultLimit"
- (click)="resultLimit = resultGroup.changes.length"
+ *ngIf="resultGroup.length > resultLimit"
+ (click)="resultLimit = resultGroup.length"
style="cursor: pointer">
- ⋮ {{resultGroup.changes.length - resultLimit}} more changed results
+ ⋮ {{resultGroup.length - resultLimit}} more changed results
</div>
</span>
</div>
