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

Develop into master #4393

Merged
merged 94 commits into from
Oct 7, 2024
Merged
Show file tree
Hide file tree
Changes from 40 commits
Commits
Show all changes
94 commits
Select commit Hold shift + click to select a range
8d6d55e
Add migration for unique-together index on ancestor and descendant
yoldas Sep 12, 2024
5896e32
Applied migration to db schema
yoldas Sep 12, 2024
b9661b0
Pass through control assets to control locator
SHIV5T3R Sep 19, 2024
8a58ffd
Add function to inversely populate control wells on the destination p…
SHIV5T3R Sep 19, 2024
4e9934d
Rubocop
SHIV5T3R Sep 19, 2024
2a76fb5
Prettier
SHIV5T3R Sep 19, 2024
6c3ef2c
Added dev notes for converstion function
SHIV5T3R Sep 19, 2024
ab1f587
Include column names in the unique index
yoldas Sep 23, 2024
775d85e
Apply migration to schema
yoldas Sep 23, 2024
1b0fb4e
Override create_edge on asset_links to handle race conditions
yoldas Sep 23, 2024
e601915
Merge branch 'develop' into 4133-y24-120-bug-taken-link-error-when-cr…
yoldas Sep 23, 2024
d720c8e
Bubble up non-duplication errors
yoldas Sep 23, 2024
6b9b83d
Remove custom exception class definitions from test
yoldas Sep 23, 2024
480e3d3
Update comment about return values
yoldas Sep 23, 2024
937460d
Merge branch 'develop' into 4133-y24-120-bug-taken-link-error-when-cr…
yoldas Sep 23, 2024
dc6a704
Add guard against nil before converting link to edge
yoldas Sep 24, 2024
80d377a
Replace the recover proecedure with auditing as it is wrong to put du…
yoldas Sep 24, 2024
bac2c98
Rubocop
yoldas Sep 24, 2024
03124f4
Clean the database after tests because of the new DB connections in t…
yoldas Sep 24, 2024
dee9a7e
Removed after block
yoldas Sep 24, 2024
c4a13a8
Add after all cleanup
yoldas Sep 24, 2024
5862b9f
Merge branch 'develop' into 4133-y24-120-bug-taken-link-error-when-cr…
yoldas Sep 24, 2024
165dd5e
Delete test asset link and labware records
yoldas Sep 24, 2024
6f50b2c
Debug side effect
yoldas Sep 24, 2024
f04d1cf
Debug 2
yoldas Sep 24, 2024
bc857a2
Revert debug
yoldas Sep 24, 2024
0aec888
Debug handles race condition at find_link
yoldas Sep 24, 2024
a69265a
Debug2 handles race condition at find_link
yoldas Sep 24, 2024
4ebd702
Rewrite test for unique validation error
yoldas Sep 25, 2024
9ea98ae
Rewrite example handles unique constraint violation
yoldas Sep 25, 2024
b393fdd
Update tests for create_edge method
yoldas Sep 25, 2024
0ac25f7
Deleted old version of the tests
yoldas Sep 25, 2024
ca87241
Remove redundant require
yoldas Sep 25, 2024
068d0b9
Merge branch 'develop' into 4133-y24-120-bug-taken-link-error-when-cr…
yoldas Sep 25, 2024
7a691cf
Pass control source plate to control locator
SHIV5T3R Sep 27, 2024
868631e
Added control placement type attr from custom metadata
SHIV5T3R Sep 27, 2024
ca3a8eb
Merge branch 'develop' into 4133-y24-120-bug-taken-link-error-when-cr…
yoldas Sep 27, 2024
aa3955c
Added logic for handling random and fixed placement
SHIV5T3R Sep 27, 2024
a1864ff
Added control placement type to control plate options in UI
SHIV5T3R Sep 27, 2024
f37337d
Rubocop
SHIV5T3R Sep 27, 2024
51913bf
Prettier
SHIV5T3R Sep 27, 2024
e0d0b01
Control plates won't show without required metadata
SHIV5T3R Sep 27, 2024
e3a4a33
Added exception handling for unknown placement type
SHIV5T3R Sep 27, 2024
66ab9d2
Merge branch 'develop' into y24-304-cherrypick-fixed-placement
SHIV5T3R Sep 27, 2024
5f03234
'control_source_plate' is now a required attribute
SHIV5T3R Sep 27, 2024
440f1df
Fixed syntax
SHIV5T3R Sep 27, 2024
c29b516
Changed control placement type values
SHIV5T3R Sep 27, 2024
23dd8f1
Added control placement type to control plate factory
SHIV5T3R Sep 27, 2024
1abbc05
Create control plate with new metadata in spec
SHIV5T3R Sep 27, 2024
4dd1e2a
Added control plate to cherrypick task spec
SHIV5T3R Sep 27, 2024
bf60f4b
Make control source plate optional for control locator
SHIV5T3R Sep 27, 2024
c54563a
Rubocop
SHIV5T3R Sep 27, 2024
7b9bb2a
Merge branch 'develop' into y24-304-cherrypick-fixed-placement
SHIV5T3R Sep 27, 2024
27fde60
Modification to inline documenation
SHIV5T3R Sep 27, 2024
6b9cad6
Rubocop
SHIV5T3R Sep 27, 2024
4d5822b
Rubocop....
SHIV5T3R Sep 27, 2024
a4e60ee
Syntax
SHIV5T3R Sep 27, 2024
49dc78c
Syntax (again)
SHIV5T3R Sep 27, 2024
506b49a
Yard doc
SHIV5T3R Sep 27, 2024
efdf4da
Pass plate template to control locator
SHIV5T3R Oct 1, 2024
1a1930f
Added method to handle incompatible plates
SHIV5T3R Oct 1, 2024
ccbdc13
Modifications to existing tests
SHIV5T3R Oct 1, 2024
b5f1d48
Added test for invalid placement type
SHIV5T3R Oct 1, 2024
ef144f9
Added test for plate compatibilty check
SHIV5T3R Oct 1, 2024
5cbdc32
Linting
SHIV5T3R Oct 1, 2024
63c00b8
Added test for invalid placement types
SHIV5T3R Oct 1, 2024
a761d37
Refactored test fror checking plate compatibility
SHIV5T3R Oct 1, 2024
6a1850a
Added test coverage for asset map conversion
SHIV5T3R Oct 1, 2024
230b4d0
Fixed weird rubocop/prettier loop
SHIV5T3R Oct 1, 2024
5bdc03e
Refactored method for handling incompatible plates
SHIV5T3R Oct 1, 2024
0e8b3ae
Display an error to user if plates are incompatible
SHIV5T3R Oct 1, 2024
59d9c5b
Modified existing tests
SHIV5T3R Oct 1, 2024
36a9dc0
Added test coverage for expecting a displayed error
SHIV5T3R Oct 1, 2024
c2dda46
Added a test for valid fixed control placement
SHIV5T3R Oct 1, 2024
00d5d9b
Improved test coverage for the control locator
SHIV5T3R Oct 1, 2024
1165090
Improved test coverage (again)
SHIV5T3R Oct 1, 2024
80954a0
Add UAT action to generate randomised FluidX barcodes
yoldas Oct 2, 2024
5bb6bb8
Fill errors collection instead of raising exception
yoldas Oct 2, 2024
8531eae
Continue index in the next iteration
yoldas Oct 2, 2024
693f6b1
Converted control locator init method to use single arg params
SHIV5T3R Oct 2, 2024
c326657
Removed 'available_positions' from param documentation
SHIV5T3R Oct 2, 2024
f7a918a
Separate random part and index with zero
yoldas Oct 2, 2024
e3bfcb2
Merge branch 'develop' into 4133-y24-120-bug-taken-link-error-when-cr…
yoldas Oct 2, 2024
e81b4a0
Deleted file as it does not belong to this PR
yoldas Oct 2, 2024
c9a058a
Rubocop
yoldas Oct 2, 2024
a9ff092
Deleting the part-1 test as it is conflicting with part-2
yoldas Oct 3, 2024
4428fee
Merge pull request #4340 from sanger/4133-y24-120-bug-taken-link-erro…
yoldas Oct 3, 2024
7cd7d95
add class AkerBarcode to handle calling this method elsewhere
wendyyang Oct 3, 2024
4cbfdfe
Update .release-version to 14.43.2
yoldas Oct 3, 2024
bf10f84
change the method & add test for the method
wendyyang Oct 3, 2024
2a5b8c6
Merge branch 'master' into develop
yoldas Oct 3, 2024
7eb412f
add comment to the added method
wendyyang Oct 3, 2024
ed77842
Merge pull request #4392 from sanger/4310-y24-289-bug-exception
wendyyang Oct 3, 2024
6b4ecdb
Merge pull request #4385 from sanger/y24-304-cherrypick-fixed-placement
SHIV5T3R Oct 7, 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
2 changes: 1 addition & 1 deletion .release-version
Original file line number Diff line number Diff line change
@@ -1 +1 @@
14.43.0
14.43.2
77 changes: 77 additions & 0 deletions app/models/asset_link.rb
Original file line number Diff line number Diff line change
Expand Up @@ -76,4 +76,81 @@ def has_#{name}?
end
end
end

# Creates an edge between the ancestor and descendant nodes using save.
#
# This method first attempts to find an existing link between the ancestor
# and descendant. If no link is found, it builds a new edge and saves it.
# If a link is found, it makes the link an edge and saves it.
#
# This method is overridden to handle race conditions in finding an
# existing link and has_duplicates validation. It also assumes that there
# is a unique-together index on ancestor_id and descendant_id columns.
#
# @param ancestor [Dag::Standard::EndPoint] The ancestor node.
# @param descendant [Dag::Standard::EndPoint] The descendant node.
# @return [Boolean] Returns true if the edge is successfully created or
# already exists, false otherwise.
# @raise [ActiveRecord::RecordNotUnique] Re-raises any exception if it is
# not a constraint violation that involves ancestor_id and descendant_id
# columns.
def self.create_edge(ancestor, descendant)
# Two processes try to find an existing link.
link = find_link(ancestor, descendant)
# Either or both may find no link and try to create a new edge.
if link.nil?
edge = build_edge(ancestor, descendant)
result = save_edge_or_handle_error(edge)
return result unless result.nil? # Bubble up.
# Losing process finds the edge created by the winning process.
link = find_link(ancestor, descendant)
end

return if link.nil?

link.make_direct
link.changed? ? link.save : true
end

# Saves the edge between the ancestor and descendant nodes or handles errors.
#
# @param edge [AssetLink] The edge object containing the errors.
# @return [Boolean] Returns true if the edge is successfully saved,
# nil if the error is unique validation or constraint violation,
# false if the error is another validation error.
# @raise [ActiveRecord::RecordNotUnique] Re-raises an exception if the
# exception caught is not a unique constraint violation.
def self.save_edge_or_handle_error(edge)
# Winning process successfully saves the edge (direct link).
return true if edge.save
# has_duplicate validation may see it for the losing process before
# hitting the DB.
return false unless unique_validation_error?(edge) # Bubble up.
edge.errors.clear # Clear all errors and use the existing link.
rescue ActiveRecord::RecordNotUnique => e
# Unique constraint violation is triggered for the losing process after
# hitting the DB.
raise unless unique_violation_error?(edge, e) # Bubble up.
end

# Checks if the validation error includes a specific message indicating a
# unique link already exists.
#
# @param edge [AssetLink] The edge object containing the errors.
# @return [Boolean] Returns true if the errors include the message "Link
# already exists between these points", false otherwise.
def self.unique_validation_error?(edge)
edge.errors[:base].include?('Link already exists between these points')
end

# Checks if the unique constraint violation involves the specified columns.
#
# @param edge [AssetLink] The edge object containing the column names.
# @param exception [ActiveRecord::RecordNotUnique] The exception raised due
# to the unique constraint violation.
# @return [Boolean] Returns true if the exception message includes both the
# ancestor and descendant column names, false otherwise.
def self.unique_violation_error?(edge, exception)
[edge.ancestor_id_column_name, edge.descendant_id_column_name].all? { |col| exception.message.include?(col) }
end
end
26 changes: 26 additions & 0 deletions db/migrate/20240912000109_add_unique_index_to_asset_links.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
# frozen_string_literal: true

# This migration adds a unique-together index on ancestor_id and descendant_id
# in order to prevent duplicate links between the same ancestor and descendant
# labware.
#
# Before this migration, the database allowed duplicate asset_links between the
# same ancestor and descendant labware. Therefore, the migration will fail if
# there are any duplicate links in the database. To fix this, they must be
# removed before running the migration using the rake task:
#
# bundle exec rake 'support:remove_duplicate_asset_links[csv_file_path]'
#
# The rake task will write the removed records into a CSV file that can be used
# for auditing purposes.
#
# Note that the column names in the index name below is used for finding the
# reason of the database unique constraint violation by the AssetLink model.
class AddUniqueIndexToAssetLinks < ActiveRecord::Migration[6.1]
def change
add_index :asset_links,
%i[ancestor_id descendant_id],
unique: true,
name: 'index_asset_links_on_ancestor_id_and_descendant_id'
end
end
1 change: 1 addition & 0 deletions db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@
t.integer "count"
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
t.index ["ancestor_id", "descendant_id"], name: "index_asset_links_on_ancestor_id_and_descendant_id", unique: true
t.index ["ancestor_id", "direct"], name: "index_asset_links_on_ancestor_id_and_direct"
t.index ["descendant_id", "direct"], name: "index_asset_links_on_descendant_id_and_direct"
end
Expand Down
Loading
Loading