From 5f80357f8ce8c4cfe328c452e3150cc033a09f46 Mon Sep 17 00:00:00 2001 From: Andrej Velichkovski Date: Mon, 8 Jul 2024 10:13:35 +0200 Subject: [PATCH 1/4] Deprecate two enum choices --- frontend/lib/models/test_execution.dart | 13 +++++++++ .../test_execution_pop_over.dart | 28 +++++++++++++------ .../artefact_page/test_execution_review.dart | 2 +- 3 files changed, 33 insertions(+), 10 deletions(-) diff --git a/frontend/lib/models/test_execution.dart b/frontend/lib/models/test_execution.dart index e3376ea0..1483a06b 100644 --- a/frontend/lib/models/test_execution.dart +++ b/frontend/lib/models/test_execution.dart @@ -127,6 +127,19 @@ enum TestExecutionReviewDecision { } } + bool get isDeprecated { + switch (this) { + case approvedFaultyHardware: + case approvedUnstablePhysicalInfra: + return true; + case rejected: + case approvedInconsistentTest: + case approvedCustomerPrerequisiteFail: + case approvedAllTestsPass: + return false; + } + } + String toJson() { switch (this) { case rejected: diff --git a/frontend/lib/ui/artefact_page/test_execution_pop_over.dart b/frontend/lib/ui/artefact_page/test_execution_pop_over.dart index 1c08fdda..65272450 100644 --- a/frontend/lib/ui/artefact_page/test_execution_pop_over.dart +++ b/frontend/lib/ui/artefact_page/test_execution_pop_over.dart @@ -36,10 +36,12 @@ class TestExecutionPopOverState extends ConsumerState { Function(bool?)? getOnChangedCheckboxListTileFunction( TestExecutionReviewDecision testExecutionReviewDecision, ) { - if ((testExecutionReviewDecision == TestExecutionReviewDecision.rejected && - (_canReject)) || - (testExecutionReviewDecision != TestExecutionReviewDecision.rejected && - _canApprove)) { + if (!testExecutionReviewDecision.isDeprecated && + ((testExecutionReviewDecision == TestExecutionReviewDecision.rejected && + _canReject) || + (testExecutionReviewDecision != + TestExecutionReviewDecision.rejected && + _canApprove))) { return (bool? value) { setState(() { if (reviewDecisions.contains(testExecutionReviewDecision)) { @@ -53,6 +55,11 @@ class TestExecutionPopOverState extends ConsumerState { return null; } + bool shouldDisplayDecision(TestExecutionReviewDecision decision) { + return !decision.isDeprecated || + (decision.isDeprecated && reviewDecisions.contains(decision)); + } + @override void initState() { super.initState(); @@ -79,12 +86,15 @@ class TestExecutionPopOverState extends ConsumerState { Column( children: TestExecutionReviewDecision.values .map( - (e) => YaruCheckboxListTile( - value: reviewDecisions.contains(e), - onChanged: getOnChangedCheckboxListTileFunction(e), - title: Text(e.name), - ), + (e) => shouldDisplayDecision(e) + ? YaruCheckboxListTile( + value: reviewDecisions.contains(e), + onChanged: getOnChangedCheckboxListTileFunction(e), + title: Text(e.name), + ) + : null, ) + .whereType() .toList(), ), const SizedBox(height: Spacing.level4), diff --git a/frontend/lib/ui/artefact_page/test_execution_review.dart b/frontend/lib/ui/artefact_page/test_execution_review.dart index f3b634be..43017fbb 100644 --- a/frontend/lib/ui/artefact_page/test_execution_review.dart +++ b/frontend/lib/ui/artefact_page/test_execution_review.dart @@ -49,7 +49,7 @@ class TestExecutionReviewButton extends StatelessWidget { ), direction: PopoverDirection.bottom, width: 500, - height: 500, + height: 400, arrowHeight: 15, arrowWidth: 30, ); From 58619421eea2c779be7376d8dc0a10538f2b345d Mon Sep 17 00:00:00 2001 From: Andrej Velichkovski Date: Mon, 8 Jul 2024 10:45:05 +0200 Subject: [PATCH 2/4] Use switch default --- frontend/lib/models/test_execution.dart | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/frontend/lib/models/test_execution.dart b/frontend/lib/models/test_execution.dart index 1483a06b..90dbdcbc 100644 --- a/frontend/lib/models/test_execution.dart +++ b/frontend/lib/models/test_execution.dart @@ -132,10 +132,7 @@ enum TestExecutionReviewDecision { case approvedFaultyHardware: case approvedUnstablePhysicalInfra: return true; - case rejected: - case approvedInconsistentTest: - case approvedCustomerPrerequisiteFail: - case approvedAllTestsPass: + default: return false; } } From 44f24bb57413990c832f88b7cfeb0e0bd480ab64 Mon Sep 17 00:00:00 2001 From: Andrej Velichkovski <33193463+andrejvelichkovski@users.noreply.github.com> Date: Tue, 16 Jul 2024 11:28:45 +0200 Subject: [PATCH 3/4] Update frontend/lib/ui/artefact_page/test_execution_pop_over.dart Co-authored-by: Omar Abou Selo --- frontend/lib/ui/artefact_page/test_execution_pop_over.dart | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/frontend/lib/ui/artefact_page/test_execution_pop_over.dart b/frontend/lib/ui/artefact_page/test_execution_pop_over.dart index 65272450..d4181cba 100644 --- a/frontend/lib/ui/artefact_page/test_execution_pop_over.dart +++ b/frontend/lib/ui/artefact_page/test_execution_pop_over.dart @@ -56,8 +56,7 @@ class TestExecutionPopOverState extends ConsumerState { } bool shouldDisplayDecision(TestExecutionReviewDecision decision) { - return !decision.isDeprecated || - (decision.isDeprecated && reviewDecisions.contains(decision)); + return !decision.isDeprecated || reviewDecisions.contains(decision); } @override From 17dfbbc6ef2f8012b44380f16d6bff2c605e5d00 Mon Sep 17 00:00:00 2001 From: Andrej Velichkovski Date: Tue, 16 Jul 2024 12:58:16 +0200 Subject: [PATCH 4/4] Explain long if condition on whether to show each checkbox --- .../ui/artefact_page/test_execution_pop_over.dart | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/frontend/lib/ui/artefact_page/test_execution_pop_over.dart b/frontend/lib/ui/artefact_page/test_execution_pop_over.dart index d4181cba..61895887 100644 --- a/frontend/lib/ui/artefact_page/test_execution_pop_over.dart +++ b/frontend/lib/ui/artefact_page/test_execution_pop_over.dart @@ -36,12 +36,14 @@ class TestExecutionPopOverState extends ConsumerState { Function(bool?)? getOnChangedCheckboxListTileFunction( TestExecutionReviewDecision testExecutionReviewDecision, ) { + // Ensure the test execution cannot be rejected and approved in the same time + final bool enableCheckboxConsistencyCheck = (testExecutionReviewDecision == + TestExecutionReviewDecision.rejected && + _canReject) || + (testExecutionReviewDecision != TestExecutionReviewDecision.rejected && + _canApprove); if (!testExecutionReviewDecision.isDeprecated && - ((testExecutionReviewDecision == TestExecutionReviewDecision.rejected && - _canReject) || - (testExecutionReviewDecision != - TestExecutionReviewDecision.rejected && - _canApprove))) { + enableCheckboxConsistencyCheck) { return (bool? value) { setState(() { if (reviewDecisions.contains(testExecutionReviewDecision)) {