diff --git a/frontend/lib/models/test_issue.dart b/frontend/lib/models/test_issue.dart new file mode 100644 index 00000000..e3c3b3bb --- /dev/null +++ b/frontend/lib/models/test_issue.dart @@ -0,0 +1,18 @@ +import 'package:freezed_annotation/freezed_annotation.dart'; + +part 'test_issue.freezed.dart'; +part 'test_issue.g.dart'; + +@freezed +class TestIssue with _$TestIssue { + const factory TestIssue({ + required int id, + @JsonKey(name: 'template_id') required String templateId, + @JsonKey(name: 'case_name') required String caseName, + required String description, + required String url, + }) = _TestIssue; + + factory TestIssue.fromJson(Map json) => + _$TestIssueFromJson(json); +} diff --git a/frontend/lib/models/test_result.dart b/frontend/lib/models/test_result.dart index b4bf04a5..98a8d099 100644 --- a/frontend/lib/models/test_result.dart +++ b/frontend/lib/models/test_result.dart @@ -27,6 +27,7 @@ class TestResult with _$TestResult { required TestResultStatus status, @Default('') String category, @Default('') String comment, + @JsonKey(name: 'template_id') @Default('') String templateId, @JsonKey(name: 'io_log') @Default('') String ioLog, @JsonKey(name: 'previous_results') @Default([]) diff --git a/frontend/lib/providers/test_result_issues.dart b/frontend/lib/providers/test_result_issues.dart new file mode 100644 index 00000000..66ae2fa7 --- /dev/null +++ b/frontend/lib/providers/test_result_issues.dart @@ -0,0 +1,26 @@ +import 'package:dartx/dartx.dart'; +import 'package:riverpod_annotation/riverpod_annotation.dart'; + +import '../models/test_issue.dart'; +import '../models/test_result.dart'; +import 'tests_issues.dart'; + +part 'test_result_issues.g.dart'; + +@riverpod +Future> testResultIssues( + TestResultIssuesRef ref, + TestResult testResult, +) { + return ref.watch( + testsIssuesProvider.selectAsync( + (issues) => issues + .filter( + (issue) => + issue.caseName == testResult.name || + issue.templateId == testResult.templateId, + ) + .toList(), + ), + ); +} diff --git a/frontend/lib/providers/tests_issues.dart b/frontend/lib/providers/tests_issues.dart new file mode 100644 index 00000000..39532229 --- /dev/null +++ b/frontend/lib/providers/tests_issues.dart @@ -0,0 +1,48 @@ +import 'package:riverpod_annotation/riverpod_annotation.dart'; + +import '../models/test_issue.dart'; +import 'api.dart'; + +part 'tests_issues.g.dart'; + +@riverpod +class TestsIssues extends _$TestsIssues { + @override + Future> build() { + final api = ref.watch(apiProvider); + return api.getTestIssues(); + } + + void updateIssue(TestIssue issue) async { + final api = ref.read(apiProvider); + final updatedIssue = await api.updateTestIssue(issue); + final issues = await future; + state = AsyncData([ + for (final issue in issues) + issue.id == updatedIssue.id ? updatedIssue : issue, + ]); + } + + void createIssue( + String url, + String description, { + String? caseName, + String? templateId, + }) async { + final api = ref.read(apiProvider); + final issue = + await api.createTestIssue(url, description, caseName, templateId); + final issues = await future; + state = AsyncData([...issues, issue]); + } + + void deleteIssue(int issueId) async { + final api = ref.read(apiProvider); + await api.deleteTestIssue(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 888fd319..5cfad9c4 100644 --- a/frontend/lib/repositories/api_repository.dart +++ b/frontend/lib/repositories/api_repository.dart @@ -6,6 +6,7 @@ import '../models/artefact_build.dart'; import '../models/family_name.dart'; import '../models/rerun_request.dart'; import '../models/test_execution.dart'; +import '../models/test_issue.dart'; import '../models/test_result.dart'; import '../models/test_event.dart'; @@ -88,4 +89,40 @@ class ApiRepository { rerunsJson.map((json) => RerunRequest.fromJson(json)).toList(); return reruns; } + + Future> getTestIssues() async { + final response = await dio.get('/v1/test-cases/reported-issues'); + final List issuesJson = response.data; + return issuesJson.map((json) => TestIssue.fromJson(json)).toList(); + } + + Future updateTestIssue(TestIssue issue) async { + final response = await dio.put( + '/v1/test-cases/reported-issues/${issue.id}', + data: issue.toJson(), + ); + return TestIssue.fromJson(response.data); + } + + Future createTestIssue( + String url, + String description, + String? caseName, + String? templateId, + ) async { + final response = await dio.post( + '/v1/test-cases/reported-issues', + data: { + 'url': url, + 'description': description, + if (caseName != null) 'case_name': caseName, + if (templateId != null) 'template_id': templateId, + }, + ); + return TestIssue.fromJson(response.data); + } + + Future deleteTestIssue(int issueId) async { + await dio.delete('/v1/test-cases/reported-issues/$issueId'); + } } diff --git a/frontend/lib/ui/artefact_page/artefact_page_body.dart b/frontend/lib/ui/artefact_page/artefact_page_body.dart index ab73eb1c..d6974608 100644 --- a/frontend/lib/ui/artefact_page/artefact_page_body.dart +++ b/frontend/lib/ui/artefact_page/artefact_page_body.dart @@ -9,6 +9,7 @@ import '../../routing.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}); @@ -42,14 +43,16 @@ class ArtefactPageBody extends ConsumerWidget { const RerunFilteredEnvironmentsButton(), ], ), - 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], + 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], + ), ), ), ), diff --git a/frontend/lib/ui/artefact_page/test_execution_pop_over.dart b/frontend/lib/ui/artefact_page/test_execution_pop_over.dart index 0715702e..8d5712fd 100644 --- a/frontend/lib/ui/artefact_page/test_execution_pop_over.dart +++ b/frontend/lib/ui/artefact_page/test_execution_pop_over.dart @@ -6,6 +6,7 @@ import '../../models/family_name.dart'; import '../../models/test_execution.dart'; import '../../providers/review_test_execution.dart'; import '../spacing.dart'; +import '../vanilla/vanilla_text_input.dart'; class TestExecutionPopOver extends ConsumerStatefulWidget { const TestExecutionPopOver({ @@ -102,19 +103,12 @@ class TestExecutionPopOverState extends ConsumerState { .toList(), ), const SizedBox(height: Spacing.level4), - Text( - 'Additional review comments:', - style: Theme.of(context).textTheme.titleLarge, - ), - const SizedBox(height: Spacing.level3), - TextField( - decoration: const InputDecoration( - border: OutlineInputBorder(), - hintText: 'Insert review comment', - ), + VanillaTextInput( + label: 'Additional review comments:', + labelStyle: Theme.of(context).textTheme.titleLarge, controller: reviewCommentController, - keyboardType: TextInputType.multiline, - maxLines: null, + multiline: true, + hintText: 'Insert review comment', ), const SizedBox(height: Spacing.level3), ElevatedButton( diff --git a/frontend/lib/ui/artefact_page/test_execution_review.dart b/frontend/lib/ui/artefact_page/test_execution_review.dart index f136f5a6..e578bed8 100644 --- a/frontend/lib/ui/artefact_page/test_execution_review.dart +++ b/frontend/lib/ui/artefact_page/test_execution_review.dart @@ -51,7 +51,7 @@ class TestExecutionReviewButton extends StatelessWidget { ), direction: PopoverDirection.bottom, width: 500, - height: 400, + height: 450, arrowHeight: 15, arrowWidth: 30, ); 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 new file mode 100644 index 00000000..4d8e932b --- /dev/null +++ b/frontend/lib/ui/artefact_page/test_issues/test_issue_form.dart @@ -0,0 +1,258 @@ +import 'package:flutter/material.dart'; +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:go_router/go_router.dart'; + +import '../../../models/test_issue.dart'; +import '../../../models/test_result.dart'; +import '../../../providers/tests_issues.dart'; +import '../../spacing.dart'; +import '../../vanilla/vanilla_text_input.dart'; + +void showTestIssueUpdateDialog({ + required BuildContext context, + required TestIssue issue, +}) => + showDialog( + context: context, + builder: (_) => Dialog( + child: Padding( + padding: const EdgeInsets.all(Spacing.level4), + child: _TestIssueUpdateForm(issue: issue), + ), + ), + ); + +void showTestIssueCreateDialog({ + required BuildContext context, + required TestResult testResult, +}) => + showDialog( + context: context, + builder: (_) => Dialog( + child: Padding( + padding: const EdgeInsets.all(Spacing.level4), + child: _TestIssueCreateForm(testResult: testResult), + ), + ), + ); + +class _TestIssueUpdateForm extends ConsumerWidget { + const _TestIssueUpdateForm({required this.issue}); + + final TestIssue issue; + + @override + Widget build(BuildContext context, WidgetRef ref) { + final issueOn = issue.templateId.isEmpty + ? 'On all cases with name: ${issue.caseName}' + : 'On all cases with template id: ${issue.templateId}'; + return _TestIssueForm( + initialUrl: issue.url, + initialDescription: issue.description, + formSubtitle: issueOn, + onSubmit: (url, description) => + ref.read(testsIssuesProvider.notifier).updateIssue( + issue.copyWith( + url: url, + description: description, + ), + ), + onDelete: () => showDialog( + context: context, + builder: (_) => _DeleteTestIssueConfirmationDialog( + issue: issue, + ), + ), + ); + } +} + +class _TestIssueCreateForm extends ConsumerWidget { + const _TestIssueCreateForm({required this.testResult}); + + final TestResult testResult; + + @override + Widget build(BuildContext context, WidgetRef ref) { + final issueOn = testResult.templateId.isEmpty + ? 'On all cases with name: ${testResult.name}' + : 'On all cases with template id: ${testResult.templateId}'; + return _TestIssueForm( + formSubtitle: issueOn, + onSubmit: (url, description) { + if (testResult.templateId.isEmpty) { + ref.read(testsIssuesProvider.notifier).createIssue( + url, + description, + caseName: testResult.name, + ); + } else { + ref.read(testsIssuesProvider.notifier).createIssue( + url, + description, + templateId: testResult.templateId, + ); + } + }, + ); + } +} + +class _TestIssueForm extends ConsumerStatefulWidget { + const _TestIssueForm({ + this.initialUrl = '', + this.initialDescription = '', + required this.formSubtitle, + required this.onSubmit, + this.onDelete, + }); + + final String initialUrl; + final String initialDescription; + final String formSubtitle; + final void Function(String url, String description) onSubmit; + final Future Function()? onDelete; + + @override + ConsumerState<_TestIssueForm> createState() => _TestIssueFormState(); +} + +class _TestIssueFormState extends ConsumerState<_TestIssueForm> { + final _formKey = GlobalKey(); + final _urlController = TextEditingController(); + final _descriptionController = TextEditingController(); + + @override + void initState() { + super.initState(); + _urlController.text = widget.initialUrl; + _descriptionController.text = widget.initialDescription; + } + + @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: (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; + }, + ), + 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: [ + 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, + ); + context.pop(); + } + }, + child: Text( + 'submit', + style: buttonFontStyle?.apply(color: Colors.black), + ), + ), + ], + ), + ], + ), + ), + ); + } +} + +class _DeleteTestIssueConfirmationDialog extends ConsumerWidget { + const _DeleteTestIssueConfirmationDialog({required this.issue}); + + final TestIssue issue; + + @override + Widget build(BuildContext context, WidgetRef ref) { + String message = 'Note that this will remove the issue for all tests with'; + if (issue.templateId.isNotEmpty) { + message += ' template id: ${issue.templateId}'; + } else { + message += ' case name: ${issue.caseName}'; + } + + return AlertDialog( + title: const Text('Are you sure you want to delete this issue?'), + content: Text(message), + actions: [ + TextButton( + onPressed: () { + context.pop(false); + }, + child: const Text('No'), + ), + TextButton( + onPressed: () { + ref.read(testsIssuesProvider.notifier).deleteIssue(issue.id); + context.pop(true); + }, + child: const Text('Yes'), + ), + ], + ); + } +} diff --git a/frontend/lib/ui/artefact_page/test_issues/test_issue_list_item.dart b/frontend/lib/ui/artefact_page/test_issues/test_issue_list_item.dart new file mode 100644 index 00000000..7339d210 --- /dev/null +++ b/frontend/lib/ui/artefact_page/test_issues/test_issue_list_item.dart @@ -0,0 +1,36 @@ +import 'package:flutter/material.dart'; + +import '../../../models/test_issue.dart'; +import '../../inline_url_text.dart'; +import 'test_issue_form.dart'; + +class TestIssueListItem extends StatelessWidget { + const TestIssueListItem({super.key, required this.issue}); + + final TestIssue 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: [ + InlineUrlText( + url: issue.url, + urlText: 'URL', + fontStyle: Theme.of(context).textTheme.bodyMedium, + ), + TextButton( + onPressed: () => + showTestIssueUpdateDialog(context: context, issue: issue), + child: const Text('edit'), + ), + ], + ), + ); + } +} diff --git a/frontend/lib/ui/artefact_page/test_issues/test_issues_expandable.dart b/frontend/lib/ui/artefact_page/test_issues/test_issues_expandable.dart new file mode 100644 index 00000000..ff2f1a76 --- /dev/null +++ b/frontend/lib/ui/artefact_page/test_issues/test_issues_expandable.dart @@ -0,0 +1,37 @@ +import 'package:flutter/material.dart'; +import 'package:flutter_riverpod/flutter_riverpod.dart'; + +import '../../../models/test_result.dart'; +import '../../../providers/test_result_issues.dart'; +import '../../expandable.dart'; +import 'test_issue_form.dart'; +import 'test_issue_list_item.dart'; + +class TestIssuesExpandable extends ConsumerWidget { + const TestIssuesExpandable({super.key, required this.testResult}); + + final TestResult testResult; + + @override + Widget build(BuildContext context, WidgetRef ref) { + final issues = ref.watch(testResultIssuesProvider(testResult)).value ?? []; + + return Expandable( + initiallyExpanded: issues.isNotEmpty, + title: Row( + children: [ + Text('Reported Issues (${issues.length})'), + const Spacer(), + TextButton( + onPressed: () => showTestIssueCreateDialog( + context: context, + testResult: testResult, + ), + child: const Text('add'), + ), + ], + ), + children: issues.map((issue) => TestIssueListItem(issue: issue)).toList(), + ); + } +} 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 new file mode 100644 index 00000000..215dcb68 --- /dev/null +++ b/frontend/lib/ui/artefact_page/test_issues/test_issues_preloader.dart @@ -0,0 +1,16 @@ +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_expandable.dart b/frontend/lib/ui/artefact_page/test_result_expandable.dart index ba05f654..a6518bd3 100644 --- a/frontend/lib/ui/artefact_page/test_result_expandable.dart +++ b/frontend/lib/ui/artefact_page/test_result_expandable.dart @@ -1,26 +1,56 @@ import 'package:flutter/material.dart'; +import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:yaru/widgets.dart'; import '../../models/test_result.dart'; +import '../../providers/test_result_issues.dart'; import '../expandable.dart'; +import 'test_issues/test_issues_expandable.dart'; -class TestResultExpandable extends StatelessWidget { +class TestResultExpandable extends ConsumerWidget { const TestResultExpandable({super.key, required this.testResult}); final TestResult testResult; @override - Widget build(BuildContext context) { + Widget build(BuildContext context, WidgetRef ref) { + final issues = ref.watch(testResultIssuesProvider(testResult)).value ?? []; + + String title = testResult.name; + if (issues.length == 1) { + title += ' (${issues.length} reported issue)'; + } else if (issues.length > 1) { + title += ' (${issues.length} reported issues)'; + } + return Expandable( title: Row( children: [ - Text(testResult.name), + Text(title), const Spacer(), PreviousTestResultsWidget( previousResults: testResult.previousResults, ), ], ), + children: [ + TestIssuesExpandable(testResult: testResult), + _TestResultOutputExpandable(testResult: testResult), + ], + ); + } +} + +class _TestResultOutputExpandable extends StatelessWidget { + const _TestResultOutputExpandable({required this.testResult}); + + final TestResult testResult; + + @override + Widget build(BuildContext context) { + return Expandable( + title: const Text('Details'), + initiallyExpanded: true, children: [ if (testResult.category != '') YaruTile( diff --git a/frontend/lib/ui/expandable.dart b/frontend/lib/ui/expandable.dart index d26b27c8..b2204a78 100644 --- a/frontend/lib/ui/expandable.dart +++ b/frontend/lib/ui/expandable.dart @@ -21,6 +21,7 @@ class Expandable extends StatelessWidget { controlAffinity: ListTileControlAffinity.leading, childrenPadding: const EdgeInsets.only(left: Spacing.level4), shape: const Border(), + collapsedShape: const Border(), title: title, initiallyExpanded: initiallyExpanded, children: children, diff --git a/frontend/lib/ui/vanilla/vanilla_text_input.dart b/frontend/lib/ui/vanilla/vanilla_text_input.dart new file mode 100644 index 00000000..dc4a3293 --- /dev/null +++ b/frontend/lib/ui/vanilla/vanilla_text_input.dart @@ -0,0 +1,68 @@ +import 'package:flutter/material.dart'; + +import '../spacing.dart'; + +class VanillaTextInput extends StatelessWidget { + const VanillaTextInput({ + super.key, + String? label, + this.enabled = true, + this.multiline = false, + this.controller, + this.validator, + this.hintText, + this.labelStyle, + }) : _label = label; + + final String? _label; + final bool enabled; + final bool multiline; + final TextEditingController? controller; + final String? Function(String?)? validator; + final String? hintText; + final TextStyle? labelStyle; + + @override + Widget build(BuildContext context) { + return Column( + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + if (_label != null) + Text( + _label, + style: labelStyle ?? Theme.of(context).textTheme.titleMedium, + ), + const SizedBox(height: Spacing.level2), + TextFormField( + controller: controller, + validator: validator, + keyboardType: multiline ? TextInputType.multiline : null, + maxLines: multiline ? null : 1, + minLines: multiline ? 4 : null, + enabled: enabled, + decoration: InputDecoration( + hintText: hintText, + enabledBorder: + const UnderlineInputBorder(borderRadius: BorderRadius.zero), + focusedBorder: const OutlineInputBorder( + borderRadius: BorderRadius.zero, + borderSide: BorderSide(color: Colors.blue), + ), + disabledBorder: const UnderlineInputBorder( + borderRadius: BorderRadius.zero, + borderSide: BorderSide(color: Colors.black12), + ), + focusedErrorBorder: const OutlineInputBorder( + borderRadius: BorderRadius.zero, + borderSide: BorderSide(color: Colors.red), + ), + errorBorder: const UnderlineInputBorder( + borderRadius: BorderRadius.zero, + borderSide: BorderSide(color: Colors.red), + ), + ), + ), + ], + ); + } +} diff --git a/frontend/pubspec.yaml b/frontend/pubspec.yaml index 2ea4252f..7fdbd7af 100644 --- a/frontend/pubspec.yaml +++ b/frontend/pubspec.yaml @@ -3,7 +3,7 @@ description: A new Flutter project. publish_to: "none" version: 1.0.0+1 environment: - sdk: ">=3.0.0 <4.0.0" + sdk: ">=3.2.0 <4.0.0" dependencies: dartx: ^1.1.0 dio: ^5.1.2