-
-
Notifications
You must be signed in to change notification settings - Fork 4.3k
feat(aci): automation creation scaffolding #89233
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Conversation
7484c94
to
e0183ce
Compare
e0183ce
to
fdb1e6d
Compare
fdb1e6d
to
27bb72e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really great start!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR should explain why two new npm dependencies are required.
Preferably that'll include a note about what an alternative implementation might be without the packages, and why one is better than the other.
Codecov ReportAll modified and coverable lines are covered by tests ✅ ✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## master #89233 +/- ##
=======================================
Coverage 87.73% 87.73%
=======================================
Files 10162 10162
Lines 573696 573695 -1
Branches 22597 22597
=======================================
Hits 503305 503305
+ Misses 69957 69956 -1
Partials 434 434 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking really awesome! Left some suggestions for refactoring with an eye on readability/maintainability moving forward.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few final things, then this should be good to merge!
static/app/views/automations/components/automationBuilderContext.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small changes but otherwise lgtm!
static/app/views/automations/components/actionFilters/ageComparison.tsx
Outdated
Show resolved
Hide resolved
static/app/views/automations/components/actionFilters/ageComparison.tsx
Outdated
Show resolved
Hide resolved
static/app/views/automations/components/actionFilters/issueOccurrences.tsx
Show resolved
Hide resolved
static/app/views/automations/components/automationBuilderContext.tsx
Outdated
Show resolved
Hide resolved
scaffolding for step 2 of automation creation at `/new-settings` this introduces a new npm dependency, [`flattie`](https://github.com/lukeed/flattie), in order to easily sync the automation form data to the automation builder state. - **the problem:** the automation builder includes lots of nesting (data condition groups >> data conditions (which can also have nested data condition groups!) >> comparison input fields). see types [here](https://github.com/getsentry/sentry/blob/c980de65aed1c79be7f3d08b81f1eeef2d89382d/static/app/types/workflowEngine/dataConditions.tsx#L65-L79) - **alternative solution without these packages:** we would have to manually iterate through the automation details in order to populate the form model's data. - **preferred solution with these packages:** `flattie` is used to take an automation details result from the API and flatten it for the form model to understand. this utility is very small (`203B`), and there would be very little benefit in maintaining a version ourselves. view preview [here](https://sentry-4xprfca9b.sentry.dev/issues/automations/new/settings) https://github.com/user-attachments/assets/b1cbcea9-8e4e-4865-a404-1812954dd1b8
scaffolding for step 2 of automation creation at
/new-settings
this introduces a new npm dependency,
flattie
, in order to easily sync the automation form data to the automation builder state.flattie
is used to take an automation details result from the API and flatten it for the form model to understand. this utility is very small (203B
), and there would be very little benefit in maintaining a version ourselves.view preview here
Screen.Recording.2025-04-14.at.5.54.59.PM.mov