Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Reporting environment issues #213

Merged
merged 12 commits into from
Sep 18, 2024
Merged

Reporting environment issues #213

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
6110b71
Add missing field from repr
omar-selo Sep 16, 2024
2f2ef42
Add support for environment issues to the backend
omar-selo Sep 16, 2024
41256c5
Require issue url be github, jira, or launchpad
omar-selo Sep 16, 2024
3648b34
Add is_confirmed to environment issue
omar-selo Sep 16, 2024
a5cf24a
Include environment issues in seed_script
omar-selo Sep 16, 2024
3946c07
Fix seed script
omar-selo Sep 16, 2024
77786ae
Environment issues frontend
omar-selo Sep 17, 2024
dbaf6a2
Minor improvements
omar-selo Sep 17, 2024
a123a51
Some code improvements
omar-selo Sep 18, 2024
90bcf0e
Add non-blocking provider preloader
omar-selo Sep 18, 2024
2b44add
Refactor loading providers to a widget
omar-selo Sep 18, 2024
e7e97d8
Fix missing event log expandable
omar-selo Sep 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 19 additions & 25 deletions frontend/lib/ui/artefact_page/artefact_page.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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()),
),
],
),
),
);
}
Expand Down
11 changes: 7 additions & 4 deletions frontend/lib/ui/artefact_page/artefact_page_body.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +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 'environment_issues/environment_issues_preloader.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});
Expand Down Expand Up @@ -44,8 +45,10 @@ class ArtefactPageBody extends ConsumerWidget {
const RerunFilteredEnvironmentsButton(),
],
),
EnvironmentIssuesPreloader(
child: TestIssuesPreloader(
NonBlockingProviderPreloader(
provider: environmentsIssuesProvider,
child: NonBlockingProviderPreloader(
provider: testsIssuesProvider,
child: Expanded(
child: ListView.builder(
itemCount: testExecutions.length,
Expand Down
23 changes: 7 additions & 16 deletions frontend/lib/ui/artefact_page/artefact_page_side.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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,
),
);
}
}
16 changes: 0 additions & 16 deletions frontend/lib/ui/artefact_page/environment_issues/environment_issues_preloader.dart
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we maybe make the TestCaseIsueePreloader a generic ReportedIssuesPreloader that will watch for both providers, it seems like we are repeating most of the code.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inspired by this comment, I refactored most loading of providers into two widgets: NonBlockingProviderPreloader and BlockingProviderRreloader.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How can you not love a code clean-up 😄

This file was deleted.

107 changes: 48 additions & 59 deletions frontend/lib/ui/artefact_page/test_event_log_expandable.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
import 'package:flutter/material.dart';
import 'package:flutter_riverpod/flutter_riverpod.dart';
import 'package:yaru/yaru.dart';

import '../../providers/test_events.dart';
import '../expandable.dart';
import '../blocking_provider_preloader.dart';

class TestEventLogExpandable extends ConsumerWidget {
class TestEventLogExpandable extends StatelessWidget {
const TestEventLogExpandable({
super.key,
required this.testExecutionId,
Expand All @@ -16,66 +14,57 @@ class TestEventLogExpandable extends ConsumerWidget {
final bool initiallyExpanded;

@override
Widget build(BuildContext context, WidgetRef ref) {
final testEvents = ref.watch(testEventsProvider(testExecutionId));

return Expandable(
title: const Text('Event Log'),
initiallyExpanded: initiallyExpanded,
children: <Widget>[
testEvents.when(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aren't we deleting the Expandable here, I don't see it anywhere in the new code

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great catch

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried manually testing everything now, so hopefully I didn't miss anything else

loading: () => const Center(child: YaruCircularProgressIndicator()),
error: (error, stackTrace) => Center(child: Text('Error: $error')),
data: (testEvents) => DataTable(
columns: const <DataColumn>[
DataColumn(
label: Expanded(
child: Text(
'Event Name',
style: TextStyle(fontStyle: FontStyle.italic),
),
),
Widget build(BuildContext context) {
return BlockingProviderPreloader(
provider: testEventsProvider(testExecutionId),
builder: (_, testEvents) => DataTable(
columns: const <DataColumn>[
DataColumn(
label: Expanded(
child: Text(
'Event Name',
style: TextStyle(fontStyle: FontStyle.italic),
),
DataColumn(
label: Expanded(
child: Text(
'Timestamp',
style: TextStyle(fontStyle: FontStyle.italic),
),
),
),
),
DataColumn(
label: Expanded(
child: Text(
'Timestamp',
style: TextStyle(fontStyle: FontStyle.italic),
),
DataColumn(
label: Expanded(
child: Text(
'Detail',
style: TextStyle(fontStyle: FontStyle.italic),
),
),
),
),
DataColumn(
label: Expanded(
child: Text(
'Detail',
style: TextStyle(fontStyle: FontStyle.italic),
),
],
rows: testEvents
.map(
(testEvent) => DataRow(
cells: <DataCell>[
DataCell(Text(testEvent.eventName)),
DataCell(Text(testEvent.timestamp)),
DataCell(
Tooltip(
message: testEvent.detail,
child: Text(
testEvent.detail,
overflow: TextOverflow.ellipsis,
maxLines: 1,
),
),
),
),
],
rows: testEvents
.map(
(testEvent) => DataRow(
cells: <DataCell>[
DataCell(Text(testEvent.eventName)),
DataCell(Text(testEvent.timestamp)),
DataCell(
Tooltip(
message: testEvent.detail,
child: Text(
testEvent.detail,
overflow: TextOverflow.ellipsis,
maxLines: 1,
),
],
),
),
)
.toList(),
),
),
],
],
),
)
.toList(),
),
);
}
}
16 changes: 0 additions & 16 deletions frontend/lib/ui/artefact_page/test_issues/test_issues_preloader.dart
View file
Open in desktop

This file was deleted.

15 changes: 6 additions & 9 deletions frontend/lib/ui/artefact_page/test_result_filter_expandable.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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;
Expand All @@ -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();
Expand Down
26 changes: 26 additions & 0 deletions frontend/lib/ui/blocking_provider_preloader.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import 'package:flutter/material.dart';
import 'package:flutter_riverpod/flutter_riverpod.dart';
import 'package:yaru/yaru.dart';

class BlockingProviderPreloader<T> extends ConsumerWidget {
const BlockingProviderPreloader({
super.key,
required this.provider,
required this.builder,
});

final ProviderListenable<AsyncValue<T>> 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()),
);
}
}
Loading
Loading