Environment issues followup (#217)

* Add is_confirmed query parameter to environment issues endpoint

* Make environment issue URL optional if issue is unconfirmed

* Make environment issues URL optional for unconfirmed cases in frontend

backend/migrations/versions/2024_09_23_0840-9d7eed94a543_make_environment_issues_url_nullable.py

@@ -0,0 +1,27 @@
+"""Make environment issues URL nullable
+
+Revision ID: 9d7eed94a543
+Revises: 505b96fd7731
+Create Date: 2024-09-23 08:40:44.972779+00:00
+
+"""
+import sqlalchemy as sa
+from alembic import op
+
+# revision identifiers, used by Alembic.
+revision = "9d7eed94a543"
+down_revision = "505b96fd7731"
+branch_labels = None
+depends_on = None
+
+
+def upgrade() -> None:
+    op.alter_column(
+        "environment_issue", "url", existing_type=sa.VARCHAR(), nullable=True
+    )
+
+
+def downgrade() -> None:
+    op.alter_column(
+        "environment_issue", "url", existing_type=sa.VARCHAR(), nullable=False
+    )

backend/test_observer/controllers/environments/models.py

@@ -1,31 +1,32 @@
 from datetime import datetime
 
-from pydantic import BaseModel, HttpUrl, field_validator
+from pydantic import BaseModel, HttpUrl, model_validator
 
 from test_observer.common.constants import VALID_ISSUE_HOSTS
 
 
 class EnvironmentReportedIssueRequest(BaseModel):
     environment_name: str
     description: str
-    url: HttpUrl
+    url: HttpUrl | None = None
     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:
+    @model_validator(mode="after")
+    def validate_url(self) -> "EnvironmentReportedIssueRequest":
+        if self.url is None and self.is_confirmed:
+            raise ValueError("A URL is required if the issue is confirmed")
+
+        if self.url is not None and self.url.host not in VALID_ISSUE_HOSTS:
             raise ValueError(f"Issue url must belong to one of {VALID_ISSUE_HOSTS}")
-        return url
+
+        return self
 
 
 class EnvironmentReportedIssueResponse(BaseModel):
     id: int
     environment_name: str
     description: str
-    url: HttpUrl
+    url: HttpUrl | None
     is_confirmed: bool
     created_at: datetime
     updated_at: datetime

backend/test_observer/controllers/environments/reported_issues.py

@@ -13,8 +13,13 @@
 
 
 @router.get(endpoint, response_model=list[EnvironmentReportedIssueResponse])
-def get_reported_issues(db: Session = Depends(get_db)):
-    return db.execute(select(EnvironmentIssue)).scalars()
+def get_reported_issues(
+    is_confirmed: bool | None = None, db: Session = Depends(get_db)
+):
+    stmt = select(EnvironmentIssue)
+    if is_confirmed is not None:
+        stmt = stmt.where(EnvironmentIssue.is_confirmed == is_confirmed)
+    return db.execute(stmt).scalars()
 
 
 @router.post(endpoint, response_model=EnvironmentReportedIssueResponse)

backend/test_observer/data_access/models.py

@@ -488,7 +488,7 @@ class EnvironmentIssue(Base):
     __tablename__ = "environment_issue"
 
     environment_name: Mapped[str]
-    url: Mapped[str]
+    url: Mapped[str | None] = mapped_column(default=None)
     description: Mapped[str]
     is_confirmed: Mapped[bool]
 
@@ -498,4 +498,5 @@ def __repr__(self) -> str:
             "environment_name",
             "url",
             "description",
+            "is_confirmed",
         )

backend/tests/controllers/environments/test_environment_issues.py

@@ -20,14 +20,33 @@ def test_empty_get(test_client: TestClient):
 
 @pytest.mark.parametrize(
     "field",
-    ["url", "description", "environment_name", "is_confirmed"],
+    ["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_url_is_required_if_confirmed(test_client: TestClient):
+    data = {**valid_post_data, "url": None}
+
+    response = test_client.post(endpoint, json=data)
+
+    assert response.status_code == 422
+
+
+def test_url_not_required_if_unconfirmed(test_client: TestClient):
+    data = {**valid_post_data, "is_confirmed": False}
+    data.pop("url")
+
+    response = test_client.post(endpoint, json=data)
+    json = response.json()
+
+    assert response.status_code == 200
+    _assert_reported_issue(json, json)
+
+
 def test_post_validates_url(test_client: TestClient):
     data = {**valid_post_data, "url": "invalid url"}
     response = test_client.post(endpoint, json=data)
@@ -68,6 +87,28 @@ def test_post_three_then_get(test_client: TestClient):
     _assert_reported_issue(json[2], issue3)
 
 
+def test_get_needs_confirmation(test_client: TestClient):
+    confirmed_issue = {
+        **valid_post_data,
+        "description": "Confirmed",
+        "is_confirmed": True,
+    }
+    unconfirmed_issue = {
+        **valid_post_data,
+        "description": "Unconfirmed",
+        "is_confirmed": False,
+    }
+
+    test_client.post(endpoint, json=confirmed_issue)
+    test_client.post(endpoint, json=unconfirmed_issue)
+
+    response = test_client.get(endpoint, params={"is_confirmed": False})
+    assert response.status_code == 200
+    json = response.json()
+    assert len(json) == 1
+    _assert_reported_issue(json[0], unconfirmed_issue)
+
+
 def test_update_description(test_client: TestClient):
     response = test_client.post(endpoint, json=valid_post_data)
     issue = response.json()

frontend/lib/models/environment_issue.dart

@@ -9,7 +9,7 @@ class EnvironmentIssue with _$EnvironmentIssue {
     required int id,
     @JsonKey(name: 'environment_name') required String environmentName,
     required String description,
-    required String url,
+    required String? url,
     @JsonKey(name: 'is_confirmed') required bool isConfirmed,
   }) = _EnvironmentIssue;
 

frontend/lib/repositories/api_repository.dart

@@ -156,7 +156,7 @@ class ApiRepository {
     final response = await dio.post(
       '/v1/environments/reported-issues',
       data: {
-        'url': url,
+        'url': url.isEmpty ? null : url,
         'description': description,
         'environment_name': environmentName,
         'is_confirmed': isConfirmed,

frontend/lib/ui/artefact_page/environment_issues/environment_issue_form.dart

@@ -45,7 +45,7 @@ class _EnvironmentIssueUpdateForm extends ConsumerWidget {
   @override
   Widget build(BuildContext context, WidgetRef ref) {
     return _EnvironmentIssueForm(
-      initialUrl: issue.url,
+      initialUrl: issue.url ?? '',
       initialDescription: issue.description,
       initialIsConfirmed: issue.isConfirmed,
       formSubtitle: 'On all environments with name: ${issue.environmentName}',
@@ -151,7 +151,15 @@ class _EnvironmentIssueFormState extends ConsumerState<_EnvironmentIssueForm> {
             VanillaTextInput(
               label: 'Url',
               controller: _urlController,
-              validator: validateIssueUrl,
+              validator: (url) {
+                if ((url == null || url.isEmpty) && _isConfirmed) {
+                  return 'Confirmed environment issues must have a URL';
+                }
+
+                if (url != null && url.isNotEmpty) return validateIssueUrl(url);
+
+                return null;
+              },
             ),
             const SizedBox(height: Spacing.level3),
             VanillaTextInput(

frontend/lib/ui/artefact_page/environment_issues/environment_issue_list_item.dart

@@ -12,6 +12,7 @@ class EnvironmentIssueListItem extends StatelessWidget {
 
   @override
   Widget build(BuildContext context) {
+    final issueUrl = issue.url;
     return ListTile(
       title: Tooltip(
         message: issue.description,
@@ -36,11 +37,12 @@ class EnvironmentIssueListItem extends StatelessWidget {
                       ?.copyWith(color: Colors.yellow.shade800),
                 ),
           const SizedBox(width: Spacing.level4),
-          InlineUrlText(
-            url: issue.url,
-            urlText: 'URL',
-            fontStyle: Theme.of(context).textTheme.bodyMedium,
-          ),
+          if (issueUrl != null)
+            InlineUrlText(
+              url: issueUrl,
+              urlText: 'URL',
+              fontStyle: Theme.of(context).textTheme.bodyMedium,
+            ),
           TextButton(
             onPressed: () => showEnvironmentIssueUpdateDialog(
               context: context,