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

Introduce an interval field to capture workflows #1358

Merged
merged 11 commits into from
Nov 15, 2024

Conversation

kiahna-tucker
Copy link
Member

@kiahna-tucker kiahna-tucker commented Nov 6, 2024

Issues

The issues directly below are advanced by this PR:
#1194

Changes

1194

The following features are included in this PR:

  • Simplify the logic for the component that exposes the interval property in capture workflows.

Tests

Manually tested

Approaches to testing are as follows:

  • Validate that the capture interval section does not appear in materialization workflows.

  • Validate that the capture interval section does not appear in capture workflows when the selected connector does not specify a default capture interval.

  • Validate that the field is prefilled with the default interval in the capture create workflow when the selected connector specifies one.

  • Validate that the field is prefilled with the default interval in the capture edit workflow when the live specification does not specify an interval but a default capture interval exists.

  • Validate that the field is prefilled with the interval specified in the live specification in the capture edit workflow when it exists.

  • Validate that the field considers input in an unsupported interval format invalid.

Automated tests

Test coverage updated for the following utils:

  • formatCaptureInterval

Playwright tests ran locally

  • Admin
  • Captures
  • Collections
  • HomePage
  • Login
  • Materialization

Screenshots

Capture interval field | Value selected

pr_screenshot-1358-capture_interval-standard-single_unit_interval

Capture interval field | Tooltip

pr_screenshot-1319-capture_interval-standard-tooltip

Capture interval field | Predefined interval options

pr_screenshot-1358-capture_interval-standard-menu-options

Capture interval field | Empty

pr_screenshot-1358-capture_interval-standard-empty

Capture interval field-level error

pr_screenshot-1358-capture_interval-standard-error-input-compound

@kiahna-tucker kiahna-tucker added the change:planned This is a planned change label Nov 6, 2024
@kiahna-tucker kiahna-tucker marked this pull request as ready for review November 6, 2024 20:36
@kiahna-tucker kiahna-tucker requested a review from a team as a code owner November 6, 2024 20:36
@travjenkins

This comment was marked as resolved.

src/utils/time-utils.ts Outdated Show resolved Hide resolved
@travjenkins

This comment was marked as resolved.

Copy link
Member

@travjenkins travjenkins left a comment

Choose a reason for hiding this comment

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

lgtm - didn't manually test the last round of changes

@kiahna-tucker kiahna-tucker merged commit 46875a6 into main Nov 15, 2024
3 checks passed
@kiahna-tucker kiahna-tucker deleted the kiahna-tucker/capture-interval/simplify-field branch November 15, 2024 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
change:planned This is a planned change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants