feat(inventory): support chemical associations for sbmm samples#2980
feat(inventory): support chemical associations for sbmm samples#2980
Conversation
9c1ff27 to
cbf1f7f
Compare
There was a problem hiding this comment.
Pull request overview
This pull request adds support for associating chemical data with sequence-based macromolecule (SBMM) samples, enabling inventory management for proteins and other macromolecules. The feature extends the existing chemical inventory system (previously only available for regular samples) to work with SBMM samples through a new polymorphic association.
Changes:
- Added database migration and foreign key to associate chemicals with SBMM samples
- Implemented exclusive-or validation in Chemical model to ensure it belongs to either a Sample OR an SBMM sample, not both
- Extended frontend ChemicalTab component to support both sample types with conditional rendering (e.g., hiding Safety tab for SBMM)
- Added inventory toggle and tab to SBMM sample details interface
Reviewed changes
Copilot reviewed 20 out of 21 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| db/migrate/20251017_add_sequence_based_macromolecule_sample_id_to_chemicals.rb | Adds foreign key column to link chemicals with SBMM samples |
| db/migrate/20260220000000_add_inventory_sample_to_sequence_based_macromolecule_samples.rb | Adds inventory_sample boolean flag to SBMM samples |
| db/migrate/20260220000001_add_index_inventory_sample_to_sequence_based_macromolecule_samples.rb | Adds index on inventory_sample for performance |
| app/models/chemical.rb | Adds SBMM association with validations ensuring exclusive parent relationship |
| app/models/sequence_based_macromolecule_sample.rb | Adds has_one :chemical relationship |
| spec/models/chemical_spec.rb | Tests for parent validation logic (both, neither, one parent scenarios) |
| spec/factories/chemicals.rb | Factory trait for SBMM-associated chemicals |
| app/api/chemotion/chemical_api.rb | Updates CRUD endpoints to handle both sample and SBMM types |
| app/api/helpers/params_helpers.rb | Adds inventory_sample parameter for SBMM samples |
| app/api/entities/sequence_based_macromolecule_sample_entity.rb | Exposes inventory_sample field |
| app/javascript/src/components/chemicals/ChemicalTab.js | Adds type parameter, conditional Safety tab, SBMM-specific logic |
| app/javascript/src/fetchers/ChemicalFetcher.js | Updates fetch/update methods to support both parent types |
| app/javascript/src/apps/mydb/elements/details/sequenceBasedMacromoleculeSamples/SequenceBasedMacromoleculeSampleDetails.js | Adds inventory checkbox, tab management, and save handlers for chemical data |
| app/javascript/src/stores/mobx/SequenceBasedMacromoleculeSamplesStore.jsx | Adds chemical edit state management |
| app/javascript/src/models/SequenceBasedMacromoleculeSample.js | Adds inventory_sample field to model |
| lib/import/import_collections.rb | Adds sort_data helper to sort imports by creation timestamp |
| app/javascript/src/apps/mydb/inbox/UnsortedBox.js | Refactors to use bulk delete for better performance |
| app/javascript/src/stores/alt/actions/InboxActions.js | Fixes array mutation issue in bulk delete |
| app/javascript/src/stores/alt/stores/InboxStore.js | Handles bulk delete with array of IDs |
| app/usecases/reactions/update_materials.rb | Comments out equivalent field assignments (likely unrelated change) |
| app/javascript/src/models/Sample.js | Sets default amount type to 'target' for reference samples |
| app/javascript/src/apps/mydb/elements/details/VersionsTable.js | Adds message for unsaved elements in history tab |
| app/javascript/src/apps/mydb/elements/details/VersionsTableChanges.js | Fixes revert button state management |
| app/javascript/src/apps/mydb/elements/details/VersionsTableFields.js | Adds checkbox change event handler |
| app/javascript/src/apps/mydb/elements/details/FieldValueSelector.js | Adds null/undefined safety for display value |
| app/javascript/src/apps/commandAndControl/CnC.js | Improves error handling for device connections |
| app/api/chemotion/attachment_api.rb | Fixes error response format in bulk delete |
| config/initializers/novnc_devices.rb | Creates directory for device connection status tracking |
| app/javascript/src/components/contextActions/import/ModalImport.js | Updates template labels from "Chemical" to "Chemical Sample" |
| app/javascript/src/apps/mydb/elements/details/reactions/schemeTab/MaterialGroup.js | Adds custom styles to reagent select dropdown |
| app/javascript/src/apps/mydb/elements/details/sequenceBasedMacromoleculeSamples/propertiesTab/PropertiesForm.js | Simplifies label text |
Comments suppressed due to low confidence (1)
app/usecases/reactions/update_materials.rb:281
- The equivalent field is commented out on both update and create operations. If this is intentional and part of this PR's changes, it should be documented why this field is being removed. If this is unintentional or temporary, the commented code should either be removed or restored. This change could potentially break functionality related to reaction equivalents.
existing_sample.short_label = sample.short_label if sample.short_label
existing_sample.short_label = fixed_label if fixed_label
existing_sample.name = sample.name if sample.name
existing_sample.dry_solvent = sample.dry_solvent
# Handle components for mixture samples using the proper use case
if sample.sample_type == Sample::SAMPLE_TYPE_MIXTURE && sample.components.present?
Usecases::Components::Create.new(existing_sample, sample.components).execute!
end
existing_sample.solvent = sample.solvent
if r = existing_sample.residues[0]
r.assign_attributes sample.residues_attributes[0]
end
existing_sample.container = update_datamodel(sample.container) if sample.container
existing_sample.skip_reaction_svg_update = true
existing_sample.save!
existing_sample
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
db/migrate/20251017_add_sequence_based_macromolecule_sample_id_to_chemicals.rb
Outdated
Show resolved
Hide resolved
| type="SBMM" | ||
| setSaveInventory={(v) => sbmmStore.setSaveInventoryAction(v)} | ||
| saveInventory={sbmmStore.saveInventoryAction} | ||
| editChemical={sbmmStore.editChemical} |
There was a problem hiding this comment.
ChemicalTab requires a handleUpdateSample prop (marked as required in PropTypes on line 1870), but this prop is not passed when ChemicalTab is used for SBMM samples on line 81-88. This will cause a PropTypes warning and potentially break functionality if handleUpdateSample is called. Either make this prop optional for SBMM usage or provide a handler function.
| editChemical={sbmmStore.editChemical} | |
| editChemical={sbmmStore.editChemical} | |
| handleUpdateSample={() => {}} |
| # frozen_string_literal: true | ||
|
|
||
| # == Schema Information | ||
| # |
There was a problem hiding this comment.
The PR title contains a typo: "asssociations" should be "associations" (three 's' instead of two).
db/migrate/20251017_add_sequence_based_macromolecule_sample_id_to_chemicals.rb
Outdated
Show resolved
Hide resolved
...lements/details/sequenceBasedMacromoleculeSamples/SequenceBasedMacromoleculeSampleDetails.js
Show resolved
Hide resolved
...lements/details/sequenceBasedMacromoleculeSamples/SequenceBasedMacromoleculeSampleDetails.js
Show resolved
Hide resolved
cbf1f7f to
d52e447
Compare
6b84aa6 to
b38bf30
Compare
b38bf30 to
8071c6f
Compare
2131492 to
b2cf0fc
Compare
…(SBMM)
- Add SBMM foreign key to chemicals table and update models for polymorphic-like association
- Enforce mutual exclusivity and presence validation for sample_id and sequence_based_macromolecule_id
- Update API, fetchers, and UI components to handle both sample- and SBMM-linked chemicals
- Extend state management and tests for new association logic
- Enable inventory tab functionality for sequence-based macromolecule samples
8071c6f to
3cfe807
Compare
rather 1-story 1-commit than sub-atomic commits
commit title is meaningful => git history search
commit description is helpful => helps the reviewer to understand the changes
code is up-to-date with the latest developments of the target branch (rebased to it or whatever) => ⏩-merge for linear history is favoured
added code is linted
tests are passing (at least locally): we still have some random test failure on CI. thinking of asking spec/examples.txt to be commited
in case the changes are visible to the end-user, video or screenshots should be added to the PR => helps with user testing
testing coverage improvement is improved.
CHANGELOG : add a bullet point on top (optional: reference to github issue/PR )
parallele PR for documentation on docusaurus if the feature/fix is tagged for a release