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

Y24-396: Refactor Automatic State Changers #2044

Open
wants to merge 18 commits into
base: y24-088-tuberacks-epic
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
0f7e5f1
refactor: convert AutomaticBehaviour from a class to a mixin module
StephenHulme Oct 30, 2024
cc7244d
tests: refactor tests to match refactored state changes
StephenHulme Oct 30, 2024
6c1916e
tests: refactor to pull out AutomaticPlateStateChanger as shared example
StephenHulme Oct 30, 2024
01706b5
tests: add tests for TubeStateChanger
StephenHulme Oct 31, 2024
f0590e1
tests: add tests for AutomaticTubeStateChanger
StephenHulme Oct 31, 2024
1c1d102
tests: expand stub_v2_tube to duplicate stub_v2_plate
StephenHulme Oct 31, 2024
0750b3a
tests: return v2_tube for automated state changers
StephenHulme Oct 31, 2024
43ff5b9
tests: use a better fort of tube purpose
StephenHulme Oct 31, 2024
a067948
refactor: rename variable for increased clarity
StephenHulme Nov 1, 2024
1a445c2
tests: use consistent library name of limber_bespoke_aggregation
StephenHulme Nov 1, 2024
ce90421
refactor: replace DefaultStateChanger with PlateStateChanger
StephenHulme Nov 1, 2024
a5bfc7a
fix: repair refactoring regressions
StephenHulme Nov 1, 2024
857bdf2
Merge branch 'develop' into y24-396-refactor-state-changes-using-mixins
StephenHulme Nov 15, 2024
4a5018d
Merge branch 'y24-088-tuberacks-epic' into y24-396-refactor-state-cha…
StephenHulme Dec 2, 2024
a265c2e
tests: refactor state changers spec to describe each class
StephenHulme Dec 2, 2024
6336534
tests: add test for tube state changer
StephenHulme Dec 2, 2024
08c7851
style: remove abstract method errors from coverage
StephenHulme Dec 9, 2024
4d724e1
tests: remove references to tube aggregation
StephenHulme Dec 10, 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
157 changes: 91 additions & 66 deletions app/models/state_changers.rb
Original file line number Diff line number Diff line change
@@ -1,16 +1,67 @@
# frozen_string_literal: true

# A State changer is responsible for transitioning the state of a plate, or
# individual wells.
# A State changer is responsible for transitioning the state of labware and its receptacles where applicable.
#
# Hierarchy:
# - BaseStateChanger
# - PlateStateChanger
# - AutomaticPlateStateChanger [includes AutomaticBehaviour]
# - TubeRackStateChanger
# - AutomaticTubeRackStateChanger [includes AutomaticBehaviour]
# - TubeStateChanger
# - AutomaticTubeStateChanger [includes AutomaticBehaviour]
#
module StateChangers
class StateChangeError < StandardError
def self.lookup_for(purpose_uuid)
(details = Settings.purposes[purpose_uuid]) || raise("Unknown purpose UUID: #{purpose_uuid}")
details[:state_changer_class].constantize
end

# The Default State changer is used by the vast majority of plates. It creates
# a simple StateChange record in Sequencescape, specifying the new 'target-state'
# As a result, Sequencescape will attempt to transition the entire plate, or the
# specified wells.
class DefaultStateChanger
# Plate state changer to automatically complete specified work requests.
# This is the abstract behaviour.
#
# Must include v2_labware method in the including class
module AutomaticBehaviour
def purpose_uuid
@purpose_uuid ||= v2_labware.purpose.uuid
end

def purpose_config
@purpose_config ||= Settings.purposes[purpose_uuid]
end

def work_completion_request_types
@work_completion_request_types ||= parse_work_completion_request_types
end

# config can be a single request type or an array of request types
# convert them here into a consistent array format
def parse_work_completion_request_types
config = purpose_config[:work_completion_request_type]
return config if config.is_a?(Array)

[config]
end

# rubocop:todo Style/OptionalBooleanParameter
def move_to!(state, reason = nil, customer_accepts_responsibility = false)
super
complete_outstanding_requests
end

# rubocop:enable Style/OptionalBooleanParameter

def complete_outstanding_requests
in_progress_submission_uuids =
v2_labware.in_progress_submission_uuids(request_types_to_complete: work_completion_request_types)
return if in_progress_submission_uuids.blank?

api.work_completion.create!(submissions: in_progress_submission_uuids, target: v2_labware.uuid, user: user_uuid)
end
end

# The Base state changer that contains common behaviour for all state changers.
class BaseStateChanger
attr_reader :labware_uuid, :api, :user_uuid
private :api

Expand Down Expand Up @@ -55,6 +106,20 @@
#
# @param target_state [String] the state to check against the FILTER_FAILS_ON list
# @return [Array<String>, nil] an array of well locations requiring the state change, or nil if no change is needed
def contents_for(_target_state)
raise 'Must be implemented on subclass' # pragma: no cover

Check warning on line 110 in app/models/state_changers.rb

View check run for this annotation

Codecov / codecov/patch

app/models/state_changers.rb#L110

Added line #L110 was not covered by tests
end

def labware
raise 'Must be implemented on subclass' # pragma: no cover

Check warning on line 114 in app/models/state_changers.rb

View check run for this annotation

Codecov / codecov/patch

app/models/state_changers.rb#L114

Added line #L114 was not covered by tests
end
end

# The Plate State changer is used by the vast majority of plates. It creates
# a simple StateChange record in Sequencescape, specifying the new 'target-state'
# As a result, Sequencescape will attempt to transition the entire plate, or the
# specified wells.
class PlateStateChanger < BaseStateChanger
def contents_for(target_state)
return nil unless FILTER_FAILS_ON.include?(target_state)

Expand All @@ -72,14 +137,9 @@
end
end

def self.lookup_for(purpose_uuid)
(details = Settings.purposes[purpose_uuid]) || raise("Unknown purpose UUID: #{purpose_uuid}")
details[:state_changer_class].constantize
end

# The tube rack state changer is used by TubeRacks.
# It contains racked tubes.
class TubeRackStateChanger < DefaultStateChanger
class TubeRackStateChanger < BaseStateChanger
# This method determines the coordinates of tubes that require a state change based on the target state.
# If the target state is not in the FILTER_FAILS_ON list, it returns nil.
# It filters out tubes that are in the 'failed' state and collects their coordinates.
Expand Down Expand Up @@ -111,81 +171,46 @@
end

# The tube state changer is used by Tubes. It works the same way as the default
# state changer but does not need to handle a subset of wells like the plate.
class TubeStateChanger < DefaultStateChanger
# plate state changer but does not need to handle a subset of wells like the plate.
class TubeStateChanger < BaseStateChanger
# Tubes have no wells so contents is always empty
def contents_for(_target_state)
nil
end
end

# Plate state changer to automatically complete specified work requests.
# This is the abstract version.
class AutomaticLabwareStateChanger < DefaultStateChanger
def v2_labware
raise 'Must be implemented on subclass'
end

def purpose_uuid
@purpose_uuid ||= v2_labware.purpose.uuid
end

def purpose_config
@purpose_config ||= Settings.purposes[purpose_uuid]
end

def work_completion_request_types
@work_completion_request_types ||= parse_work_completion_request_types
end

# config can be a single request type or an array of request types
# convert them here into a consistent array format
def parse_work_completion_request_types
config = purpose_config[:work_completion_request_type]
return config if config.is_a?(Array)

[config]
end

# rubocop:todo Style/OptionalBooleanParameter
def move_to!(state, reason = nil, customer_accepts_responsibility = false)
super
complete_outstanding_requests
end

# rubocop:enable Style/OptionalBooleanParameter

def complete_outstanding_requests
in_prog_submissions =
v2_labware.in_progress_submission_uuids(request_types_to_complete: work_completion_request_types)
return if in_prog_submissions.blank?

api.work_completion.create!(submissions: in_prog_submissions, target: v2_labware.uuid, user: user_uuid)
def labware
raise 'Tubes are not supported by API V1'

Check warning on line 182 in app/models/state_changers.rb

View check run for this annotation

Codecov / codecov/patch

app/models/state_changers.rb#L182

Added line #L182 was not covered by tests
Copy link
Member

Choose a reason for hiding this comment

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

That can't be true?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not a path that appears to be used in the current iteration - I'm hoping it will be removed with @sdjmchattie's v1 API removal changes

end
end

# This version of the AutomaticLabwareStateChanger is used by Plates.
class AutomaticPlateStateChanger < AutomaticLabwareStateChanger
class AutomaticPlateStateChanger < PlateStateChanger
include AutomaticBehaviour

def v2_labware
@v2_labware ||= Sequencescape::Api::V2.plate_for_completion(labware_uuid)
end
end

# This version of the AutomaticLabwareStateChanger is used by Tubes.
class AutomaticTubeStateChanger < AutomaticLabwareStateChanger
# This version of the AutomaticLabwareStateChanger is used by TubeRacks.
class AutomaticTubeRackStateChanger < TubeRackStateChanger
include AutomaticBehaviour

def v2_labware
@v2_labware ||= Sequencescape::Api::V2.tube_for_completion(labware_uuid)
@v2_labware ||= Sequencescape::Api::V2.tube_rack_for_completion(labware_uuid)

Check warning on line 200 in app/models/state_changers.rb

View check run for this annotation

Codecov / codecov/patch

app/models/state_changers.rb#L200

Added line #L200 was not covered by tests
end

def labware
Copy link
Member

Choose a reason for hiding this comment

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

this labware vs v2_labware nonsense will hopefully get refactored out in a later story!

@labware ||= v2_labware
end
end

# This version of the AutomaticLabwareStateChanger is used by TubeRacks.
class AutomaticTubeRackStateChanger < AutomaticLabwareStateChanger
# This version of the AutomaticLabwareStateChanger is used by Tubes.
class AutomaticTubeStateChanger < TubeStateChanger
include AutomaticBehaviour

def v2_labware
@v2_labware ||= Sequencescape::Api::V2.tube_rack_for_completion(labware_uuid)
@v2_labware ||= Sequencescape::Api::V2.tube_for_completion(labware_uuid)

Check warning on line 213 in app/models/state_changers.rb

View check run for this annotation

Codecov / codecov/patch

app/models/state_changers.rb#L213

Added line #L213 was not covered by tests
end

def labware
Expand Down
8 changes: 4 additions & 4 deletions docs/purposes_yaml_files.md
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ Example plate:
:input_plate: false
:size: 96
:presenter_class: Presenters::StandardPresenter
:state_changer_class: StateChangers::DefaultStateChanger
:state_changer_class: StateChangers::PlateStateChanger
:creator_class: LabwareCreators::TaggedPlate
:default_printer_type: :plate_a
```
Expand Down Expand Up @@ -239,15 +239,15 @@ purpose. State changers are used on updating labware state, either via a
{LabwareController#update}. In the vast majority of cases this can be left as
the default option.

Valid options are subclasses of {StateChangers::DefaultStateChanger}.
Valid options are subclasses of {StateChangers::PlateStateChanger}.

{file:docs/state_changers.md Full list of state changers and their behaviour}

```yaml
:state_changer_class: StateChangers::DefaultStateChanger
:state_changer_class: StateChangers::PlateStateChanger
```

Default: `StateChangers::DefaultStateChanger`
Default: `StateChangers::PlateStateChanger`

#### :creator_class

Expand Down
2 changes: 1 addition & 1 deletion lib/purpose_config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ class PurposeConfig

class_attribute :default_state_changer, :default_options

self.default_state_changer = 'StateChangers::DefaultStateChanger'
self.default_state_changer = 'StateChangers::PlateStateChanger'

def self.load(name, options, store, submission_templates, label_template_config)
case options.fetch(:asset_type)
Expand Down
4 changes: 1 addition & 3 deletions spec/controllers/robots_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,7 @@

setup do
Settings.robots['robot_id'] = settings[:robots][:robot_id]
create :purpose_config,
uuid: 'target_plate_purpose_uuid',
state_changer_class: 'StateChangers::DefaultStateChanger'
create :purpose_config, uuid: 'target_plate_purpose_uuid', state_changer_class: 'StateChangers::PlateStateChanger'
stub_v2_user(user)
stub_v2_plate(plate)
bed_labware_lookup(plate)
Expand Down
4 changes: 2 additions & 2 deletions spec/factories/purpose_config_factories.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
name { 'Plate Purpose' }
creator_class { 'LabwareCreators::StampedPlate' }
presenter_class { 'Presenters::StandardPresenter' }
state_changer_class { 'StateChangers::DefaultStateChanger' }
state_changer_class { 'StateChangers::PlateStateChanger' }
default_printer_type { :plate_a }
asset_type { 'plate' }
label_class { 'Labels::PlateLabel' }
Expand Down Expand Up @@ -357,7 +357,7 @@
asset_type { 'tube_rack' }
default_printer_type { :tube_rack }
presenter_class { 'Presenters::TubeRackPresenter' }
state_changer_class { 'StateChangers::DefaultStateChanger' }
state_changer_class { 'StateChangers::PlateStateChanger' }
submission { {} }
label_class { nil }
printer_type { '96 Well Plate' }
Expand Down
2 changes: 1 addition & 1 deletion spec/features/plate_transfer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@
message: ''
)

create :purpose_config, uuid: 'lb_end_prep_uuid', state_changer_class: 'StateChangers::DefaultStateChanger'
create :purpose_config, uuid: 'lb_end_prep_uuid', state_changer_class: 'StateChangers::PlateStateChanger'

bed_labware_lookup(example_plate)
stub_v2_plate(example_plate)
Expand Down
2 changes: 1 addition & 1 deletion spec/models/robots/robot_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -723,7 +723,7 @@
end

before do
create :purpose_config, uuid: 'lb_end_prep_uuid', state_changer_class: 'StateChangers::DefaultStateChanger'
create :purpose_config, uuid: 'lb_end_prep_uuid', state_changer_class: 'StateChangers::PlateStateChanger'
bed_labware_lookup(plate)
end

Expand Down
2 changes: 1 addition & 1 deletion spec/models/robots/splitting_robot_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@
end

before do
create :purpose_config, uuid: 'lb_end_prep_uuid', state_changer_class: 'StateChangers::DefaultStateChanger'
create :purpose_config, uuid: 'lb_end_prep_uuid', state_changer_class: 'StateChangers::PlateStateChanger'
bed_plate_lookup(plate, [:purpose, { wells: :downstream_plates }])
end

Expand Down
Loading
Loading