Y24-130 - Fix asset controller test for NPG actions with Rails 6.1 defaults

[dasunpubudumal]

Describe the Housekeeping

We have been using Rails 5.0 as our defaults, and are in the process of upgrading it to 6.1. The story that started the process of migrating the defaults to Rails 6.1 is https://github.com/sanger/sequencescape/issues/4005. Note that upgrading the Rails version in the Gemfile to 6.1 - which we did in https://github.com/sanger/sequencescape/issues/4006 - does not necessarily upgrade the Framework Defaults to 6.1. We need to update config/application.rb file's config.load_defaults value to 6.1, and it was done in https://github.com/sanger/sequencescape/issues/4005.

There were certain tests failing, including one Minitest, and we fixed them except for one test, which is spec/controllers/npg_actions/assets_controller_spec.rb:181.

The reason that this test fails is described below.

Take the following code snippet from app/models/batch_request.rb, which is the join table associating the Batch model and the Request model with a one-to-many relationship. The snippet is from the before_validation callback that is assigned to the BatchRequest model.

before_validation(if: :requires_position?, unless: :position?) do |record|
  record.position = (record.batch.batch_requests.filter_map(&:position).max || 0) + 1
end

Note: Since this is part of the validation process, any failure for this callback would inevitably lead to a failure to persist a batch_request record, thereby failing to create an association between a given batch and a request.

• When using Rails 5.0 defaults, the expression record.batch.batch_requests will return [] before validating the first request (with the passed state). However, when invoking the before_validation call for the second request (with the cancelled state), it also returns [] when it should return the first batch_request record in the list.
• Because of this, the proposition

  requests.all? { |request| request.asset.resource? || request.events.family_pass_and_fail.exists? }

  in app/models/batch.rb:577 returns true when using Rails 5.0 as it does not return the cancelled request for requests association.

• When using Rails 6.1 defaults, the same expression will return [] before validating the first request as in Rails 5.0. However, it correctly returns the first batch_request association created when persisting the second request. Because of this, the proposition listed in the point before returns false, as the cancelled request is returned for the requests association, preventing the state of the batch from transitioning to released.

The purpose of this story is to investigate why this occur and provide a fix for the issue.

Furthermore, it might be beneficial to setup an actual use-case for this NPG action and invoke the endpoint that the test tries to invoke. This might help to localise whether if the issue is only within the test context.

---

Update: Please refer to the comments in https://github.com/sanger/sequencescape/issues/4138#issuecomment-2163397002 as well.

• [ ] Fix the asset_controller test.
• [ ] Run the integration suite and - if found - fix the issues.

Blocking issues Describe any other issues or tickets that may be blocking this change.

Additional context Add any other context about the problem here.

[TWJW-SANGER]

Thanks Dasun.

I would add the following summary from the meeting. Feel free to correct if I've got things wrong.

For this story we require:

• [ ] The response & behavior of the npg/qc_passed API endpoint in Rails version 6.1 should be identical to the current Rails 5.x version, when there is one cancelled and one valid request for a batch.

If we believe the current endpoint behavior is incorrect we will create another story to implement the correct behavior.

If the endpoint behaviour between Rails versions is consistent we can either rework the test so it works in Rails 6 or comment it out for re-implementation in another story.

If the endpoint behaviour between Rails versions is not consistent then we will need to take a view on possible options to correct or mitigate this.

It should be possible to test the endpoint under different Rails versions using the UAT environment, having set up the requests & batches manually.