From 3955c2419c83c2d718afb7ca8db2b75b5a0517de Mon Sep 17 00:00:00 2001 From: Andrej Velichkovski Date: Mon, 4 Dec 2023 15:24:20 +0100 Subject: [PATCH 1/6] Update backend t o support test execution review --- ...1357-f45b1594fb23_migration_description.py | 35 +++++++++++++++++++ .../controllers/artefacts/models.py | 8 ++++- .../controllers/test_executions/models.py | 8 ++++- .../test_executions/test_executions.py | 6 ++++ backend/test_observer/data_access/models.py | 5 +++ .../test_observer/data_access/models_enums.py | 8 +++++ .../controllers/artefacts/test_artefacts.py | 8 +++-- .../test_executions/test_test_executions.py | 6 +++- 8 files changed, 79 insertions(+), 5 deletions(-) create mode 100644 backend/migrations/versions/2023_12_04_1357-f45b1594fb23_migration_description.py diff --git a/backend/migrations/versions/2023_12_04_1357-f45b1594fb23_migration_description.py b/backend/migrations/versions/2023_12_04_1357-f45b1594fb23_migration_description.py new file mode 100644 index 00000000..e6a95258 --- /dev/null +++ b/backend/migrations/versions/2023_12_04_1357-f45b1594fb23_migration_description.py @@ -0,0 +1,35 @@ +"""Add test execution review status and comment + +Revision ID: f45b1594fb23 +Revises: 8317277d4333 +Create Date: 2023-12-04 13:57:23.385250+00:00 + +""" +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = "f45b1594fb23" +down_revision = "8317277d4333" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + op.execute( + "CREATE TYPE test_execution_review_status_enum AS " + "ENUM('APPROVED_FLAKY_TESTS', 'APPROVED_PROVISION_ERRORS', 'APPROVED', 'MARKED_AS_FAILED', 'UNDECIDED')" + ) + op.execute( + "ALTER TABLE test_execution ADD COLUMN review_status test_execution_review_status_enum" + ) + op.add_column( + "test_execution", sa.Column("review_comment", sa.String(), nullable=True) + ) + + +def downgrade() -> None: + op.drop_column("test_execution", "review_comment") + op.drop_column("test_execution", "review_status") + op.execute("DROP TYPE test_execution_review_status_enum") diff --git a/backend/test_observer/controllers/artefacts/models.py b/backend/test_observer/controllers/artefacts/models.py index 72707641..722f959c 100644 --- a/backend/test_observer/controllers/artefacts/models.py +++ b/backend/test_observer/controllers/artefacts/models.py @@ -19,7 +19,11 @@ # Nadzeya Hutsko from pydantic import AliasPath, BaseModel, ConfigDict, Field -from test_observer.data_access.models_enums import ArtefactStatus, TestExecutionStatus +from test_observer.data_access.models_enums import ( + ArtefactStatus, + TestExecutionReviewStatus, + TestExecutionStatus, +) class ArtefactDTO(BaseModel): @@ -52,6 +56,8 @@ class TestExecutionDTO(BaseModel): c3_link: str | None environment: EnvironmentDTO status: TestExecutionStatus + review_status: TestExecutionReviewStatus + review_comment: str | None class ArtefactBuildDTO(BaseModel): diff --git a/backend/test_observer/controllers/test_executions/models.py b/backend/test_observer/controllers/test_executions/models.py index 12cfc555..951a318d 100644 --- a/backend/test_observer/controllers/test_executions/models.py +++ b/backend/test_observer/controllers/test_executions/models.py @@ -22,7 +22,11 @@ from pydantic import BaseModel, HttpUrl, field_serializer, model_validator -from test_observer.data_access.models_enums import FamilyName, TestExecutionStatus +from test_observer.data_access.models_enums import ( + FamilyName, + TestExecutionReviewStatus, + TestExecutionStatus, +) class StartTestExecutionRequest(BaseModel): @@ -62,3 +66,5 @@ class TestExecutionsPatchRequest(BaseModel): c3_link: HttpUrl | None ci_link: HttpUrl | None status: TestExecutionStatus | None + review_status: TestExecutionReviewStatus | None + review_comment: str | None diff --git a/backend/test_observer/controllers/test_executions/test_executions.py b/backend/test_observer/controllers/test_executions/test_executions.py index d56c2306..c0068be7 100644 --- a/backend/test_observer/controllers/test_executions/test_executions.py +++ b/backend/test_observer/controllers/test_executions/test_executions.py @@ -119,4 +119,10 @@ def patch_test_execution( if request.status is not None: test_execution.status = request.status + if request.review_status is not None: + test_execution.review_status = request.review_status + + if request.review_comment is not None: + test_execution.review_comment = request.review_comment + db.commit() diff --git a/backend/test_observer/data_access/models.py b/backend/test_observer/data_access/models.py index be165388..58b203d3 100644 --- a/backend/test_observer/data_access/models.py +++ b/backend/test_observer/data_access/models.py @@ -31,6 +31,7 @@ from test_observer.data_access.models_enums import ( ArtefactStatus, + TestExecutionReviewStatus, TestExecutionStatus, ) @@ -213,6 +214,10 @@ class TestExecution(Base): status: Mapped[TestExecutionStatus] = mapped_column( default=TestExecutionStatus.NOT_STARTED ) + review_status: Mapped[TestExecutionReviewStatus] = mapped_column( + default=TestExecutionReviewStatus.UNDECIDED + ) + review_comment: Mapped[str | None] __table_args__ = (UniqueConstraint("artefact_build_id", "environment_id"),) diff --git a/backend/test_observer/data_access/models_enums.py b/backend/test_observer/data_access/models_enums.py index 0e32c3c9..4b4f7cb9 100644 --- a/backend/test_observer/data_access/models_enums.py +++ b/backend/test_observer/data_access/models_enums.py @@ -35,6 +35,14 @@ class TestExecutionStatus(Enum): NOT_TESTED = "NOT_TESTED" +class TestExecutionReviewStatus(Enum): + APPROVED_FLAKY_TESTS = "APPROVED_FLAKY_TESTS" + APPROVED_PROVISION_ERRORS = "APPROVED_PROVISION_ERRORS" + APPROVED = "APPROVED" + MARKED_AS_FAILED = "MARKED_AS_FAILED" + UNDECIDED = "UNDECIDED" + + class ArtefactStatus(str, Enum): APPROVED = "APPROVED" MARKED_AS_FAILED = "MARKED_AS_FAILED" diff --git a/backend/tests/controllers/artefacts/test_artefacts.py b/backend/tests/controllers/artefacts/test_artefacts.py index c51ec513..0147081e 100644 --- a/backend/tests/controllers/artefacts/test_artefacts.py +++ b/backend/tests/controllers/artefacts/test_artefacts.py @@ -42,6 +42,8 @@ def test_get_latest_artefacts_by_family(db_session: Session, test_client: TestCl response = test_client.get("/v1/artefacts", params={"family": "snap"}) + print(response) + assert response.status_code == 200 assert response.json() == [ { @@ -53,7 +55,7 @@ def test_get_latest_artefacts_by_family(db_session: Session, test_client: TestCl "series": relevant_artefact.series, "repo": relevant_artefact.repo, "stage": relevant_artefact.stage.name, - "status": relevant_artefact.status, + "status": relevant_artefact.status.value, } ] @@ -74,7 +76,7 @@ def test_get_artefact(db_session: Session, test_client: TestClient): "series": artefact.series, "repo": artefact.repo, "stage": artefact.stage.name, - "status": artefact.status, + "status": artefact.status.value, } @@ -104,6 +106,8 @@ def test_get_artefact_builds(db_session: Session, test_client: TestClient): "ci_link": test_execution.ci_link, "c3_link": test_execution.c3_link, "status": test_execution.status.value, + "review_status": test_execution.review_status.value, + "review_comment": test_execution.review_comment, "environment": { "id": environment.id, "name": environment.name, diff --git a/backend/tests/controllers/test_executions/test_test_executions.py b/backend/tests/controllers/test_executions/test_test_executions.py index b3fc7c35..a8e917ce 100644 --- a/backend/tests/controllers/test_executions/test_test_executions.py +++ b/backend/tests/controllers/test_executions/test_test_executions.py @@ -30,7 +30,7 @@ Stage, TestExecution, ) -from test_observer.data_access.models_enums import FamilyName, TestExecutionStatus +from test_observer.data_access.models_enums import FamilyName, TestExecutionReviewStatus, TestExecutionStatus def test_creates_all_data_models(db_session: Session, test_client: TestClient): @@ -187,6 +187,8 @@ def test_updates_test_execution(db_session: Session, test_client: TestClient): "ci_link": "http://ci_link/", "c3_link": "http://c3_link/", "status": TestExecutionStatus.PASSED.name, + "review_status": TestExecutionReviewStatus.APPROVED.name, + "review_comment": "Flaky tests", }, ) @@ -194,3 +196,5 @@ def test_updates_test_execution(db_session: Session, test_client: TestClient): assert test_execution.ci_link == "http://ci_link/" assert test_execution.c3_link == "http://c3_link/" assert test_execution.status == TestExecutionStatus.PASSED + assert test_execution.review_status == TestExecutionReviewStatus.APPROVED + assert test_execution.review_comment == "Flaky tests" From 257da76313f9d7d7c7878423030c531f730b6dae Mon Sep 17 00:00:00 2001 From: Andrej Velichkovski Date: Mon, 4 Dec 2023 15:45:39 +0100 Subject: [PATCH 2/6] Fix syntax issues --- .../2023_12_04_1357-f45b1594fb23_migration_description.py | 6 ++++-- backend/tests/controllers/artefacts/test_artefacts.py | 2 -- .../controllers/test_executions/test_test_executions.py | 6 +++++- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/backend/migrations/versions/2023_12_04_1357-f45b1594fb23_migration_description.py b/backend/migrations/versions/2023_12_04_1357-f45b1594fb23_migration_description.py index e6a95258..08c3728a 100644 --- a/backend/migrations/versions/2023_12_04_1357-f45b1594fb23_migration_description.py +++ b/backend/migrations/versions/2023_12_04_1357-f45b1594fb23_migration_description.py @@ -19,10 +19,12 @@ def upgrade() -> None: op.execute( "CREATE TYPE test_execution_review_status_enum AS " - "ENUM('APPROVED_FLAKY_TESTS', 'APPROVED_PROVISION_ERRORS', 'APPROVED', 'MARKED_AS_FAILED', 'UNDECIDED')" + "ENUM('APPROVED_FLAKY_TESTS', 'APPROVED_PROVISION_ERRORS', " + "'APPROVED', 'MARKED_AS_FAILED', 'UNDECIDED')" ) op.execute( - "ALTER TABLE test_execution ADD COLUMN review_status test_execution_review_status_enum" + "ALTER TABLE test_execution ADD COLUMN " + "review_status test_execution_review_status_enum" ) op.add_column( "test_execution", sa.Column("review_comment", sa.String(), nullable=True) diff --git a/backend/tests/controllers/artefacts/test_artefacts.py b/backend/tests/controllers/artefacts/test_artefacts.py index 0147081e..723f6614 100644 --- a/backend/tests/controllers/artefacts/test_artefacts.py +++ b/backend/tests/controllers/artefacts/test_artefacts.py @@ -42,8 +42,6 @@ def test_get_latest_artefacts_by_family(db_session: Session, test_client: TestCl response = test_client.get("/v1/artefacts", params={"family": "snap"}) - print(response) - assert response.status_code == 200 assert response.json() == [ { diff --git a/backend/tests/controllers/test_executions/test_test_executions.py b/backend/tests/controllers/test_executions/test_test_executions.py index a8e917ce..8dfa2072 100644 --- a/backend/tests/controllers/test_executions/test_test_executions.py +++ b/backend/tests/controllers/test_executions/test_test_executions.py @@ -30,7 +30,11 @@ Stage, TestExecution, ) -from test_observer.data_access.models_enums import FamilyName, TestExecutionReviewStatus, TestExecutionStatus +from test_observer.data_access.models_enums import ( + FamilyName, + TestExecutionReviewStatus, + TestExecutionStatus, +) def test_creates_all_data_models(db_session: Session, test_client: TestClient): From 9ec6c5bd6c548b249b7863d30e10f11480821eb0 Mon Sep 17 00:00:00 2001 From: Andrej Velichkovski Date: Tue, 5 Dec 2023 11:02:47 +0100 Subject: [PATCH 3/6] Add initial frontend implementation --- frontend/lib/models/test_execution.dart | 45 ++++++++++++ .../lib/ui/dashboard/artefact_dialog.dart | 6 ++ .../test_execution_review_pop_over.dart | 69 +++++++++++++++++++ 3 files changed, 120 insertions(+) create mode 100644 frontend/lib/ui/dashboard/test_execution_review_pop_over.dart diff --git a/frontend/lib/models/test_execution.dart b/frontend/lib/models/test_execution.dart index 476e6010..d3f733da 100644 --- a/frontend/lib/models/test_execution.dart +++ b/frontend/lib/models/test_execution.dart @@ -16,12 +16,57 @@ class TestExecution with _$TestExecution { @JsonKey(name: 'c3_link') required String? c3Link, required TestExecutionStatus status, required Environment environment, + @JsonKey(name: 'review_status') + required TestExecutionReviewStatus reviewStatus, + @JsonKey(name: 'review_comment') required String? reviewComment, }) = _TestExecution; factory TestExecution.fromJson(Map json) => _$TestExecutionFromJson(json); } +enum TestExecutionReviewStatus { + @JsonValue('UNDECIDED') + undecided, + @JsonValue('APPROVED') + accepted, + @JsonValue('MARKED_AS_FAILED') + markedAsFailed; + + String get name { + switch (this) { + case undecided: + return 'Undecided'; + case accepted: + return 'Approved'; + case markedAsFailed: + return 'Marked as Failed'; + } + } + + Chip get chip { + switch (this) { + case undecided: + return const Chip( + label: Text('Undecided'), + backgroundColor: Color.fromARGB(255, 168, 166, 166), + side: BorderSide.none, + ); + case accepted: + return const Chip( + label: Text('Approved'), + backgroundColor: Color.fromARGB(255, 157, 205, 165), + side: BorderSide.none, + ); + case markedAsFailed: + return const Chip( + label: Text('Marked as failed'), + backgroundColor: Color.fromARGB(255, 218, 150, 166), + ); + } + } +} + enum TestExecutionStatus { @JsonValue('FAILED') failed, diff --git a/frontend/lib/ui/dashboard/artefact_dialog.dart b/frontend/lib/ui/dashboard/artefact_dialog.dart index c0186e23..54a1dc76 100644 --- a/frontend/lib/ui/dashboard/artefact_dialog.dart +++ b/frontend/lib/ui/dashboard/artefact_dialog.dart @@ -4,6 +4,7 @@ import 'package:dartx/dartx.dart'; import 'package:flutter/material.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:intersperse/intersperse.dart'; +import 'package:popover/popover.dart'; import 'package:yaru/yaru.dart'; import 'package:yaru_icons/yaru_icons.dart'; import 'package:yaru_widgets/widgets.dart'; @@ -17,6 +18,7 @@ import '../../providers/artefact_builds.dart'; import '../../routing.dart'; import '../inline_url_text.dart'; import '../spacing.dart'; +import 'test_execution_review_pop_over.dart'; class ArtefactDialog extends ConsumerWidget { const ArtefactDialog({super.key, required this.artefactId}); @@ -286,6 +288,10 @@ class _TestExecutionView extends StatelessWidget { const Spacer(), Row( children: [ + testExecution.reviewStatus.chip, + const SizedBox(width: Spacing.level4), + TestExecutionReviewButton(testExecution: testExecution), + const SizedBox(width: Spacing.level4), if (ciLink != null) InlineUrlText( url: ciLink, diff --git a/frontend/lib/ui/dashboard/test_execution_review_pop_over.dart b/frontend/lib/ui/dashboard/test_execution_review_pop_over.dart new file mode 100644 index 00000000..f68d1ca7 --- /dev/null +++ b/frontend/lib/ui/dashboard/test_execution_review_pop_over.dart @@ -0,0 +1,69 @@ +import 'package:flutter/material.dart'; +import 'package:popover/popover.dart'; + +import '../../models/test_execution.dart'; + +class TestExecutionReviewButton extends StatelessWidget { + const TestExecutionReviewButton({super.key, required this.testExecution}); + + final TestExecution testExecution; + + @override + Widget build(BuildContext context) { + return Padding( + padding: const EdgeInsets.all(4), + child: ElevatedButton( + onPressed: () { + showPopover( + context: context, + bodyBuilder: (context) => _PopOver( + testExecution: testExecution, + ), + direction: PopoverDirection.bottom, + width: 500, + height: 300, + arrowHeight: 15, + arrowWidth: 30, + ); + }, + child: const Text('Review'), + ), + ); + } +} + +class _PopOver extends StatelessWidget { + const _PopOver({required this.testExecution}); + + final TestExecution testExecution; + + @override + Widget build(BuildContext context) { + return Padding( + padding: const EdgeInsets.symmetric(vertical: 8), + child: ListView( + padding: const EdgeInsets.all(8), + children: [ + Text( + 'Select new review status', + style: Theme.of(context).textTheme.titleLarge, + ), + Text( + 'Insert review comments:', + style: Theme.of(context).textTheme.titleLarge, + ), + const TextField( + decoration: InputDecoration( + border: OutlineInputBorder(), + hintText: 'Insert review comments', + ), + ), + const ElevatedButton( + onPressed: null, + child: Text('Submit Review'), + ), + ], + ), + ); + } +} From 79cb01d68af4f85129ccf9d5cb1e9ad2077c4a94 Mon Sep 17 00:00:00 2001 From: Andrej Velichkovski Date: Tue, 5 Dec 2023 11:07:39 +0100 Subject: [PATCH 4/6] Add pop-over dependency --- frontend/pubspec.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/frontend/pubspec.yaml b/frontend/pubspec.yaml index 421b5550..7ab1387e 100644 --- a/frontend/pubspec.yaml +++ b/frontend/pubspec.yaml @@ -14,6 +14,7 @@ dependencies: go_router: ^11.1.4 intersperse: ^2.0.0 json_annotation: ^4.8.1 + popover: ^0.2.8+2 riverpod_annotation: ^2.1.1 url_launcher: ^6.1.10 yaru: ^1.1.0 From 9b299958622bba56e37c87b7a2297dcd0600f3cc Mon Sep 17 00:00:00 2001 From: Andrej Velichkovski Date: Tue, 5 Dec 2023 17:17:05 +0100 Subject: [PATCH 5/6] Add experimental artefact signoff UI --- .../lib/ui/dashboard/artefact_dialog.dart | 20 +- .../lib/ui/dashboard/review_pop_overs.dart | 199 ++++++++++++++++++ .../test_execution_review_pop_over.dart | 69 ------ 3 files changed, 212 insertions(+), 76 deletions(-) create mode 100644 frontend/lib/ui/dashboard/review_pop_overs.dart delete mode 100644 frontend/lib/ui/dashboard/test_execution_review_pop_over.dart diff --git a/frontend/lib/ui/dashboard/artefact_dialog.dart b/frontend/lib/ui/dashboard/artefact_dialog.dart index 54a1dc76..92baab7f 100644 --- a/frontend/lib/ui/dashboard/artefact_dialog.dart +++ b/frontend/lib/ui/dashboard/artefact_dialog.dart @@ -4,7 +4,6 @@ import 'package:dartx/dartx.dart'; import 'package:flutter/material.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:intersperse/intersperse.dart'; -import 'package:popover/popover.dart'; import 'package:yaru/yaru.dart'; import 'package:yaru_icons/yaru_icons.dart'; import 'package:yaru_widgets/widgets.dart'; @@ -18,7 +17,7 @@ import '../../providers/artefact_builds.dart'; import '../../routing.dart'; import '../inline_url_text.dart'; import '../spacing.dart'; -import 'test_execution_review_pop_over.dart'; +import 'review_pop_overs.dart'; class ArtefactDialog extends ConsumerWidget { const ArtefactDialog({super.key, required this.artefactId}); @@ -43,7 +42,7 @@ class ArtefactDialog extends ConsumerWidget { data: (artefact) => Column( crossAxisAlignment: CrossAxisAlignment.start, children: [ - _ArtefactHeader(title: artefact.name), + _ArtefactHeader(artefact: artefact), const SizedBox(height: Spacing.level4), _ArtefactInfoSection(artefact: artefact), const SizedBox(height: Spacing.level4), @@ -63,9 +62,9 @@ class ArtefactDialog extends ConsumerWidget { } class _ArtefactHeader extends StatelessWidget { - const _ArtefactHeader({required this.title}); + const _ArtefactHeader({required this.artefact}); - final String title; + final Artefact artefact; @override Widget build(BuildContext context) { @@ -77,9 +76,16 @@ class _ArtefactHeader extends StatelessWidget { ), ), child: Row( - mainAxisAlignment: MainAxisAlignment.spaceBetween, children: [ - Text(title, style: Theme.of(context).textTheme.headlineLarge), + Row( + children: [ + Text(artefact.name, + style: Theme.of(context).textTheme.headlineLarge), + const SizedBox(height: Spacing.level6), + ApproveArtefactButton(artefact: artefact), + ], + ), + const Spacer(), InkWell( child: const Icon( YaruIcons.window_close, diff --git a/frontend/lib/ui/dashboard/review_pop_overs.dart b/frontend/lib/ui/dashboard/review_pop_overs.dart new file mode 100644 index 00000000..a64d8672 --- /dev/null +++ b/frontend/lib/ui/dashboard/review_pop_overs.dart @@ -0,0 +1,199 @@ +import 'package:flutter/material.dart'; +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:popover/popover.dart'; +import 'package:yaru_widgets/yaru_widgets.dart'; + +import '../../models/artefact.dart'; +import '../../models/artefact_build.dart'; +import '../../models/test_execution.dart'; +import '../../providers/artefact_builds.dart'; +import '../spacing.dart'; + +class TestExecutionReviewButton extends StatelessWidget { + const TestExecutionReviewButton({super.key, required this.testExecution}); + + final TestExecution testExecution; + + @override + Widget build(BuildContext context) { + return Padding( + padding: const EdgeInsets.all(4), + child: ElevatedButton( + onPressed: () { + showPopover( + context: context, + bodyBuilder: (context) => _TestExecutionPopOver( + testExecution: testExecution, + ), + direction: PopoverDirection.bottom, + width: 500, + height: 300, + arrowHeight: 15, + arrowWidth: 30, + ); + }, + child: const Text('Review'), + ), + ); + } +} + +class _TestExecutionPopOver extends StatelessWidget { + const _TestExecutionPopOver({required this.testExecution}); + + final TestExecution testExecution; + + @override + Widget build(BuildContext context) { + return Padding( + padding: const EdgeInsets.symmetric(vertical: 8), + child: ListView( + padding: const EdgeInsets.all(8), + children: [ + Text( + 'Select new review status', + style: Theme.of(context).textTheme.titleLarge, + ), + Text( + 'Insert review comments:', + style: Theme.of(context).textTheme.titleLarge, + ), + const TextField( + decoration: InputDecoration( + border: OutlineInputBorder(), + hintText: 'Insert review comments', + ), + ), + const ElevatedButton( + onPressed: null, + child: Text('Submit Review'), + ), + ], + ), + ); + } +} + +class ApproveArtefactButton extends ConsumerWidget { + const ApproveArtefactButton({super.key, required this.artefact}); + + final Artefact artefact; + @override + Widget build(BuildContext context, WidgetRef ref) { + final artefactBuilds = ref.watch(artefactBuildsProvider(artefact.id)); + + return artefactBuilds.when( + data: (artefactBuilds) => Padding( + padding: const EdgeInsets.all(4), + child: ElevatedButton( + onPressed: () { + showPopover( + context: context, + bodyBuilder: (context) => _ArtefactReviewPopOver( + artefact: artefact, + artefactBuilds: artefactBuilds, + ), + direction: PopoverDirection.top, + width: 350, + height: 150, + arrowHeight: 15, + arrowWidth: 30, + ); + }, + child: const Text('Approve Artefact'), + ), + ), + loading: () => const Center(child: YaruCircularProgressIndicator()), + error: (error, stackTrace) { + return Center(child: Text('Error: $error')); + }, + ); + } +} + +class _ArtefactReviewPopOver extends StatelessWidget { + const _ArtefactReviewPopOver({ + required this.artefact, + required this.artefactBuilds, + }); + final Artefact artefact; + final List artefactBuilds; + + @override + Widget build(BuildContext context) { + if (artefactBuilds.any( + (build) => build.testExecutions.any( + (execution) => + execution.reviewStatus == + TestExecutionReviewStatus.markedAsFailed || + execution.reviewStatus == TestExecutionReviewStatus.undecided, + ), + )) { + return _DisabledArtefactReviewPopOver(); + } else { + return _EnabledArtefactReviewPopOver(artefactName: artefact.name); + } + } +} + +class _EnabledArtefactReviewPopOver extends StatelessWidget { + const _EnabledArtefactReviewPopOver({ + required this.artefactName, + }); + final String artefactName; + @override + Widget build(BuildContext context) { + return Padding( + padding: const EdgeInsets.symmetric(vertical: 8), + child: ListView( + padding: const EdgeInsets.all(8), + children: [ + RichText( + textAlign: TextAlign.center, + text: TextSpan( + style: Theme.of(context).textTheme.titleMedium, + children: [ + const TextSpan( + text: 'You are about to sign-off the ', + ), + TextSpan( + text: artefactName, + style: const TextStyle(fontWeight: FontWeight.bold), + ), + const TextSpan( + text: ' artefact. Please confirm this action below.', + ), + ], + ), + ), + const Padding( + padding: EdgeInsets.all(Spacing.level4), + child: ElevatedButton( + onPressed: null, + child: Text('Approve Artefact'), + ), + ), + ], + ), + ); + } +} + +class _DisabledArtefactReviewPopOver extends StatelessWidget { + @override + Widget build(BuildContext context) { + return Padding( + padding: const EdgeInsets.symmetric(vertical: 8), + child: ListView( + padding: const EdgeInsets.all(8), + children: [ + Text( + 'At least one Test Execution is marked as failed or undecided. Please approve all individual test executions before approving the whole artefact.', + style: Theme.of(context).textTheme.titleMedium, + textAlign: TextAlign.center, + ), + ], + ), + ); + } +} diff --git a/frontend/lib/ui/dashboard/test_execution_review_pop_over.dart b/frontend/lib/ui/dashboard/test_execution_review_pop_over.dart deleted file mode 100644 index f68d1ca7..00000000 --- a/frontend/lib/ui/dashboard/test_execution_review_pop_over.dart +++ /dev/null @@ -1,69 +0,0 @@ -import 'package:flutter/material.dart'; -import 'package:popover/popover.dart'; - -import '../../models/test_execution.dart'; - -class TestExecutionReviewButton extends StatelessWidget { - const TestExecutionReviewButton({super.key, required this.testExecution}); - - final TestExecution testExecution; - - @override - Widget build(BuildContext context) { - return Padding( - padding: const EdgeInsets.all(4), - child: ElevatedButton( - onPressed: () { - showPopover( - context: context, - bodyBuilder: (context) => _PopOver( - testExecution: testExecution, - ), - direction: PopoverDirection.bottom, - width: 500, - height: 300, - arrowHeight: 15, - arrowWidth: 30, - ); - }, - child: const Text('Review'), - ), - ); - } -} - -class _PopOver extends StatelessWidget { - const _PopOver({required this.testExecution}); - - final TestExecution testExecution; - - @override - Widget build(BuildContext context) { - return Padding( - padding: const EdgeInsets.symmetric(vertical: 8), - child: ListView( - padding: const EdgeInsets.all(8), - children: [ - Text( - 'Select new review status', - style: Theme.of(context).textTheme.titleLarge, - ), - Text( - 'Insert review comments:', - style: Theme.of(context).textTheme.titleLarge, - ), - const TextField( - decoration: InputDecoration( - border: OutlineInputBorder(), - hintText: 'Insert review comments', - ), - ), - const ElevatedButton( - onPressed: null, - child: Text('Submit Review'), - ), - ], - ), - ); - } -} From 0481cf7a20d4796cfb35e43a2083fe9575b748b9 Mon Sep 17 00:00:00 2001 From: Andrej Velichkovski Date: Wed, 6 Dec 2023 14:48:33 +0100 Subject: [PATCH 6/6] Add backend updates --- ...889ba2349ffa_add_review_status_and_comment.py} | 12 ++++++------ .../controllers/test_executions/models.py | 3 +++ .../test_executions/test_executions.py | 15 +++++++++++++++ 3 files changed, 24 insertions(+), 6 deletions(-) rename backend/migrations/versions/{2023_12_04_1357-f45b1594fb23_migration_description.py => 2023_12_06_1316-889ba2349ffa_add_review_status_and_comment.py} (80%) diff --git a/backend/migrations/versions/2023_12_04_1357-f45b1594fb23_migration_description.py b/backend/migrations/versions/2023_12_06_1316-889ba2349ffa_add_review_status_and_comment.py similarity index 80% rename from backend/migrations/versions/2023_12_04_1357-f45b1594fb23_migration_description.py rename to backend/migrations/versions/2023_12_06_1316-889ba2349ffa_add_review_status_and_comment.py index 08c3728a..daea7853 100644 --- a/backend/migrations/versions/2023_12_04_1357-f45b1594fb23_migration_description.py +++ b/backend/migrations/versions/2023_12_06_1316-889ba2349ffa_add_review_status_and_comment.py @@ -1,8 +1,8 @@ -"""Add test execution review status and comment +"""Add review status and comment -Revision ID: f45b1594fb23 -Revises: 8317277d4333 -Create Date: 2023-12-04 13:57:23.385250+00:00 +Revision ID: 889ba2349ffa +Revises: 871eec26dc90 +Create Date: 2023-12-06 13:16:07.171288+00:00 """ from alembic import op @@ -10,8 +10,8 @@ # revision identifiers, used by Alembic. -revision = "f45b1594fb23" -down_revision = "8317277d4333" +revision = '889ba2349ffa' +down_revision = '871eec26dc90' branch_labels = None depends_on = None diff --git a/backend/test_observer/controllers/test_executions/models.py b/backend/test_observer/controllers/test_executions/models.py index 7e5c5106..21f1e1d2 100644 --- a/backend/test_observer/controllers/test_executions/models.py +++ b/backend/test_observer/controllers/test_executions/models.py @@ -88,5 +88,8 @@ class TestExecutionsPatchRequest(BaseModel): c3_link: HttpUrl | None ci_link: HttpUrl | None status: TestExecutionStatus | None + + +class TestExecutionsReviewPatchRequest(BaseModel): review_status: TestExecutionReviewStatus | None review_comment: str | None diff --git a/backend/test_observer/controllers/test_executions/test_executions.py b/backend/test_observer/controllers/test_executions/test_executions.py index ca7111ac..5dc5638b 100644 --- a/backend/test_observer/controllers/test_executions/test_executions.py +++ b/backend/test_observer/controllers/test_executions/test_executions.py @@ -38,6 +38,7 @@ EndTestExecutionRequest, StartTestExecutionRequest, TestExecutionsPatchRequest, + TestExecutionsReviewPatchRequest, ) router = APIRouter() @@ -144,6 +145,20 @@ def patch_test_execution( if request.status is not None: test_execution.status = request.status + db.commit() + + +@router.patch("/{id}/review") +def review_test_execution( + id: int, + request: TestExecutionsReviewPatchRequest, + db: Session = Depends(get_db), +): + test_execution = db.get(TestExecution, id) + + if test_execution is None: + raise HTTPException(status_code=404, detail="TestExecution not found") + if request.review_status is not None: test_execution.review_status = request.review_status