RTW-313: Implement percentage completed UI (#189)

* Initial implementation of UI for showing percentage completed

* Fix error when loading artefact builds

* Update code to show exact counts

* Fix import issues

* Fix mypy errors

* Fix issue with tests

* Add doc-string to new helper methods

* Implement new logic for fetching

* Fix ruff issue

* Remove unnecessary changes on frontend

* Small improvements

* Fix user avatar

* Implement PR suggestions

* Fix automated tests

* Fix mypy errors

backend/test_observer/controllers/artefacts/artefacts.py

@@ -44,7 +44,14 @@
 
 
 def _get_artefact_from_db(artefact_id: int, db: Session = Depends(get_db)) -> Artefact:
-    a = db.get(Artefact, artefact_id)
+    a = (
+        db.query(Artefact)
+        .options(
+            joinedload(Artefact.builds).joinedload(ArtefactBuild.test_executions),
+        )
+        .filter(Artefact.id == artefact_id)
+        .one_or_none()
+    )
     if a is None:
         msg = f"Artefact with id {artefact_id} not found"
         raise HTTPException(status_code=404, detail=msg)
@@ -62,6 +69,7 @@ def get_artefacts(family: FamilyName | None = None, db: Session = Depends(get_db
             db,
             family,
             load_stage=True,
+            load_test_executions=True,
             order_by_columns=order_by,
         )
     else:
@@ -70,14 +78,17 @@ def get_artefacts(family: FamilyName | None = None, db: Session = Depends(get_db
                 db,
                 family,
                 load_stage=True,
+                load_test_executions=True,
                 order_by_columns=order_by,
             )
 
     return artefacts
 
 
 @router.get("/{artefact_id}", response_model=ArtefactDTO)
-def get_artefact(artefact: Artefact = Depends(_get_artefact_from_db)):
+def get_artefact(
+    artefact: Artefact = Depends(_get_artefact_from_db),
+):
     return artefact
 
 

backend/test_observer/controllers/artefacts/models.py

@@ -30,6 +30,8 @@
 
 
 class UserDTO(BaseModel):
+    model_config = ConfigDict(from_attributes=True)
+
     id: int
     launchpad_handle: str
     launchpad_email: str
@@ -51,6 +53,8 @@ class ArtefactDTO(BaseModel):
     assignee: UserDTO | None
     due_date: date | None
     bug_link: str
+    all_test_executions_count: int
+    completed_test_executions_count: int
 
 
 class EnvironmentDTO(BaseModel):

backend/test_observer/data_access/models.py

@@ -18,6 +18,7 @@
 #        Nadzeya Hutsko <nadzeya.hutsko@canonical.com>
 #        Omar Selo <omar.selo@canonical.com>
 
+from collections import defaultdict
 from datetime import date, datetime, timedelta
 from typing import TypeVar
 
@@ -207,6 +208,28 @@ def is_kernel(self) -> bool:
         """Kernel artefacts start with 'linix-' or end with '-kernel'"""
         return self.name.startswith("linux-") or self.name.endswith("-kernel")
 
+    def _get_latest_builds(self) -> list["ArtefactBuild"]:
+        # Group builds by architecture
+        grouped_builds = defaultdict(list)
+        for build in self.builds:
+            grouped_builds[build.architecture].append(build)
+
+        return [
+            max(builds, key=lambda b: b.revision if b.revision else 0)
+            for builds in grouped_builds.values()
+        ]
+
+    @property
+    def all_test_executions_count(self) -> int:
+        return sum(len(ab.test_executions) for ab in self._get_latest_builds())
+
+    @property
+    def completed_test_executions_count(self) -> int:
+        return sum(
+            len([te for te in ab.test_executions if te.review_decision])
+            for ab in self._get_latest_builds()
+        )
+
 
 class ArtefactBuild(Base):
     """A model to represent specific builds of artefact (e.g. arm64 revision 2)"""

backend/test_observer/data_access/repository.py

@@ -27,7 +27,7 @@
 from sqlalchemy.exc import IntegrityError
 from sqlalchemy.orm import Session, joinedload
 
-from .models import Artefact, DataModel, Family, Stage
+from .models import Artefact, ArtefactBuild, DataModel, Family, Stage
 from .models_enums import FamilyName
 
 
@@ -55,6 +55,7 @@ def get_artefacts_by_family(
     family_name: FamilyName,
     latest_only: bool = True,
     load_stage: bool = False,
+    load_test_executions: bool = False,
     order_by_columns: Iterable[Any] | None = None,
 ) -> list[Artefact]:
     """
@@ -126,6 +127,11 @@ def get_artefacts_by_family(
     if load_stage:
         query = query.options(joinedload(Artefact.stage))
 
+    if load_test_executions:
+        query = query.options(
+            joinedload(Artefact.builds).joinedload(ArtefactBuild.test_executions)
+        )
+
     if order_by_columns:
         query = query.order_by(*order_by_columns)
 

backend/tests/controllers/artefacts/test_artefacts.py

@@ -19,6 +19,8 @@
 #        Nadzeya Hutsko <nadzeya.hutsko@canonical.com>
 from datetime import date, timedelta
 
+from sqlalchemy.orm import Session
+
 from fastapi.testclient import TestClient
 
 from test_observer.data_access.models import TestExecution
@@ -64,6 +66,8 @@ def test_get_latest_artefacts_by_family(
                 else None
             ),
             "bug_link": "",
+            "all_test_executions_count": 0,
+            "completed_test_executions_count": 0,
         }
     ]
 
@@ -100,9 +104,62 @@ def test_get_artefact(test_client: TestClient, generator: DataGenerator):
         },
         "due_date": "2024-12-24",
         "bug_link": a.bug_link,
+        "all_test_executions_count": 0,
+        "completed_test_executions_count": 0,
     }
 
 
+def test_get_artefact_test_execution_counts_only_latest_build(
+    test_client: TestClient, generator: DataGenerator
+):
+    a = generator.gen_artefact("beta")
+    ab = generator.gen_artefact_build(artefact=a, revision=1)
+    e = generator.gen_environment()
+    # Test Execution for the first artefact build
+    generator.gen_test_execution(ab, e)
+
+    ab_second = generator.gen_artefact_build(artefact=a, revision=2)
+    # Test Execution for the second artefact build
+    generator.gen_test_execution(
+        artefact_build=ab_second,
+        environment=e,
+        review_decision=[TestExecutionReviewDecision.APPROVED_ALL_TESTS_PASS],
+    )
+
+    response = test_client.get(f"/v1/artefacts/{a.id}")
+    assert response.status_code == 200
+    # Verify only the counts of the latest build is returned
+    assert response.json()["all_test_executions_count"] == 1
+    assert response.json()["completed_test_executions_count"] == 1
+
+
+def test_get_artefact_test_execution_counts(
+    test_client: TestClient,
+    generator: DataGenerator,
+    db_session: Session,
+):
+    a = generator.gen_artefact("beta")
+    ab = generator.gen_artefact_build(a)
+    e = generator.gen_environment()
+    te = generator.gen_test_execution(ab, e)
+
+    # Verify completed test execution count is zero, it is not reviewed yet
+    response = test_client.get(f"/v1/artefacts/{a.id}")
+    assert response.status_code == 200
+    assert response.json()["all_test_executions_count"] == 1
+    assert response.json()["completed_test_executions_count"] == 0
+
+    te.review_decision = [TestExecutionReviewDecision.APPROVED_ALL_TESTS_PASS]
+    db_session.commit()
+    db_session.refresh(te)
+
+    # Verify completed test execution count is one, it is reviewed
+    response = test_client.get(f"/v1/artefacts/{a.id}")
+    assert response.status_code == 200
+    assert response.json()["all_test_executions_count"] == 1
+    assert response.json()["completed_test_executions_count"] == 1
+
+
 def test_get_artefact_builds(test_client: TestClient, generator: DataGenerator):
     a = generator.gen_artefact("beta")
     ab = generator.gen_artefact_build(a)
@@ -268,6 +325,8 @@ def test_artefact_signoff_approve(test_client: TestClient, generator: DataGenera
             artefact.due_date.strftime("%Y-%m-%d") if artefact.due_date else None
         ),
         "bug_link": "",
+        "all_test_executions_count": 0,
+        "completed_test_executions_count": 0,
     }
 
 

frontend/benchmarks/common.dart

@@ -51,6 +51,8 @@ class ApiRepositoryMock extends Mock implements ApiRepository {
       status: ArtefactStatus.undecided,
       stage: StageName.beta,
       bugLink: '',
+      allTestExecutionsCount: 1,
+      completedTestExecutionsCount: 0,
     );
 
     return {

frontend/lib/models/artefact.dart

@@ -22,6 +22,10 @@ class Artefact with _$Artefact {
     required String repo,
     required ArtefactStatus status,
     required StageName stage,
+    @JsonKey(name: 'all_test_executions_count')
+    required int allTestExecutionsCount,
+    @JsonKey(name: 'completed_test_executions_count')
+    required int completedTestExecutionsCount,
     User? assignee,
     @JsonKey(name: 'bug_link') required String bugLink,
     @JsonKey(name: 'due_date') DateTime? dueDate,

frontend/lib/providers/family_artefacts.dart

@@ -24,4 +24,18 @@ class FamilyArtefacts extends _$FamilyArtefacts {
     final previousState = await future;
     state = AsyncData({...previousState, artefact.id: artefact});
   }
+
+  Future<void> updateCompletedTestExecutionsCount(
+    int artefactId,
+    int count,
+  ) async {
+    final previousState = await future;
+    final artefact = previousState[artefactId];
+    if (artefact != null) {
+      state = AsyncData({
+        ...previousState,
+        artefactId: artefact.copyWith(completedTestExecutionsCount: count),
+      });
+    }
+  }
 }

frontend/lib/providers/review_test_execution.dart

@@ -0,0 +1,50 @@
+import 'package:riverpod_annotation/riverpod_annotation.dart';
+
+import '../models/family_name.dart';
+import '../models/test_execution.dart';
+import 'artefact_builds.dart';
+import 'family_artefacts.dart';
+
+part 'review_test_execution.g.dart';
+
+@riverpod
+class ReviewTestExecution extends _$ReviewTestExecution {
+  @override
+  Future<void> build() async {
+    return;
+  }
+
+  Future<void> reviewTestExecution(
+    int testExecutionId,
+    String reviewComment,
+    List<TestExecutionReviewDecision> reviewDecision,
+    FamilyName familyName,
+    int artefactId,
+  ) async {
+    await ref
+        .read(artefactBuildsProvider(artefactId).notifier)
+        .changeReviewDecision(
+          testExecutionId,
+          reviewComment,
+          reviewDecision,
+        );
+
+    final artefactBuilds =
+        ref.read(artefactBuildsProvider(artefactId)).requireValue;
+
+    final newCompletedTestExecutionsCount = artefactBuilds
+        .map(
+          (build) => build.testExecutions
+              .where((testExecution) => testExecution.reviewDecision.isNotEmpty)
+              .length,
+        )
+        .fold(0, (a, b) => a + b);
+
+    await ref
+        .read(familyArtefactsProvider(familyName).notifier)
+        .updateCompletedTestExecutionsCount(
+          artefactId,
+          newCompletedTestExecutionsCount,
+        );
+  }
+}

frontend/lib/ui/artefact_page/artefact_page_header.dart

@@ -15,13 +15,19 @@ class ArtefactPageHeader extends StatelessWidget {
   Widget build(BuildContext context) {
     final assignee = artefact.assignee;
     final dueDate = artefact.dueDateString;
+
     return Row(
       children: [
         Text(artefact.name, style: Theme.of(context).textTheme.headlineLarge),
         const SizedBox(width: Spacing.level4),
         ArtefactSignoffButton(artefact: artefact),
         const SizedBox(width: Spacing.level4),
-        if (assignee != null) UserAvatar(user: assignee),
+        if (assignee != null)
+          UserAvatar(
+            user: assignee,
+            allTestExecutionsCount: artefact.allTestExecutionsCount,
+            completedTestExecutionsCount: artefact.completedTestExecutionsCount,
+          ),
         const SizedBox(width: Spacing.level4),
         if (dueDate != null)
           Text(

frontend/lib/ui/artefact_page/test_execution_pop_over.dart

@@ -2,19 +2,22 @@ import 'package:flutter/material.dart';
 import 'package:flutter_riverpod/flutter_riverpod.dart';
 import 'package:yaru_widgets/yaru_widgets.dart';
 
+import '../../models/family_name.dart';
 import '../../models/test_execution.dart';
-import '../../providers/artefact_builds.dart';
+import '../../providers/review_test_execution.dart';
 import '../spacing.dart';
 
 class TestExecutionPopOver extends ConsumerStatefulWidget {
   const TestExecutionPopOver({
     super.key,
     required this.testExecution,
     required this.artefactId,
+    required this.family,
   });
 
   final TestExecution testExecution;
   final int artefactId;
+  final FamilyName family;
 
   @override
   TestExecutionPopOverState createState() => TestExecutionPopOverState();
@@ -116,12 +119,12 @@ class TestExecutionPopOverState extends ConsumerState<TestExecutionPopOver> {
         const SizedBox(height: Spacing.level3),
         ElevatedButton(
           onPressed: () {
-            ref
-                .read(ArtefactBuildsProvider(widget.artefactId).notifier)
-                .changeReviewDecision(
+            ref.read(reviewTestExecutionProvider.notifier).reviewTestExecution(
                   widget.testExecution.id,
                   reviewCommentController.text,
                   reviewDecisions,
+                  widget.family,
+                  widget.artefactId,
                 );
             Navigator.pop(context);
           },

frontend/lib/ui/artefact_page/test_execution_review.dart

@@ -37,6 +37,7 @@ class TestExecutionReviewButton extends StatelessWidget {
 
   @override
   Widget build(BuildContext context) {
+    final family = AppRoutes.familyFromUri(AppRoutes.uriFromContext(context));
     final artefactId =
         AppRoutes.artefactIdFromUri(AppRoutes.uriFromContext(context));
     return GestureDetector(
@@ -46,6 +47,7 @@ class TestExecutionReviewButton extends StatelessWidget {
           bodyBuilder: (context) => TestExecutionPopOver(
             testExecution: testExecution,
             artefactId: artefactId,
+            family: family,
           ),
           direction: PopoverDirection.bottom,
           width: 500,

frontend/lib/ui/dashboard/artefact_card.dart

@@ -63,7 +63,13 @@ class ArtefactCard extends ConsumerWidget {
                       fontColor: YaruColors.red,
                     ),
                   const Spacer(),
-                  if (assignee != null) UserAvatar(user: assignee),
+                  if (assignee != null)
+                    UserAvatar(
+                      user: assignee,
+                      allTestExecutionsCount: artefact.allTestExecutionsCount,
+                      completedTestExecutionsCount:
+                          artefact.completedTestExecutionsCount,
+                    ),
                 ],
               ),
             ],