From d877415377455bf852f5376116e9881ab47047d1 Mon Sep 17 00:00:00 2001 From: Omar Abou Selo Date: Wed, 18 Sep 2024 12:19:52 +0300 Subject: [PATCH] Reporting environment issues (#213) * Add missing field from repr * Add support for environment issues to the backend * Require issue url be github, jira, or launchpad * Add is_confirmed to environment issue * Include environment issues in seed_script * Fix seed script * Environment issues frontend * Minor improvements * Some code improvements * Add non-blocking provider preloader * Refactor loading providers to a widget * Fix missing event log expandable --- ...505b96fd7731_add_environmentissue_table.py | 33 +++ backend/scripts/seed_data.py | 45 ++- backend/test_observer/common/constants.py | 2 + .../controllers/environments/__init__.py | 6 + .../controllers/environments/models.py | 31 ++ .../environments/reported_issues.py | 52 ++++ backend/test_observer/controllers/router.py | 3 +- .../controllers/test_cases/models.py | 18 +- .../controllers/test_cases/reported_issues.py | 14 +- backend/test_observer/data_access/models.py | 22 ++ backend/tests/asserts.py | 8 + .../environments/test_environment_issues.py | 115 ++++++++ .../test_cases/test_reported_issues.py | 27 +- frontend/lib/constants.dart | 5 + frontend/lib/helpers.dart | 16 ++ frontend/lib/models/environment_issue.dart | 18 ++ .../lib/providers/environments_issues.dart | 52 ++++ frontend/lib/repositories/api_repository.dart | 39 +++ .../lib/ui/artefact_page/artefact_page.dart | 44 ++- .../ui/artefact_page/artefact_page_body.dart | 26 +- .../ui/artefact_page/artefact_page_side.dart | 23 +- .../environment_issue_form.dart | 268 ++++++++++++++++++ .../environment_issue_list_item.dart | 55 ++++ .../environment_issues_expandable.dart | 51 ++++ .../test_event_log_expandable.dart | 20 +- .../test_execution_expandable.dart | 2 + .../test_issues/test_issue_form.dart | 11 +- .../test_issues/test_issues_preloader.dart | 16 -- .../test_result_filter_expandable.dart | 15 +- .../lib/ui/blocking_provider_preloader.dart | 26 ++ frontend/lib/ui/dashboard/dashboard.dart | 25 +- .../dashboard_body/dashboard_body.dart | 15 +- .../dashboard_header/view_mode_toggle.dart | 28 +- .../ui/non_blocking_provider_preloader.dart | 19 ++ 34 files changed, 980 insertions(+), 170 deletions(-) create mode 100644 backend/migrations/versions/2024_09_16_1052-505b96fd7731_add_environmentissue_table.py create mode 100644 backend/test_observer/controllers/environments/__init__.py create mode 100644 backend/test_observer/controllers/environments/models.py create mode 100644 backend/test_observer/controllers/environments/reported_issues.py create mode 100644 backend/tests/asserts.py create mode 100644 backend/tests/controllers/environments/test_environment_issues.py create mode 100644 frontend/lib/constants.dart create mode 100644 frontend/lib/helpers.dart create mode 100644 frontend/lib/models/environment_issue.dart create mode 100644 frontend/lib/providers/environments_issues.dart create mode 100644 frontend/lib/ui/artefact_page/environment_issues/environment_issue_form.dart create mode 100644 frontend/lib/ui/artefact_page/environment_issues/environment_issue_list_item.dart create mode 100644 frontend/lib/ui/artefact_page/environment_issues/environment_issues_expandable.dart delete mode 100644 frontend/lib/ui/artefact_page/test_issues/test_issues_preloader.dart create mode 100644 frontend/lib/ui/blocking_provider_preloader.dart create mode 100644 frontend/lib/ui/non_blocking_provider_preloader.dart diff --git a/backend/migrations/versions/2024_09_16_1052-505b96fd7731_add_environmentissue_table.py b/backend/migrations/versions/2024_09_16_1052-505b96fd7731_add_environmentissue_table.py new file mode 100644 index 00000000..5828bc3a --- /dev/null +++ b/backend/migrations/versions/2024_09_16_1052-505b96fd7731_add_environmentissue_table.py @@ -0,0 +1,33 @@ +"""Add EnvironmentIssue table + +Revision ID: 505b96fd7731 +Revises: ba6550a03bc8 +Create Date: 2024-09-16 10:52:25.226261+00:00 + +""" +import sqlalchemy as sa +from alembic import op + +# revision identifiers, used by Alembic. +revision = "505b96fd7731" +down_revision = "ba6550a03bc8" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + op.create_table( + "environment_issue", + sa.Column("environment_name", sa.String(), nullable=False), + sa.Column("url", sa.String(), nullable=False), + sa.Column("description", sa.String(), nullable=False), + sa.Column("is_confirmed", sa.Boolean(), nullable=False), + sa.Column("id", sa.Integer(), autoincrement=True, nullable=False), + sa.Column("created_at", sa.DateTime(), nullable=False), + sa.Column("updated_at", sa.DateTime(), nullable=False), + sa.PrimaryKeyConstraint("id", name=op.f("environment_issue_pkey")), + ) + + +def downgrade() -> None: + op.drop_table("environment_issue") diff --git a/backend/scripts/seed_data.py b/backend/scripts/seed_data.py index 38d28109..7a41a845 100644 --- a/backend/scripts/seed_data.py +++ b/backend/scripts/seed_data.py @@ -11,7 +11,10 @@ from sqlalchemy import select from sqlalchemy.orm import Session -from test_observer.controllers.test_cases.models import ReportedIssueRequest +from test_observer.controllers.environments.models import ( + EnvironmentReportedIssueRequest, +) +from test_observer.controllers.test_cases.models import TestReportedIssueRequest from test_observer.controllers.test_executions.models import ( C3TestResult, C3TestResultStatus, @@ -28,6 +31,7 @@ END_TEST_EXECUTION_URL = f"{BASE_URL}/test-executions/end-test" RERUN_TEST_EXECUTION_URL = f"{BASE_URL}/test-executions/reruns" TEST_CASE_ISSUE_URL = f"{BASE_URL}/test-cases/reported-issues" +ENVIRONMENT_ISSUE_URL = f"{BASE_URL}/environments/reported-issues" START_TEST_EXECUTION_REQUESTS = [ StartTestExecutionRequest( @@ -288,20 +292,41 @@ ] TEST_CASE_ISSUE_REQUESTS = [ - ReportedIssueRequest( + TestReportedIssueRequest( template_id=END_TEST_EXECUTION_REQUESTS[0].test_results[2].template_id, # type: ignore - url=HttpUrl("http://bug1.link"), + url=HttpUrl("https://github.com"), description="known issue 1", ), - ReportedIssueRequest( + TestReportedIssueRequest( case_name=END_TEST_EXECUTION_REQUESTS[0].test_results[0].name, - url=HttpUrl("http://bug2.link"), + url=HttpUrl("https://warthogs.atlassian.net"), description="known issue 2", ), - ReportedIssueRequest( + TestReportedIssueRequest( case_name=END_TEST_EXECUTION_REQUESTS[0].test_results[1].name, - url=HttpUrl("http://bug3.link"), + url=HttpUrl("https://bugs.launchpad.net"), + description="known issue 3", + ), +] + +ENVIRONMENT_ISSUE_REQUESTS = [ + EnvironmentReportedIssueRequest( + environment_name=START_TEST_EXECUTION_REQUESTS[0].environment, + url=HttpUrl("https://github.com"), + description="known issue 1", + is_confirmed=True, + ), + EnvironmentReportedIssueRequest( + environment_name=START_TEST_EXECUTION_REQUESTS[1].environment, + url=HttpUrl("https://warthogs.atlassian.net"), + description="known issue 2", + is_confirmed=False, + ), + EnvironmentReportedIssueRequest( + environment_name=START_TEST_EXECUTION_REQUESTS[2].environment, + url=HttpUrl("https://bugs.launchpad.net"), description="known issue 3", + is_confirmed=True, ), ] @@ -334,6 +359,12 @@ def seed_data(client: TestClient | requests.Session, session: Session | None = N TEST_CASE_ISSUE_URL, json=case_issue_request.model_dump(mode="json") ).raise_for_status() + for environment_issue_request in ENVIRONMENT_ISSUE_REQUESTS: + client.post( + ENVIRONMENT_ISSUE_URL, + json=environment_issue_request.model_dump(mode="json"), + ).raise_for_status() + _rerun_some_test_executions(client, test_executions) _add_bugurl_and_duedate(session) diff --git a/backend/test_observer/common/constants.py b/backend/test_observer/common/constants.py index 1ab58f6b..15c32ec2 100644 --- a/backend/test_observer/common/constants.py +++ b/backend/test_observer/common/constants.py @@ -1 +1,3 @@ PREVIOUS_TEST_RESULT_COUNT = 10 + +VALID_ISSUE_HOSTS = {"github.com", "warthogs.atlassian.net", "bugs.launchpad.net"} diff --git a/backend/test_observer/controllers/environments/__init__.py b/backend/test_observer/controllers/environments/__init__.py new file mode 100644 index 00000000..90ff6009 --- /dev/null +++ b/backend/test_observer/controllers/environments/__init__.py @@ -0,0 +1,6 @@ +from fastapi import APIRouter + +from . import reported_issues + +router = APIRouter(tags=["environments"]) +router.include_router(reported_issues.router) diff --git a/backend/test_observer/controllers/environments/models.py b/backend/test_observer/controllers/environments/models.py new file mode 100644 index 00000000..631a4feb --- /dev/null +++ b/backend/test_observer/controllers/environments/models.py @@ -0,0 +1,31 @@ +from datetime import datetime + +from pydantic import BaseModel, HttpUrl, field_validator + +from test_observer.common.constants import VALID_ISSUE_HOSTS + + +class EnvironmentReportedIssueRequest(BaseModel): + environment_name: str + description: str + url: HttpUrl + is_confirmed: bool + + @field_validator("url") + @classmethod + def url_host_must_be_allowed( + cls: type["EnvironmentReportedIssueRequest"], url: HttpUrl + ) -> HttpUrl: + if url.host not in VALID_ISSUE_HOSTS: + raise ValueError(f"Issue url must belong to one of {VALID_ISSUE_HOSTS}") + return url + + +class EnvironmentReportedIssueResponse(BaseModel): + id: int + environment_name: str + description: str + url: HttpUrl + is_confirmed: bool + created_at: datetime + updated_at: datetime diff --git a/backend/test_observer/controllers/environments/reported_issues.py b/backend/test_observer/controllers/environments/reported_issues.py new file mode 100644 index 00000000..c56a2e28 --- /dev/null +++ b/backend/test_observer/controllers/environments/reported_issues.py @@ -0,0 +1,52 @@ +from fastapi import APIRouter, Depends +from sqlalchemy import select +from sqlalchemy.orm import Session + +from test_observer.data_access.models import EnvironmentIssue +from test_observer.data_access.setup import get_db + +from .models import EnvironmentReportedIssueRequest, EnvironmentReportedIssueResponse + +router = APIRouter() + +endpoint = "/reported-issues" + + +@router.get(endpoint, response_model=list[EnvironmentReportedIssueResponse]) +def get_reported_issues(db: Session = Depends(get_db)): + return db.execute(select(EnvironmentIssue)).scalars() + + +@router.post(endpoint, response_model=EnvironmentReportedIssueResponse) +def create_reported_issue( + request: EnvironmentReportedIssueRequest, db: Session = Depends(get_db) +): + issue = EnvironmentIssue( + environment_name=request.environment_name, + url=request.url, + description=request.description, + is_confirmed=request.is_confirmed, + ) + db.add(issue) + db.commit() + + return issue + + +@router.put(endpoint + "/{issue_id}", response_model=EnvironmentReportedIssueResponse) +def update_reported_issue( + issue_id: int, + request: EnvironmentReportedIssueRequest, + db: Session = Depends(get_db), +): + issue = db.get(EnvironmentIssue, issue_id) + for field in request.model_fields: + setattr(issue, field, getattr(request, field)) + db.commit() + return issue + + +@router.delete(endpoint + "/{issue_id}") +def delete_reported_issue(issue_id: int, db: Session = Depends(get_db)): + db.delete(db.get(EnvironmentIssue, issue_id)) + db.commit() diff --git a/backend/test_observer/controllers/router.py b/backend/test_observer/controllers/router.py index 37c87a41..02819be6 100644 --- a/backend/test_observer/controllers/router.py +++ b/backend/test_observer/controllers/router.py @@ -23,7 +23,7 @@ from test_observer.data_access.setup import get_db -from . import test_cases, test_executions +from . import environments, test_cases, test_executions from .application import version from .artefacts import artefacts from .reports import reports @@ -34,6 +34,7 @@ router.include_router(artefacts.router, prefix="/v1/artefacts") router.include_router(reports.router, prefix="/v1/reports") router.include_router(test_cases.router, prefix="/v1/test-cases") +router.include_router(environments.router, prefix="/v1/environments") @router.get("/") diff --git a/backend/test_observer/controllers/test_cases/models.py b/backend/test_observer/controllers/test_cases/models.py index 78c3fb89..3052a00b 100644 --- a/backend/test_observer/controllers/test_cases/models.py +++ b/backend/test_observer/controllers/test_cases/models.py @@ -1,9 +1,11 @@ from datetime import datetime -from pydantic import BaseModel, HttpUrl, model_validator +from pydantic import BaseModel, HttpUrl, field_validator, model_validator +from test_observer.common.constants import VALID_ISSUE_HOSTS -class ReportedIssueRequest(BaseModel): + +class TestReportedIssueRequest(BaseModel): template_id: str = "" case_name: str = "" description: str @@ -13,10 +15,20 @@ class ReportedIssueRequest(BaseModel): def check_a_or_b(self): if not self.case_name and not self.template_id: raise ValueError("Either case_name or template_id is required") + return self + @field_validator("url") + @classmethod + def name_must_contain_space( + cls: type["TestReportedIssueRequest"], url: HttpUrl + ) -> HttpUrl: + if url.host not in VALID_ISSUE_HOSTS: + raise ValueError(f"Issue url must belong to one of {VALID_ISSUE_HOSTS}") + return url + -class ReportedIssueResponse(BaseModel): +class TestReportedIssueResponse(BaseModel): id: int template_id: str = "" case_name: str = "" diff --git a/backend/test_observer/controllers/test_cases/reported_issues.py b/backend/test_observer/controllers/test_cases/reported_issues.py index 95966655..3f675e01 100644 --- a/backend/test_observer/controllers/test_cases/reported_issues.py +++ b/backend/test_observer/controllers/test_cases/reported_issues.py @@ -5,7 +5,7 @@ from test_observer.data_access.models import TestCaseIssue from test_observer.data_access.setup import get_db -from .models import ReportedIssueRequest, ReportedIssueResponse +from .models import TestReportedIssueRequest, TestReportedIssueResponse router = APIRouter() @@ -13,7 +13,7 @@ endpoint = "/reported-issues" -@router.get(endpoint, response_model=list[ReportedIssueResponse]) +@router.get(endpoint, response_model=list[TestReportedIssueResponse]) def get_reported_issues( template_id: str | None = None, case_name: str | None = None, @@ -27,8 +27,10 @@ def get_reported_issues( return db.execute(stmt).scalars() -@router.post(endpoint, response_model=ReportedIssueResponse) -def create_reported_issue(request: ReportedIssueRequest, db: Session = Depends(get_db)): +@router.post(endpoint, response_model=TestReportedIssueResponse) +def create_reported_issue( + request: TestReportedIssueRequest, db: Session = Depends(get_db) +): issue = TestCaseIssue( template_id=request.template_id, url=request.url, @@ -41,9 +43,9 @@ def create_reported_issue(request: ReportedIssueRequest, db: Session = Depends(g return issue -@router.put(endpoint + "/{issue_id}", response_model=ReportedIssueResponse) +@router.put(endpoint + "/{issue_id}", response_model=TestReportedIssueResponse) def update_reported_issue( - issue_id: int, request: ReportedIssueRequest, db: Session = Depends(get_db) + issue_id: int, request: TestReportedIssueRequest, db: Session = Depends(get_db) ): issue = db.get(TestCaseIssue, issue_id) for field in request.model_fields: diff --git a/backend/test_observer/data_access/models.py b/backend/test_observer/data_access/models.py index 33f4b6da..ab805c82 100644 --- a/backend/test_observer/data_access/models.py +++ b/backend/test_observer/data_access/models.py @@ -474,6 +474,28 @@ def __repr__(self) -> str: return data_model_repr( self, "template_id", + "case_name", + "url", + "description", + ) + + +class EnvironmentIssue(Base): + """ + A table to store issues reported on certain environments + """ + + __tablename__ = "environment_issue" + + environment_name: Mapped[str] + url: Mapped[str] + description: Mapped[str] + is_confirmed: Mapped[bool] + + def __repr__(self) -> str: + return data_model_repr( + self, + "environment_name", "url", "description", ) diff --git a/backend/tests/asserts.py b/backend/tests/asserts.py new file mode 100644 index 00000000..2a820688 --- /dev/null +++ b/backend/tests/asserts.py @@ -0,0 +1,8 @@ +from httpx import Response + + +def assert_fails_validation(response: Response, field: str, type: str) -> None: + assert response.status_code == 422 + problem = response.json()["detail"][0] + assert problem["type"] == type + assert problem["loc"] == ["body", field] diff --git a/backend/tests/controllers/environments/test_environment_issues.py b/backend/tests/controllers/environments/test_environment_issues.py new file mode 100644 index 00000000..7a37d5b4 --- /dev/null +++ b/backend/tests/controllers/environments/test_environment_issues.py @@ -0,0 +1,115 @@ +import pytest +from fastapi.testclient import TestClient + +from tests.asserts import assert_fails_validation + +endpoint = "/v1/environments/reported-issues" +valid_post_data = { + "environment_name": "template 1", + "url": "https://github.com/", + "description": "some description", + "is_confirmed": True, +} + + +def test_empty_get(test_client: TestClient): + response = test_client.get(endpoint) + assert response.status_code == 200 + assert response.json() == [] + + +@pytest.mark.parametrize( + "field", + ["url", "description", "environment_name", "is_confirmed"], +) +def test_post_requires_field(test_client: TestClient, field: str): + data = {k: v for k, v in valid_post_data.items() if k != field} + response = test_client.post(endpoint, json=data) + assert_fails_validation(response, field, "missing") + + +def test_post_validates_url(test_client: TestClient): + data = {**valid_post_data, "url": "invalid url"} + response = test_client.post(endpoint, json=data) + assert_fails_validation(response, "url", "url_parsing") + + +def test_url_cannot_be_canonical_chat(test_client: TestClient): + response = test_client.post( + endpoint, + json={ + **valid_post_data, + "url": "https://chat.canonical.com/canonical/pl/n7oahef13jdpde7p6nf7s5yisw", + }, + ) + assert response.status_code == 422 + + +def test_valid_post(test_client: TestClient): + response = test_client.post(endpoint, json=valid_post_data) + assert response.status_code == 200 + _assert_reported_issue(response.json(), valid_post_data) + + +def test_post_three_then_get(test_client: TestClient): + issue1 = {**valid_post_data, "description": "Description 1"} + issue2 = {**valid_post_data, "description": "Description 2"} + issue3 = {**valid_post_data, "description": "Description 3"} + + test_client.post(endpoint, json=issue1) + test_client.post(endpoint, json=issue2) + test_client.post(endpoint, json=issue3) + + response = test_client.get(endpoint) + assert response.status_code == 200 + json = response.json() + _assert_reported_issue(json[0], issue1) + _assert_reported_issue(json[1], issue2) + _assert_reported_issue(json[2], issue3) + + +def test_update_description(test_client: TestClient): + response = test_client.post(endpoint, json=valid_post_data) + issue = response.json() + issue["description"] = "Updated" + response = test_client.put(f"{endpoint}/{issue['id']}", json=issue) + + assert response.status_code == 200 + _assert_reported_issue(response.json(), issue) + + response = test_client.get(endpoint) + _assert_reported_issue(response.json()[0], issue) + + +def test_mark_unconfirmed(test_client: TestClient): + response = test_client.post(endpoint, json=valid_post_data) + issue = response.json() + issue["is_confirmed"] = False + response = test_client.put(f"{endpoint}/{issue['id']}", json=issue) + + assert response.status_code == 200 + _assert_reported_issue(response.json(), issue) + + response = test_client.get(endpoint) + _assert_reported_issue(response.json()[0], issue) + + +def test_delete_issue(test_client: TestClient): + response = test_client.post(endpoint, json=valid_post_data) + issue_id = response.json()["id"] + + response = test_client.delete(f"{endpoint}/{issue_id}") + assert response.status_code == 200 + + response = test_client.get(endpoint) + assert response.json() == [] + + +def _assert_reported_issue(value: dict, expected: dict) -> None: + assert value["environment_name"] == expected["environment_name"] + assert value["url"] == expected["url"] + assert value["description"] == expected["description"] + assert value["is_confirmed"] == expected["is_confirmed"] + assert isinstance(value["id"], int) + assert isinstance(value["created_at"], str) + assert isinstance(value["updated_at"], str) diff --git a/backend/tests/controllers/test_cases/test_reported_issues.py b/backend/tests/controllers/test_cases/test_reported_issues.py index 8c43c7ce..e82d12ac 100644 --- a/backend/tests/controllers/test_cases/test_reported_issues.py +++ b/backend/tests/controllers/test_cases/test_reported_issues.py @@ -5,11 +5,13 @@ from fastapi.testclient import TestClient from httpx import Response +from tests.asserts import assert_fails_validation + endpoint = "/v1/test-cases/reported-issues" valid_post_data = { "template_id": "template 1", "case_name": "case", - "url": "http://issue.link/", + "url": "https://github.com/", "description": "some description", } @@ -62,7 +64,7 @@ def test_empty_get(get: Get): def test_post_requires_field(post: Post, field: str): data = {k: v for k, v in valid_post_data.items() if k != field} response = post(data) - _assert_fails_validation(response, field, "missing") + assert_fails_validation(response, field, "missing") def test_post_requires_template_id_or_case_name(post: Post): @@ -76,7 +78,17 @@ def test_post_requires_template_id_or_case_name(post: Post): def test_post_validates_url(post: Post): response = post({**valid_post_data, "url": "invalid url"}) - _assert_fails_validation(response, "url", "url_parsing") + assert_fails_validation(response, "url", "url_parsing") + + +def test_url_cannot_be_canonical_chat(post: Post): + response = post( + { + **valid_post_data, + "url": "https://chat.canonical.com/canonical/pl/n7oahef13jdpde7p6nf7s5yisw", + } + ) + assert response.status_code == 422 def test_valid_template_id_post(post: Post): @@ -142,7 +154,7 @@ def test_update_description(post: Post, get: Get, put: Put): response = post(valid_post_data) issue = response.json() issue["description"] = "Updated" - response = put(issue["id"], {**issue, "description": "Updated"}) + response = put(issue["id"], issue) assert response.status_code == 200 _assert_reported_issue(response.json(), issue) @@ -161,13 +173,6 @@ def test_delete_issue(post: Post, get: Get, delete: Delete): assert response.json() == [] -def _assert_fails_validation(response: Response, field: str, type: str) -> None: - assert response.status_code == 422 - problem = response.json()["detail"][0] - assert problem["type"] == type - assert problem["loc"] == ["body", field] - - def _assert_reported_issue(value: dict, expected: dict) -> None: if "template_id" in expected: assert value["template_id"] == expected["template_id"] diff --git a/frontend/lib/constants.dart b/frontend/lib/constants.dart new file mode 100644 index 00000000..d422e37b --- /dev/null +++ b/frontend/lib/constants.dart @@ -0,0 +1,5 @@ +const validIssueHosts = { + 'github.com', + 'warthogs.atlassian.net', + 'bugs.launchpad.net', +}; diff --git a/frontend/lib/helpers.dart b/frontend/lib/helpers.dart new file mode 100644 index 00000000..80367c04 --- /dev/null +++ b/frontend/lib/helpers.dart @@ -0,0 +1,16 @@ +import 'constants.dart'; + +String? validateIssueUrl(String? url) { + if (url == null || url.isEmpty) { + return 'Must provide a bug/jira link to the issue'; + } + + final parsedUrl = Uri.tryParse(url); + if (parsedUrl == null) { + return 'Provided url is not valid'; + } else if (!validIssueHosts.contains(parsedUrl.host)) { + final validUrlPrefixes = validIssueHosts.map((host) => 'https://$host'); + return 'Issue url must must start with one of $validUrlPrefixes'; + } + return null; +} diff --git a/frontend/lib/models/environment_issue.dart b/frontend/lib/models/environment_issue.dart new file mode 100644 index 00000000..51d546a9 --- /dev/null +++ b/frontend/lib/models/environment_issue.dart @@ -0,0 +1,18 @@ +import 'package:freezed_annotation/freezed_annotation.dart'; + +part 'environment_issue.freezed.dart'; +part 'environment_issue.g.dart'; + +@freezed +class EnvironmentIssue with _$EnvironmentIssue { + const factory EnvironmentIssue({ + required int id, + @JsonKey(name: 'environment_name') required String environmentName, + required String description, + required String url, + @JsonKey(name: 'is_confirmed') required bool isConfirmed, + }) = _EnvironmentIssue; + + factory EnvironmentIssue.fromJson(Map json) => + _$EnvironmentIssueFromJson(json); +} diff --git a/frontend/lib/providers/environments_issues.dart b/frontend/lib/providers/environments_issues.dart new file mode 100644 index 00000000..ff1fba32 --- /dev/null +++ b/frontend/lib/providers/environments_issues.dart @@ -0,0 +1,52 @@ +import 'package:riverpod_annotation/riverpod_annotation.dart'; + +import '../models/environment_issue.dart'; +import 'api.dart'; + +part 'environments_issues.g.dart'; + +@riverpod +class EnvironmentsIssues extends _$EnvironmentsIssues { + @override + Future> build() { + final api = ref.watch(apiProvider); + return api.getEnvironmentsIssues(); + } + + Future updateIssue(EnvironmentIssue issue) async { + final api = ref.read(apiProvider); + final updatedIssue = await api.updateEnvironmentIssue(issue); + final issues = await future; + state = AsyncData([ + for (final issue in issues) + issue.id == updatedIssue.id ? updatedIssue : issue, + ]); + } + + Future createIssue( + String url, + String description, + String environmentName, + bool isConfirmed, + ) async { + final api = ref.read(apiProvider); + final issue = await api.createEnvironmentIssue( + url, + description, + environmentName, + isConfirmed, + ); + final issues = await future; + state = AsyncData([...issues, issue]); + } + + Future deleteIssue(int issueId) async { + final api = ref.read(apiProvider); + await api.deleteEnvironmentIssue(issueId); + final issues = await future; + state = AsyncData([ + for (final issue in issues) + if (issue.id != issueId) issue, + ]); + } +} diff --git a/frontend/lib/repositories/api_repository.dart b/frontend/lib/repositories/api_repository.dart index f3b88709..4b2095c0 100644 --- a/frontend/lib/repositories/api_repository.dart +++ b/frontend/lib/repositories/api_repository.dart @@ -4,6 +4,7 @@ import '../models/artefact.dart'; import '../models/artefact_build.dart'; import '../models/artefact_version.dart'; +import '../models/environment_issue.dart'; import '../models/family_name.dart'; import '../models/rerun_request.dart'; import '../models/test_execution.dart'; @@ -139,4 +140,42 @@ class ApiRepository { final List data = response.data; return data.map((json) => ArtefactVersion.fromJson(json)).toList(); } + + Future> getEnvironmentsIssues() async { + final response = await dio.get('/v1/environments/reported-issues'); + final List issuesJson = response.data; + return issuesJson.map((json) => EnvironmentIssue.fromJson(json)).toList(); + } + + Future createEnvironmentIssue( + String url, + String description, + String environmentName, + bool isConfirmed, + ) async { + final response = await dio.post( + '/v1/environments/reported-issues', + data: { + 'url': url, + 'description': description, + 'environment_name': environmentName, + 'is_confirmed': isConfirmed, + }, + ); + return EnvironmentIssue.fromJson(response.data); + } + + Future updateEnvironmentIssue( + EnvironmentIssue issue, + ) async { + final response = await dio.put( + '/v1/environments/reported-issues/${issue.id}', + data: issue.toJson(), + ); + return EnvironmentIssue.fromJson(response.data); + } + + Future deleteEnvironmentIssue(int issueId) async { + await dio.delete('/v1/environments/reported-issues/$issueId'); + } } diff --git a/frontend/lib/ui/artefact_page/artefact_page.dart b/frontend/lib/ui/artefact_page/artefact_page.dart index 445c560b..3dfbe800 100644 --- a/frontend/lib/ui/artefact_page/artefact_page.dart +++ b/frontend/lib/ui/artefact_page/artefact_page.dart @@ -1,48 +1,42 @@ import 'package:flutter/material.dart'; -import 'package:flutter_riverpod/flutter_riverpod.dart'; -import 'package:yaru/widgets.dart'; import '../../providers/artefact.dart'; +import '../blocking_provider_preloader.dart'; import '../spacing.dart'; import 'artefact_page_body.dart'; import 'artefact_page_header.dart'; import 'artefact_page_side.dart'; -class ArtefactPage extends ConsumerWidget { +class ArtefactPage extends StatelessWidget { const ArtefactPage({super.key, required this.artefactId}); final int artefactId; @override - Widget build(BuildContext context, WidgetRef ref) { - final artefact = ref.watch(artefactProvider(artefactId)); + Widget build(BuildContext context) { return Padding( padding: const EdgeInsets.only( left: Spacing.pageHorizontalPadding, right: Spacing.pageHorizontalPadding, top: Spacing.level5, ), - child: artefact.when( - data: (artefact) { - return Column( - crossAxisAlignment: CrossAxisAlignment.start, - children: [ - ArtefactPageHeader(artefact: artefact), - const SizedBox(height: Spacing.level4), - Expanded( - child: Row( - children: [ - ArtefactPageSide(artefact: artefact), - Expanded(child: ArtefactPageBody(artefact: artefact)), - ], - ), + child: BlockingProviderPreloader( + provider: artefactProvider(artefactId), + builder: (_, artefact) => Column( + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + ArtefactPageHeader(artefact: artefact), + const SizedBox(height: Spacing.level4), + Expanded( + child: Row( + children: [ + ArtefactPageSide(artefact: artefact), + Expanded(child: ArtefactPageBody(artefact: artefact)), + ], ), - ], - ); - }, - error: (e, stack) => - Center(child: Text('Error:\n$e\nStackTrace:\n$stack')), - loading: () => const Center(child: YaruCircularProgressIndicator()), + ), + ], + ), ), ); } diff --git a/frontend/lib/ui/artefact_page/artefact_page_body.dart b/frontend/lib/ui/artefact_page/artefact_page_body.dart index d6974608..30bb086e 100644 --- a/frontend/lib/ui/artefact_page/artefact_page_body.dart +++ b/frontend/lib/ui/artefact_page/artefact_page_body.dart @@ -4,12 +4,14 @@ import 'package:intersperse/intersperse.dart'; import 'package:yaru/yaru.dart'; import '../../models/artefact.dart'; import '../../models/test_execution.dart'; +import '../../providers/environments_issues.dart'; import '../../providers/filtered_test_executions.dart'; +import '../../providers/tests_issues.dart'; import '../../routing.dart'; +import '../non_blocking_provider_preloader.dart'; import '../spacing.dart'; import 'rerun_filtered_environments_button.dart'; import 'test_execution_expandable/test_execution_expandable.dart'; -import 'test_issues/test_issues_preloader.dart'; class ArtefactPageBody extends ConsumerWidget { const ArtefactPageBody({super.key, required this.artefact}); @@ -43,15 +45,19 @@ class ArtefactPageBody extends ConsumerWidget { const RerunFilteredEnvironmentsButton(), ], ), - TestIssuesPreloader( - child: Expanded( - child: ListView.builder( - itemCount: testExecutions.length, - itemBuilder: (_, i) => Padding( - // Padding is to avoid scroll bar covering trailing buttons - padding: const EdgeInsets.only(right: Spacing.level3), - child: TestExecutionExpandable( - testExecution: testExecutions[i], + NonBlockingProviderPreloader( + provider: environmentsIssuesProvider, + child: NonBlockingProviderPreloader( + provider: testsIssuesProvider, + child: Expanded( + child: ListView.builder( + itemCount: testExecutions.length, + itemBuilder: (_, i) => Padding( + // Padding is to avoid scroll bar covering trailing buttons + padding: const EdgeInsets.only(right: Spacing.level3), + child: TestExecutionExpandable( + testExecution: testExecutions[i], + ), ), ), ), diff --git a/frontend/lib/ui/artefact_page/artefact_page_side.dart b/frontend/lib/ui/artefact_page/artefact_page_side.dart index 1fd07f76..34207161 100644 --- a/frontend/lib/ui/artefact_page/artefact_page_side.dart +++ b/frontend/lib/ui/artefact_page/artefact_page_side.dart @@ -1,9 +1,8 @@ import 'package:flutter/material.dart'; -import 'package:flutter_riverpod/flutter_riverpod.dart'; -import 'package:yaru/widgets.dart'; import '../../models/artefact.dart'; import '../../providers/artefact_builds.dart'; +import '../blocking_provider_preloader.dart'; import '../page_filters/page_filters.dart'; import '../spacing.dart'; import 'artefact_page_info_section.dart'; @@ -37,20 +36,12 @@ class _ArtefactPageSideFilters extends StatelessWidget { @override Widget build(BuildContext context) { - return Consumer( - builder: (context, ref, child) { - final artefactBuilds = ref.watch(artefactBuildsProvider(artefact.id)); - - return artefactBuilds.when( - loading: () => const YaruCircularProgressIndicator(), - error: (e, stack) => - Center(child: Text('Error:\n$e\nStackTrace:\n$stack')), - data: (artefactBuilds) => const PageFiltersView( - searchHint: 'Search by environment name', - width: double.infinity, - ), - ); - }, + return BlockingProviderPreloader( + provider: artefactBuildsProvider(artefact.id), + builder: (_, artefactBuilds) => const PageFiltersView( + searchHint: 'Search by environment name', + width: double.infinity, + ), ); } } diff --git a/frontend/lib/ui/artefact_page/environment_issues/environment_issue_form.dart b/frontend/lib/ui/artefact_page/environment_issues/environment_issue_form.dart new file mode 100644 index 00000000..a9dc374b --- /dev/null +++ b/frontend/lib/ui/artefact_page/environment_issues/environment_issue_form.dart @@ -0,0 +1,268 @@ +import 'package:flutter/material.dart'; +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:go_router/go_router.dart'; + +import '../../../helpers.dart'; +import '../../../models/environment.dart'; +import '../../../models/environment_issue.dart'; +import '../../../providers/environments_issues.dart'; +import '../../spacing.dart'; +import '../../vanilla/vanilla_text_input.dart'; + +void showEnvironmentIssueUpdateDialog({ + required BuildContext context, + required EnvironmentIssue issue, +}) => + showDialog( + context: context, + builder: (_) => Dialog( + child: Padding( + padding: const EdgeInsets.all(Spacing.level4), + child: _EnvironmentIssueUpdateForm(issue: issue), + ), + ), + ); + +void showEnvironmentIssueCreateDialog({ + required BuildContext context, + required Environment environment, +}) => + showDialog( + context: context, + builder: (_) => Dialog( + child: Padding( + padding: const EdgeInsets.all(Spacing.level4), + child: _EnvironmentIssueCreateForm(environment: environment), + ), + ), + ); + +class _EnvironmentIssueUpdateForm extends ConsumerWidget { + const _EnvironmentIssueUpdateForm({required this.issue}); + + final EnvironmentIssue issue; + + @override + Widget build(BuildContext context, WidgetRef ref) { + return _EnvironmentIssueForm( + initialUrl: issue.url, + initialDescription: issue.description, + initialIsConfirmed: issue.isConfirmed, + formSubtitle: 'On all environments with name: ${issue.environmentName}', + onSubmit: (url, description, isConfirmed) => + ref.read(environmentsIssuesProvider.notifier).updateIssue( + issue.copyWith( + url: url, + description: description, + isConfirmed: isConfirmed, + ), + ), + onDelete: () => showDialog( + context: context, + builder: (_) => _DeleteEnvironmentIssueConfirmationDialog( + issue: issue, + ), + ), + ); + } +} + +class _EnvironmentIssueCreateForm extends ConsumerWidget { + const _EnvironmentIssueCreateForm({required this.environment}); + + final Environment environment; + + @override + Widget build(BuildContext context, WidgetRef ref) { + return _EnvironmentIssueForm( + formSubtitle: 'On all runs of environment ${environment.name}', + onSubmit: (url, description, isConfirmed) { + ref.read(environmentsIssuesProvider.notifier).createIssue( + url, + description, + environment.name, + isConfirmed, + ); + }, + ); + } +} + +class _EnvironmentIssueForm extends ConsumerStatefulWidget { + const _EnvironmentIssueForm({ + this.initialUrl = '', + this.initialDescription = '', + this.initialIsConfirmed = false, + required this.formSubtitle, + required this.onSubmit, + this.onDelete, + }); + + final String initialUrl; + final String initialDescription; + final bool initialIsConfirmed; + final String formSubtitle; + final void Function(String url, String description, bool isConfirmed) + onSubmit; + final Future Function()? onDelete; + + @override + ConsumerState<_EnvironmentIssueForm> createState() => + _EnvironmentIssueFormState(); +} + +class _EnvironmentIssueFormState extends ConsumerState<_EnvironmentIssueForm> { + final _formKey = GlobalKey(); + final _urlController = TextEditingController(); + final _descriptionController = TextEditingController(); + late bool _isConfirmed; + + @override + void initState() { + super.initState(); + _urlController.text = widget.initialUrl; + _descriptionController.text = widget.initialDescription; + _isConfirmed = widget.initialIsConfirmed; + } + + @override + void dispose() { + _urlController.dispose(); + _descriptionController.dispose(); + super.dispose(); + } + + @override + Widget build(BuildContext context) { + final buttonFontStyle = Theme.of(context).textTheme.labelLarge; + + return Form( + key: _formKey, + child: SizedBox( + width: 700, + child: Column( + mainAxisSize: MainAxisSize.min, + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + Text('Report Issue', style: Theme.of(context).textTheme.titleLarge), + const SizedBox(height: Spacing.level4), + Text(widget.formSubtitle), + const SizedBox(height: Spacing.level3), + VanillaTextInput( + label: 'Url', + controller: _urlController, + validator: validateIssueUrl, + ), + const SizedBox(height: Spacing.level3), + VanillaTextInput( + label: 'Description', + multiline: true, + controller: _descriptionController, + validator: (url) => url == null || url.isEmpty + ? 'Must provide a description of the issue' + : null, + ), + const SizedBox(height: Spacing.level4), + Row( + children: [ + const Text('Needs confirmation'), + const SizedBox(width: Spacing.level2), + Checkbox( + value: !_isConfirmed, + onChanged: (value) { + if (value != null) { + setState(() { + _isConfirmed = !value; + }); + } + }, + ), + const SizedBox(width: Spacing.level2), + Text( + '(Does this issue require certlab\'s confirmation?)', + style: Theme.of(context) + .textTheme + .bodyMedium + ?.copyWith(color: Colors.grey), + ), + ], + ), + const SizedBox(height: Spacing.level4), + Row( + children: [ + TextButton( + onPressed: () => context.pop(), + child: Text( + 'cancel', + style: buttonFontStyle?.apply(color: Colors.grey), + ), + ), + const Spacer(), + if (widget.onDelete != null) + TextButton( + onPressed: () { + widget.onDelete + ?.call() + .then((didDelete) => context.pop()); + }, + child: Text( + 'delete', + style: buttonFontStyle?.apply(color: Colors.red), + ), + ), + TextButton( + onPressed: () { + if (_formKey.currentState?.validate() == true) { + widget.onSubmit( + _urlController.text, + _descriptionController.text, + _isConfirmed, + ); + context.pop(); + } + }, + child: Text( + 'submit', + style: buttonFontStyle?.apply(color: Colors.black), + ), + ), + ], + ), + ], + ), + ), + ); + } +} + +class _DeleteEnvironmentIssueConfirmationDialog extends ConsumerWidget { + const _DeleteEnvironmentIssueConfirmationDialog({required this.issue}); + + final EnvironmentIssue issue; + + @override + Widget build(BuildContext context, WidgetRef ref) { + return AlertDialog( + title: const Text('Are you sure you want to delete this issue?'), + content: Text( + 'Note that this will remove the issue for' + ' runs of environment ${issue.environmentName}', + ), + actions: [ + TextButton( + onPressed: () { + context.pop(false); + }, + child: const Text('No'), + ), + TextButton( + onPressed: () { + ref.read(environmentsIssuesProvider.notifier).deleteIssue(issue.id); + context.pop(true); + }, + child: const Text('Yes'), + ), + ], + ); + } +} diff --git a/frontend/lib/ui/artefact_page/environment_issues/environment_issue_list_item.dart b/frontend/lib/ui/artefact_page/environment_issues/environment_issue_list_item.dart new file mode 100644 index 00000000..775b41e9 --- /dev/null +++ b/frontend/lib/ui/artefact_page/environment_issues/environment_issue_list_item.dart @@ -0,0 +1,55 @@ +import 'package:flutter/material.dart'; + +import '../../../models/environment_issue.dart'; +import '../../inline_url_text.dart'; +import '../../spacing.dart'; +import 'environment_issue_form.dart'; + +class EnvironmentIssueListItem extends StatelessWidget { + const EnvironmentIssueListItem({super.key, required this.issue}); + + final EnvironmentIssue issue; + + @override + Widget build(BuildContext context) { + return ListTile( + title: Tooltip( + message: issue.description, + child: Text(issue.description, overflow: TextOverflow.ellipsis), + ), + trailing: Row( + mainAxisSize: MainAxisSize.min, + children: [ + issue.isConfirmed + ? Text( + 'confirmed', + style: Theme.of(context) + .textTheme + .bodyMedium + ?.copyWith(color: Colors.green.shade600), + ) + : Text( + 'unconfirmed', + style: Theme.of(context) + .textTheme + .bodyMedium + ?.copyWith(color: Colors.yellow.shade800), + ), + const SizedBox(width: Spacing.level4), + InlineUrlText( + url: issue.url, + urlText: 'URL', + fontStyle: Theme.of(context).textTheme.bodyMedium, + ), + TextButton( + onPressed: () => showEnvironmentIssueUpdateDialog( + context: context, + issue: issue, + ), + child: const Text('edit'), + ), + ], + ), + ); + } +} diff --git a/frontend/lib/ui/artefact_page/environment_issues/environment_issues_expandable.dart b/frontend/lib/ui/artefact_page/environment_issues/environment_issues_expandable.dart new file mode 100644 index 00000000..f9aae2ea --- /dev/null +++ b/frontend/lib/ui/artefact_page/environment_issues/environment_issues_expandable.dart @@ -0,0 +1,51 @@ +import 'package:dartx/dartx.dart'; +import 'package:flutter/material.dart'; +import 'package:flutter_riverpod/flutter_riverpod.dart'; + +import '../../../models/environment.dart'; +import '../../../providers/environments_issues.dart'; +import '../../expandable.dart'; +import 'environment_issue_form.dart'; +import 'environment_issue_list_item.dart'; + +class EnvironmentIssuesExpandable extends ConsumerWidget { + const EnvironmentIssuesExpandable({super.key, required this.environment}); + + final Environment environment; + + @override + Widget build(BuildContext context, WidgetRef ref) { + final issues = ref + .watch( + environmentsIssuesProvider.select( + (value) => value.whenData( + (issues) => issues.filter( + (issue) => issue.environmentName == environment.name, + ), + ), + ), + ) + .value ?? + []; + + return Expandable( + initiallyExpanded: issues.isNotEmpty, + title: Row( + children: [ + Text('Reported Issues (${issues.length})'), + const Spacer(), + TextButton( + onPressed: () => showEnvironmentIssueCreateDialog( + context: context, + environment: environment, + ), + child: const Text('add'), + ), + ], + ), + children: issues + .map((issue) => EnvironmentIssueListItem(issue: issue)) + .toList(), + ); + } +} diff --git a/frontend/lib/ui/artefact_page/test_event_log_expandable.dart b/frontend/lib/ui/artefact_page/test_event_log_expandable.dart index 5eef3254..134b7b1b 100644 --- a/frontend/lib/ui/artefact_page/test_event_log_expandable.dart +++ b/frontend/lib/ui/artefact_page/test_event_log_expandable.dart @@ -1,11 +1,10 @@ import 'package:flutter/material.dart'; -import 'package:flutter_riverpod/flutter_riverpod.dart'; -import 'package:yaru/yaru.dart'; import '../../providers/test_events.dart'; +import '../blocking_provider_preloader.dart'; import '../expandable.dart'; -class TestEventLogExpandable extends ConsumerWidget { +class TestEventLogExpandable extends StatelessWidget { const TestEventLogExpandable({ super.key, required this.testExecutionId, @@ -16,17 +15,14 @@ class TestEventLogExpandable extends ConsumerWidget { final bool initiallyExpanded; @override - Widget build(BuildContext context, WidgetRef ref) { - final testEvents = ref.watch(testEventsProvider(testExecutionId)); - + Widget build(BuildContext context) { return Expandable( - title: const Text('Event Log'), + title: const Text('Event log'), initiallyExpanded: initiallyExpanded, - children: [ - testEvents.when( - loading: () => const Center(child: YaruCircularProgressIndicator()), - error: (error, stackTrace) => Center(child: Text('Error: $error')), - data: (testEvents) => DataTable( + children: [ + BlockingProviderPreloader( + provider: testEventsProvider(testExecutionId), + builder: (_, testEvents) => DataTable( columns: const [ DataColumn( label: Expanded( diff --git a/frontend/lib/ui/artefact_page/test_execution_expandable/test_execution_expandable.dart b/frontend/lib/ui/artefact_page/test_execution_expandable/test_execution_expandable.dart index 004b1bb5..7beb2945 100644 --- a/frontend/lib/ui/artefact_page/test_execution_expandable/test_execution_expandable.dart +++ b/frontend/lib/ui/artefact_page/test_execution_expandable/test_execution_expandable.dart @@ -6,6 +6,7 @@ import '../../../models/test_result.dart'; import '../../expandable.dart'; import '../../inline_url_text.dart'; import '../../spacing.dart'; +import '../environment_issues/environment_issues_expandable.dart'; import '../test_execution_review.dart'; import '../test_result_filter_expandable.dart'; import '../test_event_log_expandable.dart'; @@ -21,6 +22,7 @@ class TestExecutionExpandable extends ConsumerWidget { return Expandable( title: _TestExecutionTileTitle(testExecution: testExecution), children: [ + EnvironmentIssuesExpandable(environment: testExecution.environment), TestEventLogExpandable( testExecutionId: testExecution.id, initiallyExpanded: !testExecution.status.isCompleted, diff --git a/frontend/lib/ui/artefact_page/test_issues/test_issue_form.dart b/frontend/lib/ui/artefact_page/test_issues/test_issue_form.dart index 4d8e932b..6e7c58a9 100644 --- a/frontend/lib/ui/artefact_page/test_issues/test_issue_form.dart +++ b/frontend/lib/ui/artefact_page/test_issues/test_issue_form.dart @@ -2,6 +2,7 @@ import 'package:flutter/material.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:go_router/go_router.dart'; +import '../../../helpers.dart'; import '../../../models/test_issue.dart'; import '../../../models/test_result.dart'; import '../../../providers/tests_issues.dart'; @@ -155,15 +156,7 @@ class _TestIssueFormState extends ConsumerState<_TestIssueForm> { VanillaTextInput( label: 'Url', controller: _urlController, - validator: (url) { - if (url == null || url.isEmpty) { - return 'Must provide a bug/jira link to the issue'; - } - if (Uri.tryParse(url) == null) { - return 'Provided url is not valid'; - } - return null; - }, + validator: validateIssueUrl, ), const SizedBox(height: Spacing.level3), VanillaTextInput( diff --git a/frontend/lib/ui/artefact_page/test_issues/test_issues_preloader.dart b/frontend/lib/ui/artefact_page/test_issues/test_issues_preloader.dart deleted file mode 100644 index 215dcb68..00000000 --- a/frontend/lib/ui/artefact_page/test_issues/test_issues_preloader.dart +++ /dev/null @@ -1,16 +0,0 @@ -import 'package:flutter/material.dart'; -import 'package:flutter_riverpod/flutter_riverpod.dart'; - -import '../../../providers/tests_issues.dart'; - -class TestIssuesPreloader extends ConsumerWidget { - const TestIssuesPreloader({super.key, required this.child}); - - final Widget child; - - @override - Widget build(BuildContext context, WidgetRef ref) { - ref.watch(testsIssuesProvider); - return child; - } -} diff --git a/frontend/lib/ui/artefact_page/test_result_filter_expandable.dart b/frontend/lib/ui/artefact_page/test_result_filter_expandable.dart index cf83643f..2307cdab 100644 --- a/frontend/lib/ui/artefact_page/test_result_filter_expandable.dart +++ b/frontend/lib/ui/artefact_page/test_result_filter_expandable.dart @@ -1,14 +1,14 @@ import 'package:dartx/dartx.dart'; import 'package:flutter/material.dart'; -import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:yaru/yaru.dart'; import '../../models/test_result.dart'; import '../../providers/test_results.dart'; +import '../blocking_provider_preloader.dart'; import '../expandable.dart'; import 'test_result_expandable.dart'; -class TestResultsFilterExpandable extends ConsumerWidget { +class TestResultsFilterExpandable extends StatelessWidget { const TestResultsFilterExpandable({ super.key, required this.statusToFilterBy, @@ -19,9 +19,7 @@ class TestResultsFilterExpandable extends ConsumerWidget { final int testExecutionId; @override - Widget build(BuildContext context, WidgetRef ref) { - final testResults = ref.watch(testResultsProvider(testExecutionId)); - + Widget build(BuildContext context) { Color? fontColor; if (statusToFilterBy == TestResultStatus.failed) { fontColor = YaruColors.red; @@ -32,10 +30,9 @@ class TestResultsFilterExpandable extends ConsumerWidget { final headerStyle = Theme.of(context).textTheme.titleMedium?.apply(color: fontColor); - return testResults.when( - loading: () => const Center(child: YaruCircularProgressIndicator()), - error: (error, stackTrace) => Center(child: Text('Error: $error')), - data: (testResults) { + return BlockingProviderPreloader( + provider: testResultsProvider(testExecutionId), + builder: (_, testResults) { final filteredTestResults = testResults .filter((testResult) => testResult.status == statusToFilterBy) .toList(); diff --git a/frontend/lib/ui/blocking_provider_preloader.dart b/frontend/lib/ui/blocking_provider_preloader.dart new file mode 100644 index 00000000..7a16e2ff --- /dev/null +++ b/frontend/lib/ui/blocking_provider_preloader.dart @@ -0,0 +1,26 @@ +import 'package:flutter/material.dart'; +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:yaru/yaru.dart'; + +class BlockingProviderPreloader extends ConsumerWidget { + const BlockingProviderPreloader({ + super.key, + required this.provider, + required this.builder, + }); + + final ProviderListenable> provider; + final Widget Function(BuildContext context, T value) builder; + + @override + Widget build(BuildContext context, WidgetRef ref) { + final value = ref.watch(provider); + + return value.when( + data: (value) => builder(context, value), + error: (e, stack) => + Center(child: Text('Error:\n$e\nStackTrace:\n$stack')), + loading: () => const Center(child: YaruCircularProgressIndicator()), + ); + } +} diff --git a/frontend/lib/ui/dashboard/dashboard.dart b/frontend/lib/ui/dashboard/dashboard.dart index 916db271..dd457c2e 100644 --- a/frontend/lib/ui/dashboard/dashboard.dart +++ b/frontend/lib/ui/dashboard/dashboard.dart @@ -6,6 +6,7 @@ import 'package:yaru/widgets.dart'; import '../../providers/family_artefacts.dart'; import '../../providers/artefact_side_filters_visibility.dart'; import '../../routing.dart'; +import '../blocking_provider_preloader.dart'; import '../page_filters/page_filters.dart'; import '../spacing.dart'; import 'dashboard_body/dashboard_body.dart'; @@ -35,8 +36,9 @@ class Dashboard extends ConsumerWidget { ), ), Expanded( - child: _ArtefactsLoader( - child: Row( + child: BlockingProviderPreloader( + provider: familyArtefactsProvider(family), + builder: (_, __) => Row( crossAxisAlignment: CrossAxisAlignment.start, children: [ YaruOptionButton( @@ -61,22 +63,3 @@ class Dashboard extends ConsumerWidget { ); } } - -class _ArtefactsLoader extends ConsumerWidget { - const _ArtefactsLoader({required this.child}); - - final Widget child; - - @override - Widget build(BuildContext context, WidgetRef ref) { - final family = AppRoutes.familyFromUri(AppRoutes.uriFromContext(context)); - final artefacts = ref.watch(familyArtefactsProvider(family)); - - return artefacts.when( - data: (_) => child, - error: (e, stack) => - Center(child: Text('Error:\n$e\nStackTrace:\n$stack')), - loading: () => const Center(child: YaruCircularProgressIndicator()), - ); - } -} diff --git a/frontend/lib/ui/dashboard/dashboard_body/dashboard_body.dart b/frontend/lib/ui/dashboard/dashboard_body/dashboard_body.dart index d00e4c86..a3e883b4 100644 --- a/frontend/lib/ui/dashboard/dashboard_body/dashboard_body.dart +++ b/frontend/lib/ui/dashboard/dashboard_body/dashboard_body.dart @@ -1,27 +1,24 @@ import 'package:flutter/widgets.dart'; -import 'package:flutter_riverpod/flutter_riverpod.dart'; -import 'package:yaru/widgets.dart'; import '../../../models/family_name.dart'; import '../../../models/view_modes.dart'; import '../../../providers/view_mode.dart'; import '../../../routing.dart'; +import '../../blocking_provider_preloader.dart'; import 'artefacts_columns_view.dart'; import 'artefacts_list_view/artefacts_list_view.dart'; -class DashboardBody extends ConsumerWidget { +class DashboardBody extends StatelessWidget { const DashboardBody({super.key}); @override - Widget build(BuildContext context, WidgetRef ref) { + Widget build(BuildContext context) { final uri = AppRoutes.uriFromContext(context); final family = AppRoutes.familyFromUri(uri); - final viewMode = ref.watch(viewModeProvider); - return viewMode.when( - loading: () => const YaruCircularProgressIndicator(), - error: (_, __) => const SizedBox.shrink(), - data: (viewMode) => switch ((family, viewMode)) { + return BlockingProviderPreloader( + provider: viewModeProvider, + builder: (_, viewMode) => switch ((family, viewMode)) { (_, ViewModes.dashboard) => const ArtefactsColumnsView(), (FamilyName.snap, ViewModes.list) => const ArtefactsListView.snaps(), (FamilyName.deb, ViewModes.list) => const ArtefactsListView.debs(), diff --git a/frontend/lib/ui/dashboard/dashboard_header/view_mode_toggle.dart b/frontend/lib/ui/dashboard/dashboard_header/view_mode_toggle.dart index 9d3e301c..145dea55 100644 --- a/frontend/lib/ui/dashboard/dashboard_header/view_mode_toggle.dart +++ b/frontend/lib/ui/dashboard/dashboard_header/view_mode_toggle.dart @@ -9,22 +9,20 @@ class ViewModeToggle extends ConsumerWidget { @override Widget build(BuildContext context, WidgetRef ref) { - final viewMode = ref.watch(viewModeProvider); + final viewMode = ref.watch(viewModeProvider).value; - return viewMode.when( - loading: () => const SizedBox.shrink(), - error: (_, __) => const SizedBox.shrink(), - data: (viewMode) => ToggleButtons( - isSelected: [ - viewMode == ViewModes.list, - viewMode == ViewModes.dashboard, - ], - children: const [Icon(Icons.list), Icon(Icons.dashboard)], - onPressed: (i) { - final selectedView = [ViewModes.list, ViewModes.dashboard][i]; - ref.watch(viewModeProvider.notifier).set(selectedView); - }, - ), + if (viewMode == null) return const SizedBox.shrink(); + + return ToggleButtons( + isSelected: [ + viewMode == ViewModes.list, + viewMode == ViewModes.dashboard, + ], + children: const [Icon(Icons.list), Icon(Icons.dashboard)], + onPressed: (i) { + final selectedView = [ViewModes.list, ViewModes.dashboard][i]; + ref.watch(viewModeProvider.notifier).set(selectedView); + }, ); } } diff --git a/frontend/lib/ui/non_blocking_provider_preloader.dart b/frontend/lib/ui/non_blocking_provider_preloader.dart new file mode 100644 index 00000000..bcb78ae6 --- /dev/null +++ b/frontend/lib/ui/non_blocking_provider_preloader.dart @@ -0,0 +1,19 @@ +import 'package:flutter/material.dart'; +import 'package:flutter_riverpod/flutter_riverpod.dart'; + +class NonBlockingProviderPreloader extends ConsumerWidget { + const NonBlockingProviderPreloader({ + super.key, + required this.provider, + required this.child, + }); + + final ProviderListenable provider; + final Widget child; + + @override + Widget build(BuildContext context, WidgetRef ref) { + ref.watch(provider); + return child; + } +}