[results feed] The 'approved' field in results is no longer optional
Old documents may still be missing this field, handle them correctly.
Change-Id: I7e99e3bdfc2173b9504363bd27e2a2cd034cd31c
Reviewed-on: https://dart-review.googlesource.com/c/dart_ci/+/128592
Reviewed-by: Alexander Thomas <athom@google.com>
diff --git a/results_feed/doc/results_feed.proto b/results_feed/doc/results_feed.proto
index dfa312d..64e16ca 100644
--- a/results_feed/doc/results_feed.proto
+++ b/results_feed/doc/results_feed.proto
@@ -42,11 +42,7 @@
// Chosen by a user in the results feed UI.
int32 pinned_index = 10;
- // If the result has been approved, or subsequently unapproved, by a comment,
- // this field is present and has the appropriate boolean value.
- // If this field is present, then there is a comment record containing this
- // result's "id" field in its "results" repeated field.
- bool approved = 13;
+ bool approved = 13; // May be missing in older records, interpreted as false.
}
// Similar to a Result message, but representing a result of a try builder on the CQ.
@@ -75,11 +71,7 @@
int32 review = 11;
int32 patchset = 12;
- // If the try result has been approved, or subsequently unapproved, by a
- // comment, this field is present and has the appropriate boolean value.
- // If this field is present, then there is a comment record containing this
- // try result's "id" field in its "try_results" repeated field.
- bool approved = 13;
+ bool approved = 13; // May be missing in older records, interpreted as false.
}
// Represents a single build of a CI builder
@@ -172,7 +164,7 @@
string link = 5;
// If approved is true, this comment approves the linked tests
- // If approved is false, it unapproves them, removing earlier approvals
+ // If approved is false, it removes any earlier approvals on these tests
// If the field is missing, it does not change the current approval status
bool approved = 6;
diff --git a/results_feed/lib/src/components/results_panel.dart b/results_feed/lib/src/components/results_panel.dart
index bade11d..f9acce5 100644
--- a/results_feed/lib/src/components/results_panel.dart
+++ b/results_feed/lib/src/components/results_panel.dart
@@ -55,9 +55,6 @@
RelativePosition.OffsetTopLeft
];
- String approvalContent(Change change) {
- if (change.approved) return formatting.checkmark;
- if (change.disapproved) return formatting.bigRedX;
- return "";
- }
+ String approvalContent(Change change) =>
+ change.approved ? formatting.checkmark: '';
}
diff --git a/results_feed/lib/src/components/results_selector_panel.dart b/results_feed/lib/src/components/results_selector_panel.dart
index 9c666ba..38ed5d0 100644
--- a/results_feed/lib/src/components/results_selector_panel.dart
+++ b/results_feed/lib/src/components/results_selector_panel.dart
@@ -112,11 +112,8 @@
Map<String, List<String>> summaries(List<List<Change>> group) =>
group.first.first.configurations.summaries;
- String approvalContent(Change change) {
- if (change.approved) return formatting.checkmark;
- if (change.disapproved) return formatting.bigRedX;
- return "";
- }
+ String approvalContent(Change change) =>
+ change.approved ? formatting.checkmark : '';
void initializeSelected() {
if (_selected != null && _changes != null) {
diff --git a/results_feed/lib/src/formatting.dart b/results_feed/lib/src/formatting.dart
index 1561ac5..b8b1a12 100644
--- a/results_feed/lib/src/formatting.dart
+++ b/results_feed/lib/src/formatting.dart
@@ -22,6 +22,5 @@
return email;
}
-// Both characters are followed by a space.
+// Checkmark followed by a space. Used for approved results.
String checkmark = "\u2714 ";
-String bigRedX = "\u274C ";
diff --git a/results_feed/lib/src/model/comment.dart b/results_feed/lib/src/model/comment.dart
index 40434b5..aeafdf5 100644
--- a/results_feed/lib/src/model/comment.dart
+++ b/results_feed/lib/src/model/comment.dart
@@ -54,8 +54,7 @@
review = document.get('review'),
patchset = document.get('patchset');
- String approvedText() =>
- (approved == null) ? "" : approved ? "approved" : "disapproved";
+ String approvedText() => approved ? 'approved' : '';
int compareTo(Object other) => created.compareTo((other as Comment).created);
}
diff --git a/results_feed/lib/src/model/commit.dart b/results_feed/lib/src/model/commit.dart
index cd035dc..98ca5fb 100644
--- a/results_feed/lib/src/model/commit.dart
+++ b/results_feed/lib/src/model/commit.dart
@@ -162,8 +162,7 @@
this.blamelistStartIndex,
this.blamelistEndIndex,
this.pinnedIndex,
- this.approved,
- this.disapproved)
+ this.approved)
: changesText = '$previousResult -> $result (expected $expected)',
configurationsText = configurations.text;
@@ -178,9 +177,9 @@
document.get('blamelist_start_index'),
document.get('blamelist_end_index'),
document.get('pinned_index'),
- // approved field is null for unapproved new failures.
- document.get('approved') == true,
- document.get('approved') == false);
+ // Old documents may not have this field.
+ document.get('approved') ?? false
+);
final String id;
final String name;
@@ -192,7 +191,6 @@
final int blamelistEndIndex;
int pinnedIndex;
bool approved;
- bool disapproved; // An approval has been explicitly revoked.
final String configurationsText;
final String changesText;
@@ -208,8 +206,7 @@
blamelistStartIndex,
blamelistEndIndex,
pinnedIndex,
- approved,
- disapproved);
+ approved);
int compareTo(Object other) => name.compareTo((other as Change).name);
