Refactor AlertManagement::Alert to fully utilize state machine
Summary
Currently, AlertManagement::Alert
use a state machine to define statuses.
Additionally, it is defining mappings between state name and value and state name and event.
There are two problems with this approach:
- The defined mapping constants are used outside of the alert model leaking implementation details
- In the unlikely event of adding a new status is potentially error-prone because we need to add the state, define
STATUSES
and definedSTATUS_EVENT
Improvements
The main idea is to hide the implementation details behind an set of methods defined in the alert model so can remove these additional mappings (STATUSES
and STATUS_EVENTS
) and utilize state machine's features interally:
-
(MR !44589 (merged)) Change to value of
Types::AlertManagement::StatusEnum
to use names (triggered
,resolved
) instead of numerical value (0
,2
)⚠ -
UpdateService
-
(MR !44589 (merged)) Mapping
STATUSES
value -> name can be removed- Alternatively, if we cannot change the enum from above we can utilize
state_machines[:status].states[value, :value]
to lookup value by name
- Alternatively, if we cannot change the enum from above we can utilize
-
(MR !44309 (merged)) Replace hardcoded event mapping (
STATUS_EVENT
) withstate_machine.events.transitions_for(alert, :to => :resolved).first.event
-
(MR !44589 (merged)) Mapping
-
(MR !44589 (merged)) Let
AlertsFinder.counts_by_status
return names as keys instead of values -
(MR !44589 (merged)) Replace
AlertStatusCounts::STATUSES
withAlert.status_states
(or similar - callsstate_machines[:status].states
internally) -
(MR !44455 (merged)) Wrap
Alert::OPEN_STATUSES
with a method and private (viaprivate_constant
) to prevent leakage -
(MR !44280 (merged)) Use
scope :not_resolved, -> { without_status(:resolved) }
inAlert
model - Adjust specs
Risks
Involved components
-
Alert
model Types::AlertManagement::StatusEnum
AlertsFinder
AlertStatusCounts
- specs
List of files
rg "Alert::STATUS\w+" -l | sort
app/finders/alert_management/alerts_finder.rb
app/graphql/types/alert_management/status_enum.rb
app/services/alert_management/alerts/update_service.rb
lib/gitlab/alert_management/alert_status_counts.rb
spec/factories/alert_management/alerts.rb
spec/finders/alert_management/alerts_finder_spec.rb
spec/lib/gitlab/alert_management/alert_status_counts_spec.rb
spec/models/alert_management/alert_spec.rb
spec/services/alert_management/alerts/update_service_spec.rb
spec/services/projects/alerting/notify_service_spec.rb
Optional: Intended side effects
This refactoring should simplify the interaction with defined alert status states and improve readability