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.

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

Implement frontend for test execution review #93

Merged
merged 24 commits into from
Jan 5, 2024

Conversation

andrejvelichkovski
Copy link
Contributor

@andrejvelichkovski andrejvelichkovski commented Jan 2, 2024

This PR resolves RTW-204 and implements the frontend functionality for test execution review.

Implementation Changes

As part of this change, a few things have been updated:

  • New popover for the review process was added
  • Button to trigger the popover
  • Riverpod provider ArtefactBuilds was updated to also include functionality for patching the test execution

Screenshots

Screenshot from 2024-01-03 09-17-27
Screenshot from 2024-01-03 09-17-34
Screenshot from 2024-01-03 09-17-48

Design changes

I have changed the design slightly, as I couldn't find a way to use YaruPopupMenuButton which is specified in the design to be a standard button like TextButton.

Unresolved issues

When updating an existing Test Execution Review, I managed to load the selected decisions. However, I couldn't find a way to load the review comment into the TextField widget.

@andrejvelichkovski andrejvelichkovski changed the title Add test execution review Implement frontend for test execution review Jan 2, 2024
@andrejvelichkovski andrejvelichkovski marked this pull request as ready for review January 2, 2024 16:47
Copy link
Collaborator

@omar-selo omar-selo left a comment

Choose a reason for hiding this comment

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

Thanks for the changes. Most are small points but one issue is about passing artefactId

frontend/lib/providers/artefact_builds.dart Outdated Show resolved Hide resolved
frontend/lib/providers/artefact_builds.dart Outdated Show resolved Hide resolved
frontend/pubspec.yaml Outdated Show resolved Hide resolved
frontend/lib/ui/artefact_dialog/test_execution_review.dart Outdated Show resolved Hide resolved
frontend/lib/ui/artefact_dialog/test_execution_review.dart Outdated Show resolved Hide resolved
frontend/lib/ui/artefact_dialog/test_execution_review.dart Outdated Show resolved Hide resolved
frontend/lib/ui/artefact_dialog/test_execution_review.dart Outdated Show resolved Hide resolved
frontend/lib/ui/artefact_dialog/test_execution_review.dart Outdated Show resolved Hide resolved
Copy link
Collaborator

@omar-selo omar-selo left a comment

Choose a reason for hiding this comment

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

Looks much better, just a few more comments

backend/test_observer/data_access/models.py Outdated Show resolved Hide resolved
frontend/lib/providers/artefact_builds.dart Outdated Show resolved Hide resolved
frontend/lib/providers/artefact_builds.dart Outdated Show resolved Hide resolved
Copy link
Contributor

@plars plars left a comment

Choose a reason for hiding this comment

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

It looks like the c3 link is still there in the code, but I don't see it in the screenshots you posted. Is that just something off with the screenshot, or was there some UI change that somehow removed the c3 link here?

@andrejvelichkovski
Copy link
Contributor Author

It looks like the c3 link is still there in the code, but I don't see it in the screenshots you posted. Is that just something off with the screenshot, or was there some UI change that somehow removed the c3 link here?

The C3 Link button is conditionally rendered, it just happened that none of these test executions have it set up in the database, so it is not shown on the frontend either.

Copy link
Collaborator

@omar-selo omar-selo left a comment

Choose a reason for hiding this comment

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

Looks great, good job!

@andrejvelichkovski andrejvelichkovski merged commit 9a94578 into main Jan 5, 2024
8 checks passed
@andrejvelichkovski andrejvelichkovski deleted the add-test-execution-review branch January 5, 2024 08:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants