fix: cellline_materials add index and created_by column #2353

PiTrem · PiTrem · commit 7d4ac6028b9a · 2025-03-12T14:14:52.000+01:00
Squashed commit of the following: commit 44b54e8 Author: Adrian Herrmann <adrian.herrmann@kit.edu> Date: Tue Mar 11 08:13:08 2025 +0000 review findings commit a427fc3 Author: Adrian Herrmann <adrian.herrmann@kit.edu> Date: Fri Feb 28 09:28:10 2025 +0000 refactor: rubocop & typo commit 5d32e53 Author: Adrian Herrmann <adrian.herrmann@kit.edu> Date: Wed Feb 26 14:46:10 2025 +0000 feat: added column created_by and set access commit faf855e Author: Adrian Herrmann <adrian.herrmann@kit.edu> Date: Wed Feb 26 10:04:43 2025 +0000 fix: tests because of index commit 03b432c Author: Adrian Herrmann <adrian.herrmann@kit.edu> Date: Fri Feb 21 11:37:14 2025 +0000 added index on (name, source) in cellline_materials
diff --git a/app/javascript/src/apps/mydb/elements/details/cellLines/propertiesTab/CellLineName.js b/app/javascript/src/apps/mydb/elements/details/cellLines/propertiesTab/CellLineName.js
@@ -18,7 +18,7 @@ export default class CellLineName extends React.Component {
 
   componentDidMount() {
     CellLinesFetcher.getAllCellLineNames()
-      .then((data) => data.map((x) => ({ value: x.id, label: x.name, name: x.name })))
+      .then((data) => data.map((x) => ({ value: x.id, label: `${x.name} - ${x.source}`, name: x.name })))
       .then((data) => {
         this.setState({ nameSuggestions: data });
       });
diff --git a/app/javascript/src/apps/mydb/elements/details/cellLines/propertiesTab/GeneralProperties.js b/app/javascript/src/apps/mydb/elements/details/cellLines/propertiesTab/GeneralProperties.js
@@ -9,11 +9,28 @@ import PropTypes from 'prop-types';
 import CellLineName from 'src/apps/mydb/elements/details/cellLines/propertiesTab/CellLineName';
 import Amount from 'src/apps/mydb/elements/details/cellLines/propertiesTab/Amount';
 import InvalidPropertyWarning from 'src/apps/mydb/elements/details/cellLines/propertiesTab/InvalidPropertyWarning';
+import UserStore from 'src/stores/alt/stores/UserStore';
 
 class GeneralProperties extends React.Component {
   // eslint-disable-next-line react/static-property-placement
   static contextType = StoreContext;
 
+  checkPermission(attributeName) {
+    const readonlyAttributes = [
+      'Disease', 'Organism', 'Tissue', 'Growth medium', 'Mutation', 'Variant', 'Biosafety level',
+      'Cryopreservation medium', 'Opt. growth temperature', 'Gender', 'Cell type', 'Material Description'
+    ];
+    const { item } = this.props;
+    const { currentUser } = UserStore.getState();
+    if (item.created_by == null) {
+      const { cellLineDetailsStore } = this.context;
+      const cellLine = cellLineDetailsStore.cellLines(item.id);
+      return readonlyAttributes.includes(attributeName)
+        && cellLine.created_by !== '' && cellLine.created_by !== currentUser.id.toString();
+    }
+    return readonlyAttributes.includes(attributeName) && item.created_by !== currentUser.id;
+  }
+
   renderOptionalAttribute(attributeName, defaultValue, onChangeCallBack) {
     return this.renderAttribute(attributeName, defaultValue, onChangeCallBack, true);
   }
@@ -43,7 +60,7 @@ class GeneralProperties extends React.Component {
         <Form.Label column sm={3}>{attributeName}</Form.Label>
         <Col sm={9}>
           <Form.Control
-            disabled={readOnly}
+            disabled={readOnly || this.checkPermission(attributeName)}
             className={styleClass}
             type="text"
             value={defaultValue}
@@ -69,10 +86,10 @@ class GeneralProperties extends React.Component {
         <Form.Label column sm={3}>Biosafety level</Form.Label>
         <Col sm={9}>
           <Select
-            isDisabled={readOnly}
+            isDisabled={readOnly || this.checkPermission('Biosafety level')}
             options={options}
             isClearable={false}
-            value={options.find(({value}) => value === item.bioSafetyLevel)}
+            value={options.find(({ value }) => value === item.bioSafetyLevel)}
             onChange={(e) => { cellLineDetailsStore.changeBioSafetyLevel(item.id, e.value); }}
           />
         </Col>
@@ -117,7 +134,7 @@ class GeneralProperties extends React.Component {
           <Amount
             cellLineId={item.id}
             initialValue={item.amount}
-            readOnly={readOnly}
+            readOnly={readOnly || this.checkPermission('Amount')}
           />
         </Col>
         <Col sm={3} className="amount-unit">
@@ -187,7 +204,7 @@ class GeneralProperties extends React.Component {
             {this.renderOptionalAttribute('Cell type', cellLineItem.cellType, (e) => {
               cellLineDetailsStore.changeCellType(cellLineId, e.target.value);
             })}
-            {this.renderOptionalAttribute('Description', cellLineItem.materialDescription, (e) => {
+            {this.renderOptionalAttribute('Material Description', cellLineItem.materialDescription, (e) => {
               cellLineDetailsStore.changeMaterialDescription(cellLineId, e.target.value);
             })}
           </Accordion.Body>
@@ -209,7 +226,7 @@ class GeneralProperties extends React.Component {
             {this.renderOptionalAttribute('Name of specific probe', cellLineItem.itemName, (e) => {
               cellLineDetailsStore.changeItemName(cellLineId, e.target.value);
             })}
-            {this.renderOptionalAttribute('Description', cellLineItem.itemDescription, (e) => {
+            {this.renderOptionalAttribute('Sample Description', cellLineItem.itemDescription, (e) => {
               cellLineDetailsStore.changeItemDescription(cellLineId, e.target.value);
             })}
           </Accordion.Body>
@@ -224,6 +241,8 @@ export default observer(GeneralProperties);
 GeneralProperties.propTypes = {
   readOnly: PropTypes.bool.isRequired,
   item: PropTypes.shape({
-    id: PropTypes.string.isRequired
+    id: PropTypes.string.isRequired,
+    can_update: PropTypes.bool,
+    created_by: PropTypes.number
   }).isRequired
 };
diff --git a/app/javascript/src/models/cellLine/CellLine.js b/app/javascript/src/models/cellLine/CellLine.js
@@ -45,6 +45,7 @@ export default class CellLine extends Element {
     cellLine.cellType = response.cellline_material.cell_type;
     cellLine.bioSafetyLevel = response.cellline_material.biosafety_level;
     cellLine.cryopreservationMedium = response.cellline_material.cryo_pres_medium;
+    cellLine.created_by = response.cellline_material.created_by;
     cellLine.is_new = false;
 
     cellLine.container = response.container;
@@ -67,6 +68,7 @@ export default class CellLine extends Element {
     this.gender = cellLineItem.gender;
     this.materialDescription = cellLineItem.materialDescription;
     this.source = cellLineItem.source;
+    this.created_by = cellLineItem.created_by;
   }
 
   adoptPropsFromMobXModel(mobx) {
@@ -91,5 +93,6 @@ export default class CellLine extends Element {
     this.bioSafetyLevel = mobx.bioSafetyLevel;
     this.cellType = mobx.cellType;
     this.cryopreservationMedium = mobx.cryopreservationMedium;
+    this.created_by = mobx.created_by;
   }
 }
diff --git a/app/javascript/src/stores/mobx/CellLineDetailsStore.jsx b/app/javascript/src/stores/mobx/CellLineDetailsStore.jsx
@@ -44,6 +44,7 @@ const CellLineItem = types
     growthMedium: '',
     itemDescription: '',
     itemName: '',
+    created_by: '',
     changed: false
   }).views((self) => ({
     isAmountValid() {
@@ -165,6 +166,7 @@ export const CellLineDetailsStore = types
       item.mutation = properties.mutation;
       item.source = properties.source;
       item.growthMedium = properties.growth_medium;
+      item.created_by = properties.created_by?.toString();
     },
     convertJsModelToMobxModel(container) {
       return CellLineAnalysis.create({
@@ -203,6 +205,7 @@ export const CellLineDetailsStore = types
         growthMedium: jsCellLineModel.growthMedium,
         itemName: jsCellLineModel.itemName,
         shortLabel: jsCellLineModel.short_label,
+        created_by: jsCellLineModel.created_by?.toString(),
       }));
     }
   }))
diff --git a/app/models/cellline_material.rb b/app/models/cellline_material.rb
@@ -22,6 +22,7 @@
 #  deleted_at          :datetime
 #  created_at          :datetime         not null
 #  updated_at          :datetime         not null
+#  created_by          :integer
 #
 class CelllineMaterial < ApplicationRecord
   acts_as_paranoid
diff --git a/app/usecases/cell_lines/create.rb b/app/usecases/cell_lines/create.rb
@@ -45,6 +45,7 @@ def create_cellline_material
           cryo_pres_medium: @params[:cryo_pres_medium],
           gender: @params[:gender],
           description: @params[:material_description],
+          created_by: @current_user.id,
         )
       end
 
diff --git a/app/usecases/cell_lines/update.rb b/app/usecases/cell_lines/update.rb
@@ -16,7 +16,10 @@ def execute!
 
         @cell_line_sample.cellline_material = find_material || create_new_material
         update_sample_properties
-        update_material_properties(@cell_line_sample.cellline_material)
+        if @cell_line_sample.cellline_material.created_by.nil? ||
+           @cell_line_sample.cellline_material.created_by == @current_user.id
+          update_material_properties(@cell_line_sample.cellline_material)
+        end
         @cell_line_sample.save
         @cell_line_sample
       end
diff --git a/db/migrate/20250226095051_add_column_to_cellline_material.rb b/db/migrate/20250226095051_add_column_to_cellline_material.rb
@@ -0,0 +1,8 @@
+# frozen_string_literal: true
+
+class AddColumnToCelllineMaterial < ActiveRecord::Migration[6.1]
+  def change
+    add_index :cellline_materials, %i[name source], unique: true
+    add_column :cellline_materials, :created_by, :integer, null: true
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
@@ -132,6 +132,8 @@
     t.datetime "deleted_at"
     t.datetime "created_at", precision: 6, null: false
     t.datetime "updated_at", precision: 6, null: false
+    t.integer "created_by"
+    t.index ["name", "source"], name: "index_cellline_materials_on_name_and_source", unique: true
   end
 
   create_table "cellline_samples", force: :cascade do |t|
diff --git a/spec/api/chemotion/cell_line_api_spec.rb b/spec/api/chemotion/cell_line_api_spec.rb
@@ -10,10 +10,12 @@
   describe 'GET /api/v1/cell_lines/' do
     let(:collection) { create(:collection) }
     let!(:user) { create(:user, collections: [collection]) }
-    let!(:cell_line) { create(:cellline_sample, collections: [collection]) }
+    let(:material) { create(:cellline_material) }
+    let!(:cell_line) { create(:cellline_sample, collections: [collection], cellline_material: material) }
     let!(:cell_line2) do
       create(:cellline_sample,
              collections: [collection],
+             cellline_material: material,
              created_at: DateTime.parse('2000-01-01'),
              updated_at: DateTime.parse('2010-01-01'))
     end
diff --git a/spec/api/chemotion/suggestion_api_spec.rb b/spec/api/chemotion/suggestion_api_spec.rb
@@ -5,10 +5,10 @@
 
 describe Chemotion::SuggestionAPI do
   let!(:user) { create(:person, first_name: 'tam', last_name: 'M') }
+  let(:material) { create(:cellline_material) }
   let(:collection) { create(:collection, user: user, is_shared: true, permission_level: 1, sample_detail_level: 10) }
   let(:query) { 'query' }
-  let(:json_repsonse) { JSON.parse(response.body) }
-  let(:json_response) { response.body }
+  let(:json_response) { JSON.parse(response.body) }
   let(:params) do
     {
       collection_id: collection.id,
@@ -19,9 +19,11 @@
 
   describe 'GET /api/v1/cell_lines/suggestions/cell_lines' do
     include_context 'api request authorization context'
-    let!(:cell_line) { create(:cellline_sample, collections: [collection]) }
-    let!(:cell_line2) { create(:cellline_sample, name: 'search-example', collections: [collection]) }
-    let!(:cell_line_without_col) { create(:cellline_sample, name: 'search-example') }
+    let!(:cell_line) { create(:cellline_sample, collections: [collection], cellline_material: material) }
+    let!(:cell_line2) do
+      create(:cellline_sample, name: 'search-example', collections: [collection], cellline_material: material)
+    end
+    let!(:cell_line_without_col) { create(:cellline_sample, name: 'search-example', cellline_material: material) }
     let!(:sample) { create(:sample, name: 'search-example', collections: [collection]) }
 
     before do
@@ -36,9 +38,9 @@
       end
 
       it 'suggestions should be returned' do
-        expect(json_repsonse['suggestions'].length).to be 1
-        expect(json_repsonse['suggestions'].first['name']).to eq 'name-001'
-        expect(json_repsonse['suggestions'].first['search_by_method']).to eq 'cell_line_material_name'
+        expect(json_response['suggestions'].length).to be 1
+        expect(json_response['suggestions'].first['name']).to eq 'name-001'
+        expect(json_response['suggestions'].first['search_by_method']).to eq 'cell_line_material_name'
       end
     end
 
@@ -50,17 +52,19 @@
       end
 
       it 'suggestions should be returned' do
-        expect(json_repsonse['suggestions'].length).to be 1
-        expect(json_repsonse['suggestions'].first['name']).to eq 'search-example'
-        expect(json_repsonse['suggestions'].first['search_by_method']).to eq 'cell_line_sample_name'
+        expect(json_response['suggestions'].length).to be 1
+        expect(json_response['suggestions'].first['name']).to eq 'search-example'
+        expect(json_response['suggestions'].first['search_by_method']).to eq 'cell_line_sample_name'
       end
     end
   end
 
   describe 'GET /api/v1/cell_lines/suggestions/all' do
     include_context 'api request authorization context'
-    let!(:cell_line) { create(:cellline_sample, collections: [collection]) }
-    let!(:cell_line2) { create(:cellline_sample, name: 'search-example', collections: [collection]) }
+    let!(:cell_line) { create(:cellline_sample, collections: [collection], cellline_material: material) }
+    let!(:cell_line2) do
+      create(:cellline_sample, name: 'search-example', collections: [collection], cellline_material: material)
+    end
     let!(:sample) { create(:sample, name: 'search-example', collections: [collection]) }
 
     before do
@@ -75,9 +79,9 @@
       end
 
       it 'suggestions should be returned' do
-        expect(json_repsonse['suggestions'].length).to be 1
-        expect(json_repsonse['suggestions'].first['name']).to eq 'name-001'
-        expect(json_repsonse['suggestions'].first['search_by_method']).to eq 'cell_line_material_name'
+        expect(json_response['suggestions'].length).to be 1
+        expect(json_response['suggestions'].first['name']).to eq 'name-001'
+        expect(json_response['suggestions'].first['search_by_method']).to eq 'cell_line_material_name'
       end
     end
 
@@ -89,17 +93,17 @@
       end
 
       it 'two suggestions were found' do
-        expect(json_repsonse['suggestions'].length).to be 2
+        expect(json_response['suggestions'].length).to be 2
       end
 
       it 'first suggestion from sample' do
-        expect(json_repsonse['suggestions'].first['name']).to eq 'search-example'
-        expect(json_repsonse['suggestions'].first['search_by_method']).to eq 'sample_name'
+        expect(json_response['suggestions'].first['name']).to eq 'search-example'
+        expect(json_response['suggestions'].first['search_by_method']).to eq 'sample_name'
       end
 
       it 'second suggestion from cell line' do
-        expect(json_repsonse['suggestions'].second['name']).to eq 'search-example'
-        expect(json_repsonse['suggestions'].second['search_by_method']).to eq 'cell_line_sample_name'
+        expect(json_response['suggestions'].second['name']).to eq 'search-example'
+        expect(json_response['suggestions'].second['search_by_method']).to eq 'cell_line_sample_name'
       end
     end
   end
@@ -114,7 +118,6 @@
             is_sync: false,
           }
       expect(response).to have_http_status(:success)
-      json_response = JSON.parse(response.body)
       expect(json_response.keys).to contain_exactly('suggestions')
       suggestions = json_response['suggestions']
       expect(suggestions).to be_an(Array)
diff --git a/spec/api/collection_api_spec.rb b/spec/api/collection_api_spec.rb
@@ -48,6 +48,7 @@
       create(:sync_collections_user, collection_id: c_sync_w.id, user_id: user.id, permission_level: 1,
                                      shared_by_id: owner.id, fake_ancestry: c_sync_ancestry.id.to_s)
     end
+    let(:material) { create(:cellline_material) }
 
     before do
       allow_any_instance_of(WardenAuthentication).to receive(:current_user).and_return(user)
@@ -184,7 +185,7 @@
             JSON.parse(response.body)['collections'].map do |root|
               root['children'].pluck('id')
             end.flatten,
-          ).to match_array [c4.id, c6.id]
+          ).to contain_exactly(c4.id, c6.id)
         end
       end
     end
@@ -303,9 +304,9 @@
         #   c_target.reload2
         # end
         describe 'PUT /api/v1/collections/elements' do
-          let!(:cell_line_1) { create(:cellline_sample, collections: [c_source]) }
-          let!(:cell_line_2) { create(:cellline_sample, collections: [c_source]) }
-          let!(:cell_line_3) { create(:cellline_sample, collections: [c_source]) }
+          let!(:cell_line_1) { create(:cellline_sample, collections: [c_source], cellline_material: material) }
+          let!(:cell_line_2) { create(:cellline_sample, collections: [c_source], cellline_material: material) }
+          let!(:cell_line_3) { create(:cellline_sample, collections: [c_source], cellline_material: material) }
           let(:cell_line_ids) { [] }
           let!(:ui_state) do
             {
@@ -354,7 +355,9 @@
           end
 
           context 'when try to move cell line element into collection where it already exists' do
-            let!(:cellline_in_two_colls) { create(:cellline_sample, collections: [c_source, c_target]) }
+            let!(:cellline_in_two_colls) do
+              create(:cellline_sample, collections: [c_source, c_target], cellline_material: material)
+            end
             let(:target_collection_id) { c_target.id }
             let(:cell_line_ids) { [cellline_in_two_colls.id] }
 
@@ -392,7 +395,7 @@
         end
 
         describe 'POST /api/v1/collections/elements' do
-          let!(:cell_line_sample) { create(:cellline_sample, collections: [c_source]) }
+          let!(:cell_line_sample) { create(:cellline_sample, collections: [c_source], cellline_material: material) }
           let!(:ui_state) do
             {
               cell_line: {
@@ -686,10 +689,10 @@
             c = Collection.where(is_shared: true, user_id: u2.id, shared_by_id: user.id)
                           .where("label LIKE 'My project with%'").first
             expect(c).not_to be_nil
-            expect(c.samples).to match_array [s1, s3]
-            expect(c.reactions).to match_array [r1]
-            expect(c.wellplates).to match_array [w1]
-            expect(c.screens).to match_array []
+            expect(c.samples).to contain_exactly(s1, s3)
+            expect(c.reactions).to contain_exactly(r1)
+            expect(c.wellplates).to contain_exactly(w1)
+            expect(c.screens).to be_empty
           end
         end
 
diff --git a/spec/api/element_api_spec.rb b/spec/api/element_api_spec.rb
diff --git a/spec/usecases/cell_lines/load_spec.rb b/spec/usecases/cell_lines/load_spec.rb

Original file line number	Diff line number	Diff line change
`@@ -45,6 +45,7 @@ export default class CellLine extends Element {`
`45`	`45`	`cellLine.cellType = response.cellline_material.cell_type;`
`46`	`46`	`cellLine.bioSafetyLevel = response.cellline_material.biosafety_level;`
`47`	`47`	`cellLine.cryopreservationMedium = response.cellline_material.cryo_pres_medium;`
	`48`	`+ cellLine.created_by = response.cellline_material.created_by;`
`48`	`49`	`cellLine.is_new = false;`
`49`	`50`
`50`	`51`	`cellLine.container = response.container;`
`@@ -67,6 +68,7 @@ export default class CellLine extends Element {`
`67`	`68`	`this.gender = cellLineItem.gender;`
`68`	`69`	`this.materialDescription = cellLineItem.materialDescription;`
`69`	`70`	`this.source = cellLineItem.source;`
	`71`	`+ this.created_by = cellLineItem.created_by;`
`70`	`72`	`}`
`71`	`73`
`72`	`74`	`adoptPropsFromMobXModel(mobx) {`
`@@ -91,5 +93,6 @@ export default class CellLine extends Element {`
`91`	`93`	`this.bioSafetyLevel = mobx.bioSafetyLevel;`
`92`	`94`	`this.cellType = mobx.cellType;`
`93`	`95`	`this.cryopreservationMedium = mobx.cryopreservationMedium;`
	`96`	`+ this.created_by = mobx.created_by;`
`94`	`97`	`}`
`95`	`98`	`}`
Original file line number	Diff line number	Diff line change
`@@ -22,6 +22,7 @@`
`22`	`22`	`# deleted_at :datetime`
`23`	`23`	`# created_at :datetime not null`
`24`	`24`	`# updated_at :datetime not null`
	`25`	`+# created_by :integer`
`25`	`26`	`#`
`26`	`27`	`class CelllineMaterial < ApplicationRecord`
`27`	`28`	`acts_as_paranoid`
Original file line number	Diff line number	Diff line change
`@@ -45,6 +45,7 @@ def create_cellline_material`
`45`	`45`	`cryo_pres_medium: @params[:cryo_pres_medium],`
`46`	`46`	`gender: @params[:gender],`
`47`	`47`	`description: @params[:material_description],`
	`48`	`+ created_by: @current_user.id,`
`48`	`49`	`)`
`49`	`50`	`end`
`50`	`51`