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

fix: cellline_materials add index and created_by column #2353

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
1 change: 1 addition & 0 deletions app/api/helpers/cell_line_api_params_helpers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ module CellLineApiParamsHelpers
optional :mutation, type: String, desc: 'mutation of a cell line'
optional :description, type: String, desc: 'description of a cell line sample'
optional :short_label, type: String, desc: 'short label of a cell line sample'
optional :created_by, type: Integer, desc: 'creator of cell line material'
requires :container, type: Hash, desc: 'root Container of element'
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,29 @@ 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;

// eslint-disable-next-line class-methods-use-this
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) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Couldn't this guard be removed if the cell line (item) is instantiated with (created_by = '') in CellLine.buildEmpty()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No. Tested it and it doesn't work when creating celllines.

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);
}
Expand Down Expand Up @@ -43,7 +61,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}
Expand All @@ -69,10 +87,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>
Expand Down Expand Up @@ -117,7 +135,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">
Expand Down Expand Up @@ -187,7 +205,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>
Expand All @@ -209,7 +227,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>
Expand All @@ -224,6 +242,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
};
3 changes: 3 additions & 0 deletions app/javascript/src/models/cellLine/CellLine.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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) {
Expand All @@ -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;
}
}
3 changes: 3 additions & 0 deletions app/javascript/src/stores/mobx/CellLineDetailsStore.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ const CellLineItem = types
growthMedium: '',
itemDescription: '',
itemName: '',
created_by: '',
changed: false
}).views((self) => ({
isAmountValid() {
Expand Down Expand Up @@ -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({
Expand Down Expand Up @@ -203,6 +205,7 @@ export const CellLineDetailsStore = types
growthMedium: jsCellLineModel.growthMedium,
itemName: jsCellLineModel.itemName,
shortLabel: jsCellLineModel.short_label,
created_by: jsCellLineModel.created_by?.toString(),
}));
}
}))
Expand Down
1 change: 1 addition & 0 deletions app/models/cellline_material.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions app/usecases/cell_lines/create.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
5 changes: 4 additions & 1 deletion app/usecases/cell_lines/update.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 8 additions & 0 deletions db/migrate/20250226095051_add_column_to_cellline_material.rb
Original file line number Diff line number Diff line change
@@ -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
4 changes: 3 additions & 1 deletion db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema.define(version: 2025_02_18_161800) do
ActiveRecord::Schema.define(version: 2025_02_26_095051) do

# These are extensions that must be enabled in order to support this database
enable_extension "hstore"
Expand Down Expand Up @@ -131,6 +131,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|
Expand Down
4 changes: 3 additions & 1 deletion spec/api/chemotion/cell_line_api_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
47 changes: 25 additions & 22 deletions spec/api/chemotion/suggestion_api_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand All @@ -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

Expand All @@ -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
Expand All @@ -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

Expand All @@ -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
Expand All @@ -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)
Expand Down
23 changes: 13 additions & 10 deletions spec/api/collection_api_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
{
Expand Down Expand Up @@ -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] }

Expand Down Expand Up @@ -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: {
Expand Down Expand Up @@ -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

Expand Down
Loading