-
Notifications
You must be signed in to change notification settings - Fork 54
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
780 enhencing versioning feature #800
780 enhencing versioning feature #800
Conversation
7da662c
to
fedc3a1
Compare
4bdcf48
to
5e65cf0
Compare
@Allenskywalker92 I had a notification/mention about this PR that I can no longer find. Has the problem been resolved in the meantime? |
@mschneider85 I don't know either. Couldn't find any comment. |
Revert function in the histroy tab of reactions does not work. Also name field in history is empty if the sample has no name. I think maybe it would be better to have common name of any other type of sample identification, so that user can have an idea which sample is this? - or what do you think? revert_function_doesNotWork.mp4 |
5e65cf0
to
7543dea
Compare
* removed .match from integer parameter * add fixture for spectrum params * fixed parameter json * add test for edit a jcamp file - WIP * Switch state to done after creating all sub attachments and deleting all files Co-authored-by: FabianMauz <[email protected]>
* - upgrade html2pdf.js to version 0.10.1 - add process as dev dependency to solve webpack compilation error * move process/browser modules to webpack custom file Co-authored-by: Mehreen <mehreen.mansur>
70c5770
to
7543dea
Compare
Bumps [decode-uri-component](https://github.com/SamVerschueren/decode-uri-component) from 0.2.0 to 0.2.2. - [Release notes](https://github.com/SamVerschueren/decode-uri-component/releases) - [Commits](SamVerschueren/decode-uri-component@v0.2.0...v0.2.2) --- updated-dependencies: - dependency-name: decode-uri-component dependency-type: indirect ... Signed-off-by: dependabot[bot] <[email protected]> Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Current link to docs (https://www.chemotion.net/chemotionsaurus/docs/category/manual) is broken, returns 500.
* obey rubocop rules and removed unused methods * removed no more used methods: store_tmp_file_and_thumbnail, store_file_and_thumbnail_for_dup, move_from_store * Moved attachment logic to after save callback * add tests for attachment model * removed duplicate test * refactor attachment_api * moved attach actions to after save hook * Moved attachment logic to after save callback - report template api * removed validation before save * nothing was validated * Extract creation of report_template to usecase * add size validation of attached file * only attach file if path to file exists * Moved attachment logic to after save callback - generic helpers * tests must be updated * add validation of file size * refactor tests for generic_helpers * more refactoring needed, but first i have to understand the method signatures * Moved attachment logic to after save callback - jcamp_aasm * update to latest dev5 * Moved attachment logic to after save callback - upload_chunk_complete * Moved attachment logic to after save callback - report_templates.seed.db * update to latest dev5 * Move attachment logic to after save callback - sample usecase * update to latest dev5 * Moved attachment logic to after save callback - SampleTask::Update * Moved attachment logic to after save callback - coverter.rb * add file_path to attachment * Fixed bugs and added test * Moved attachment logic to after save callback - datacollector_file.rb * Moved attachment logic to after save callback - datacollector_file.rb - sftp * Moved attachment logic to after save callback - datacollector_folder.rb * removed manual file attaching in test * repaired factory for reports * renamed test folder as in src package * Moved attachment logic to after save callback - datacollector/mailcollector.rb * Moved attachment logic to after save callback - import/import_collections.rb * Moved attachment logic to after save callback - Reporter/worker.rb * add test for creating attachment in worker * Update to latest main * removed useless test * set fixed key to test attachment * fixed bug in test * fixed bug in test (2) * fixed location of files in test * reintroduce test for checking attaching files * update to current main * linted attachment_api.rb * linted generic_helpers.rb * linted attachment.rb * linted upload_chunk_complete.rb * linted import_collections.rb * linted sample_api_spec.rb * linted generic_helpers_spec.rb * add linting for factories: attachments, reports * linted import_wellplate_spreadsheet_spec.rb * removed customized local development folder * add a linebreak to see why rubocop is annoyed * add a linebreak to see why rubocop is annoyed * update to latest main * removed override of key attribute, which leads to further errors * if the key from the frontend is overritten, further updating of container hierarchy will fail * remove duplicate line Co-authored-by: Jan C. Brammer <[email protected]> Co-authored-by: Fabian <fabian@fabian-Virtual-Machine> Co-authored-by: FabianMauz <[email protected]>
* Add SampleTask Inbox (unfinished) * Extract FreeScanCard to allow for individual Sample Dropzones per Task * Add a drop receptacle to open free scans * Update admin app to use newer react-dnd syntax * Fix component reuse bug * Add SampleTask creation dropzone * Fix crashing drag and drop functionality in table sorting popovers Newer versions of react-dnd use React-Contexts to access their DragAndDropManager instance. This instance is wrapped around the toplevel App object in packs/src/apps/{mydb,admin}/index.js Unfortunately, the ancient react-bootstrap package appends Overlays created by an OverlayTrigger OUTSIDE the react tree, so any component that relies on the react tree to supply contexts can no longer access them when inside an overlay. To rectify this, all instances where a drag and drop enabled component is used within an overlay have been rewritten not to use OverlayTrigger. The implemented solution was inspired by the example at https://react-bootstrap-v3.netlify.app/components/overlays/#custom-overlays-overlay * Fix missing Hide box for element list settings * Fix DND integration in Admin App * Move all SampleTaskInbox related files into the same folder
Bumps [loofah](https://github.com/flavorjones/loofah) from 2.19.0 to 2.19.1. - [Release notes](https://github.com/flavorjones/loofah/releases) - [Changelog](https://github.com/flavorjones/loofah/blob/main/CHANGELOG.md) - [Commits](flavorjones/loofah@v2.19.0...v2.19.1) --- updated-dependencies: - dependency-name: loofah dependency-type: indirect ... Signed-off-by: dependabot[bot] <[email protected]> Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [rails-html-sanitizer](https://github.com/rails/rails-html-sanitizer) from 1.4.3 to 1.4.4. - [Release notes](https://github.com/rails/rails-html-sanitizer/releases) - [Changelog](https://github.com/rails/rails-html-sanitizer/blob/master/CHANGELOG.md) - [Commits](rails/rails-html-sanitizer@v1.4.3...v1.4.4) --- updated-dependencies: - dependency-name: rails-html-sanitizer dependency-type: indirect ... Signed-off-by: dependabot[bot] <[email protected]> Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [nokogiri](https://github.com/sparklemotion/nokogiri) from 1.13.9 to 1.13.10. - [Release notes](https://github.com/sparklemotion/nokogiri/releases) - [Changelog](https://github.com/sparklemotion/nokogiri/blob/main/CHANGELOG.md) - [Commits](sparklemotion/nokogiri@v1.13.9...v1.13.10) --- updated-dependencies: - dependency-name: nokogiri dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]> Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [httparty](https://github.com/jnunemaker/httparty) from 0.20.0 to 0.21.0. - [Release notes](https://github.com/jnunemaker/httparty/releases) - [Changelog](https://github.com/jnunemaker/httparty/blob/master/Changelog.md) - [Commits](jnunemaker/httparty@v0.20.0...v0.21.0) --- updated-dependencies: - dependency-name: httparty dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]> Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [json5](https://github.com/json5/json5) from 1.0.1 to 1.0.2. - [Release notes](https://github.com/json5/json5/releases) - [Changelog](https://github.com/json5/json5/blob/main/CHANGELOG.md) - [Commits](json5/json5@v1.0.1...v1.0.2) --- updated-dependencies: - dependency-name: json5 dependency-type: indirect ... Signed-off-by: dependabot[bot] <[email protected]> Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* WIP add react flow display * Add first version of editor * Add component_graph_data field to screen model * Update Screen API to allow saving component graph data * Fix typo preventing proper code highlighting * Add component graph data to JS Screen model * Enable saving/updating of component graph data to server and fix static preview display * Make API work properly with specs and update specs to test saving process * Rubocop * Allow relabeling edges in ResearchplanFlowEditor * Assign default width to flow panel * Fix display update issues * Allow dragging and zooming in the component graph preview * Add number of analyses and attachments to EmbeddedResearchPlan within Screen Co-authored-by: Johannes Haubold <[email protected]>
* Improve Dev setup for M1 macs M1 macs use an ARM-based processor architecture, which will be picked up by docker generating ARM linux images by default. So we specifically use the linux/amd64 image to create a non-ARM image to work with. This way all compilation and package installations (google-chrome-stable does not have ARM packages for example) run the expected way. Another issue is that docker for mac uses qemu as backend on M1 macs, which does not support inotify. Because of that, the EventedFileWatcher used in the development environment does not work and had to be replaced by a FileWatcher without usage of inotify * Add database setup to run-ruby-dev.sh
* fix: fixed reaction to response from the literatures api * style: apply eslint / rubocop rules * refactor: use UrlSearchParams instead of string concat * refactor: extract request params * feat: add ui response after removing literal * style: line length and correct isNaN usage * style: use deconstruction * test: add test for deleting literature * refactor: refactored and styled tests Refs: #1502
* chore: update node LTS to 18 * ci image with upg node version * test(js): force mocha to exit * chore(js): upg whatwg fetch 2->3 * chore(js): upg test dependencies: jsdom, mocha, sinon * devcontainer --------- Co-authored-by: Jan C. Brammer <[email protected]> Refs: #1489
* build(deps): bump citation-js to 0.6.8 * build: remove library patch in post_install script * feat: patch citation-js library to accept also SICIs Refs: #1486
Co-authored-by: Lan Le <[email protected]> Refs: #1507
fix: cannot read processed data from Bruker Co-authored-by: Lan Le <[email protected]> Refs: #1528
* Add (de)select all for datasets in device box * select all attachments in a dataset * separate unsortedbox and devicebox checkedIds * correct deleteCheckedDataset function * reset checkedDeviceIds when changing pages * move checkedDeviceIds to InboxStore * handle dataset attachments selection in inboxstore * update handlePrev / NextClick fix: pagination fix: deleteCheckedDataset to delete attachments Refs: #1437
… disable error message for decoupled samples Refs: #1531
7543dea
to
a0816e1
Compare
template = { uuid: uuid, layers: {}, select_options: {} } | ||
attributes = declared(params, include_missing: false) | ||
attributes[:properties_template]['uuid'] = uuid if attributes[:properties_template].present? | ||
attributes[:properties_template] = template unless attributes[:properties_template].present? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rails/Blank: Use if attributes[:properties_template].blank?
instead of unless attributes[:properties_template].present?
.
attributes = declared(params, include_missing: false) | ||
attributes[:properties_template]['uuid'] = uuid if attributes[:properties_template].present? | ||
attributes[:properties_template] = template unless attributes[:properties_template].present? | ||
attributes[:properties_template]['eln'] = Chemotion::Application.config.version if attributes[:properties_template].present? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/IfUnlessModifier: Modifier form of if
makes the line too long.
attributes = declared(params, include_missing: false) | ||
attributes[:properties_template]['uuid'] = uuid if attributes[:properties_template].present? | ||
attributes[:properties_template] = template unless attributes[:properties_template].present? | ||
attributes[:properties_template]['eln'] = Chemotion::Application.config.version if attributes[:properties_template].present? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Layout/LineLength: Line is too long. [134/120]
klasses = ElementKlass.where(is_active: true)&.pluck(:name) || [] | ||
bytes_written = File.write(klass_names_file, klasses) | ||
if bytes_written == klasses.length | ||
puts "File successfully written" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rails/Output: Do not write to stdout. Use Rails's logger if you want to log.
klasses = ElementKlass.where(is_active: true)&.pluck(:name) || [] | ||
bytes_written = File.write(klass_names_file, klasses) | ||
if bytes_written == klasses.length | ||
puts "File successfully written" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.
params do | ||
requires :sample_id, type: Integer, desc: 'sample id' | ||
requires :cas, type: String | ||
requires :chemical_data, type: Array[Hash] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/RedundantArrayConstructor: Remove the redundant Array
constructor.
@@ -19,6 +21,42 @@ class CollectionAPI < Grape::API | |||
get do | |||
present Collection.find(params[:id]), with: Entities::CollectionEntity, root: :collection | |||
end | |||
|
|||
desc "Return collection metadata" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.
error!('401 Unauthorized', 401) | ||
end | ||
before do | ||
error!('401 Unauthorized', 401) unless CollectionPolicy.new(current_user, Collection.find(params[:id])).read_metadata? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/IfUnlessModifier: Modifier form of unless
makes the line too long.
error!('401 Unauthorized', 401) | ||
end | ||
before do | ||
error!('401 Unauthorized', 401) unless CollectionPolicy.new(current_user, Collection.find(params[:id])).read_metadata? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Layout/LineLength: Line is too long. [128/120]
end | ||
get :metadata do | ||
metadata = Metadata.where(collection_id: params[:id]).first | ||
if metadata |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/RedundantCondition: Use double pipes ||
instead.
end | ||
end | ||
|
||
desc "Create/update collection metadata" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.
requires :metadata, type: JSON | ||
end | ||
before do | ||
error!('401 Unauthorized', 401) unless CollectionPolicy.new(current_user, Collection.find(params[:id])).update_metadata? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/IfUnlessModifier: Modifier form of unless
makes the line too long.
requires :metadata, type: JSON | ||
end | ||
before do | ||
error!('401 Unauthorized', 401) unless CollectionPolicy.new(current_user, Collection.find(params[:id])).update_metadata? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Layout/LineLength: Line is too long. [130/120]
end | ||
post :metadata do | ||
metadata = Metadata.where(collection_id: params[:id]).first | ||
unless metadata |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/IfUnlessModifier: Favor modifier unless
usage when having a single-line body. Another good alternative is the usage of control flow &&
/||
.
end | ||
post :metadata do | ||
metadata = Metadata.where(collection_id: params[:id]).first | ||
unless metadata |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/OrAssignment: Use the double pipe equals operator ||=
instead.
@@ -331,24 +340,23 @@ def search_elements(c_id = @c_id, dl = @dl) | |||
end | |||
|
|||
if search_method == 'advanced' && molecule_sort == false | |||
arg_value_str = adv_params.first['value'].split(/(\r)?\n|,/).map(&:strip) | |||
arg_value_str = adv_params.first['value'].split(/(\r)?\n/).map(&:strip) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/InverseMethods: Use reject
instead of inverting select
.
@@ -470,6 +478,8 @@ def elements_by_scope(scope, collection_id = @c_id) | |||
case search_by_method | |||
when 'structure' | |||
sample_structure_search | |||
when 'cas' | |||
Sample.by_collection_id(@c_id).by_sample_xref_cas( params[:selection][:name]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Layout/SpaceInsideParens: Space inside parentheses detected.
@@ -63,9 +63,11 @@ def search_possibilities_by_type_user_and_collection(type) | |||
sample_name = dl_s.positive? && search_by_field.call(Sample, :name, qry) || [] | |||
polymer_type = dl_s.positive? && d_for.call(Sample) | |||
.by_residues_custom_info('polymer_type', qry) | |||
.pluck("residues.custom_info -> 'polymer_type'").uniq || [] | |||
.pluck(Arel.sql("residues.custom_info->'polymer_type'")).uniq || [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Metrics/AbcSize: Assignment Branch Condition size for search_possibilities_by_type_user_and_collection is too high. [<66, 148, 116> 199.3/25]
@@ -63,9 +63,11 @@ def search_possibilities_by_type_user_and_collection(type) | |||
sample_name = dl_s.positive? && search_by_field.call(Sample, :name, qry) || [] | |||
polymer_type = dl_s.positive? && d_for.call(Sample) | |||
.by_residues_custom_info('polymer_type', qry) | |||
.pluck("residues.custom_info -> 'polymer_type'").uniq || [] | |||
.pluck(Arel.sql("residues.custom_info->'polymer_type'")).uniq || [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Metrics/CyclomaticComplexity: Cyclomatic complexity for search_possibilities_by_type_user_and_collection is too high. [98/7]
@@ -63,9 +63,11 @@ def search_possibilities_by_type_user_and_collection(type) | |||
sample_name = dl_s.positive? && search_by_field.call(Sample, :name, qry) || [] | |||
polymer_type = dl_s.positive? && d_for.call(Sample) | |||
.by_residues_custom_info('polymer_type', qry) | |||
.pluck("residues.custom_info -> 'polymer_type'").uniq || [] | |||
.pluck(Arel.sql("residues.custom_info->'polymer_type'")).uniq || [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Metrics/MethodLength: Method has too many lines. [135/30]
@@ -63,9 +63,11 @@ def search_possibilities_by_type_user_and_collection(type) | |||
sample_name = dl_s.positive? && search_by_field.call(Sample, :name, qry) || [] | |||
polymer_type = dl_s.positive? && d_for.call(Sample) | |||
.by_residues_custom_info('polymer_type', qry) | |||
.pluck("residues.custom_info -> 'polymer_type'").uniq || [] | |||
.pluck(Arel.sql("residues.custom_info->'polymer_type'")).uniq || [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Metrics/PerceivedComplexity: Perceived complexity for search_possibilities_by_type_user_and_collection is too high. [97/8]
@@ -63,9 +63,11 @@ def search_possibilities_by_type_user_and_collection(type) | |||
sample_name = dl_s.positive? && search_by_field.call(Sample, :name, qry) || [] | |||
polymer_type = dl_s.positive? && d_for.call(Sample) | |||
.by_residues_custom_info('polymer_type', qry) | |||
.pluck("residues.custom_info -> 'polymer_type'").uniq || [] | |||
.pluck(Arel.sql("residues.custom_info->'polymer_type'")).uniq || [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lint/AmbiguousOperatorPrecedence: Wrap expressions with varying precedence with parentheses to avoid ambiguity.
sum_formula = dl_s.positive? && search_by_field.call(Sample, :molecule_sum_formular, qry) || [] | ||
iupac_name = dl_s.positive? && search_by_field.call(Molecule, :iupac_name, qry) || [] | ||
# cas = dl_s.positive? && search_by_field.call(Molecule, :cas, qry) || [] | ||
cas = dl_s.positive? && search_by_field.call(Sample, :sample_xref_cas, qry) || [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lint/AmbiguousOperatorPrecedence: Wrap expressions with varying precedence with parentheses to avoid ambiguity.
@@ -128,9 +131,11 @@ def search_possibilities_by_type_user_and_collection(type) | |||
sample_external_label = dl_s > -1 && search_by_field.call(Sample, :external_label, qry) || [] | |||
polymer_type = dl_s.positive? && d_for.call(Sample) | |||
.by_residues_custom_info('polymer_type', qry) | |||
.pluck("residues.custom_info -> 'polymer_type'").uniq || [] | |||
.pluck(Arel.sql("residues.custom_info->'polymer_type'")).uniq || [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lint/AmbiguousOperatorPrecedence: Wrap expressions with varying precedence with parentheses to avoid ambiguity.
sum_formula = dl_s.positive? && search_by_field.call(Sample, :molecule_sum_formular, qry) || [] | ||
iupac_name = dl_s.positive? && search_by_field.call(Molecule, :iupac_name, qry) || [] | ||
# cas = dl_s.positive? && search_by_field.call(Molecule, :cas, qry) || [] | ||
cas = dl_s.positive? && search_by_field.call(Sample, :sample_xref_cas, qry) || [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lint/AmbiguousOperatorPrecedence: Wrap expressions with varying precedence with parentheses to avoid ambiguity.
@@ -79,7 +79,7 @@ def create_or_update_attachments(container, attachments) | |||
return unless can_update | |||
attachments.each do |att| | |||
if att[:is_new] | |||
attachment = Attachment.where(storage: 'tmp', key: att[:id]).last | |||
attachment = Attachment.where(key: att[:id], attachable: nil).last |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Metrics/PerceivedComplexity: Perceived complexity for create_or_update_attachments is too high. [11/8]
|
||
ensure |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Layout/EmptyLinesAroundExceptionHandlingKeywords: Extra empty line detected before the ensure
.
@@ -460,6 +460,7 @@ def build_sql_reaction_sample(columns, c_id, ids, checkedAll = false) | |||
sample: { | |||
external_label: ['s.external_label', '"sample external label"', 0], | |||
name: ['s."name"', '"sample name"', 0], | |||
cas: ['s.xref', nil, 0], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Metrics/ModuleLength: Module has too many lines. [545/100]
@@ -570,6 +571,8 @@ def build_column_query(sel, user_id = 0, attrs = EXP_MAP_ATTR) | |||
selection << "labels_by_user_sample(#{user_id}, s_id) as user_labels" | |||
elsif col == 'literature' | |||
selection << "literatures_by_element('Sample', s_id) as literatures" | |||
elsif col == 'cas' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Metrics/CyclomaticComplexity: Cyclomatic complexity for build_column_query is too high. [9/7]
@@ -570,6 +571,8 @@ def build_column_query(sel, user_id = 0, attrs = EXP_MAP_ATTR) | |||
selection << "labels_by_user_sample(#{user_id}, s_id) as user_labels" | |||
elsif col == 'literature' | |||
selection << "literatures_by_element('Sample', s_id) as literatures" | |||
elsif col == 'cas' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Metrics/PerceivedComplexity: Perceived complexity for build_column_query is too high. [10/8]
Logidze.clear_responsible! if @logidze_meta_set.present? | ||
end | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Layout/TrailingEmptyLines: Final newline missing.
@@ -11,7 +11,7 @@ class ApplicationController < ActionController::Base | |||
|
|||
def configure_permitted_parameters | |||
devise_parameter_sanitizer.permit(:sign_up, keys: [ | |||
:email, :first_name, :last_name, :name_abbreviation, :omniauth_provider, :omniauth_uid, | |||
:email, :first_name, :last_name, :name_abbreviation, :provider, :uid, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Layout/FirstArrayElementIndentation: Use 2 spaces for indentation in an array, relative to the first position after the preceding left parenthesis.
@@ -0,0 +1,111 @@ | |||
class Oauth::RadarController < ApplicationController |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/FrozenStringLiteralComment: Missing frozen string literal comment.
@@ -0,0 +1,111 @@ | |||
class Oauth::RadarController < ApplicationController |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/ClassAndModuleChildren: Use nested module/class definitions instead of compact style.
collection_id = request.GET['collection_id'] | ||
unless collection_id | ||
@error = 'No collection id was provided.' | ||
return render status: 400 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rails/HttpStatus: Prefer :bad_request
over 400
to define HTTP status code.
@workspaces = response['data'].map { |workspace| { | ||
'id' => workspace['id'], | ||
'label' => workspace['descriptiveMetadata']['title'] | ||
}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Layout/BlockEndNewline: Expression at 80, 8 should be on its own line.
|
||
# check if any workspaces were found | ||
if @workspaces.empty? | ||
@error = 'No workspaces could be retrieved from RADAR. Please make sure that at least one workspace is available.' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Layout/LineLength: Line is too long. [122/120]
# check if any workspaces were found | ||
if @workspaces.empty? | ||
@error = 'No workspaces could be retrieved from RADAR. Please make sure that at least one workspace is available.' | ||
return render status: 400 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rails/HttpStatus: Prefer :bad_request
over 400
to define HTTP status code.
collection = Collection.belongs_to_or_shared_by(current_user.id, current_user.group_ids).find_by(id: collection_id) | ||
unless collection | ||
@error = 'You are not allowed to access this collection.' | ||
return render status: 403 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rails/HttpStatus: Prefer :forbidden
over 403
to define HTTP status code.
return render status: 403 | ||
end | ||
|
||
unless collection.metadata.metadata['datasetUrl'].nil? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/GuardClause: Use a guard clause (return redirect_to collection.metadata.metadata['datasetUrl'] unless collection.metadata.metadata['datasetUrl'].nil?
) instead of wrapping the code inside a conditional expression.
This commit refactors `ReactionDetails` and `SampleDetails` to make them pass eslint checks. We had unused functions there, which will be removed by this commit: - SampleDetails#initiateAnalysisButton - SampleDetails#initiateAnalysisWithKind - SampleDetails#transferToDeviceButton Also disables `forbid-prop-types` in .eslintrc for now. This can be taken out later, when we can better specify the propTypes across the entire project.
Store a history which keeps track of every change made to certain entities and add a "History" tab to the Reaction/Sample detail pages. Related entities are displayed in the version history. E.g. Reaction -> ReactionsSample Save timestamp and author of the version with the changes. A change diff to the previous version can be displayed. Tracked entities: - Attachment - Container - ElementalComposition - Reaction - ReactionsSample - Residue - Sample Co-authored-by: VadimKeller <[email protected]>
This commit refactors `ReactionDetails` and `SampleDetails` to make them pass eslint checks. We had unused functions there, which will be removed by this commit: - SampleDetails#initiateAnalysisButton - SampleDetails#initiateAnalysisWithKind - SampleDetails#transferToDeviceButton Also disables `forbid-prop-types` in .eslintrc for now. This can be taken out later, when we can better specify the propTypes across the entire project.
Store a history which keeps track of every change made to certain entities and add a "History" tab to the Reaction/Sample detail pages. Related entities are displayed in the version history. E.g. Reaction -> ReactionsSample Save timestamp and author of the version with the changes. A change diff to the previous version can be displayed. Tracked entities: - Attachment - Container - ElementalComposition - Reaction - ReactionsSample - Residue - Sample Co-authored-by: VadimKeller <[email protected]>
Add revert button to versioned field reset colum information re-create schema update solvent label
- Update version list after reverting a change - Fix issue /w big svgs - Make reverting molfile / svg work - Fix merge issues - Replace passing local variables /w using state
0cec773
to
15ccde9
Compare
return render status: 403 | ||
end | ||
|
||
unless collection.metadata.metadata['datasetUrl'].nil? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/UnlessElse: Do not use unless
with else
. Rewrite these with the positive case first.
end | ||
|
||
unless collection.metadata.metadata['datasetUrl'].nil? | ||
return redirect_to collection.metadata.metadata['datasetUrl'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/RedundantReturn: Redundant return
detected.
unless collection.metadata.metadata['datasetUrl'].nil? | ||
return redirect_to collection.metadata.metadata['datasetUrl'] | ||
else | ||
return render |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/RedundantReturn: Redundant return
detected.
|
||
def first_name | ||
if auth&.provider == PROVIDER_GITHUB | ||
auth&.info&.name.split[0..-2].join(' ') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lint/SafeNavigationChain: Do not chain ordinary method call after safe navigation operator.
@@ -0,0 +1,29 @@ | |||
class ExportCollectionToRadarJob < ActiveJob::Base |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/FrozenStringLiteralComment: Missing frozen string literal comment.
@@ -206,18 +241,22 @@ def update_prediction(params, spc_type, is_regen) | |||
def create_process(is_regen) | |||
params = build_params | |||
|
|||
tmp_jcamp, tmp_img, arr_jcamp, arr_img, arr_csv, spc_type = generate_spectrum_data(params, is_regen) | |||
if params[:ext] == 'nmrium' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/IfUnlessModifier: Favor modifier if
usage when having a single-line body. Another good alternative is the usage of control flow &&
/||
.
return generate_spectrum_from_nmrium | ||
end | ||
|
||
tmp_jcamp, tmp_img, arr_jcamp, arr_img, arr_csv, arr_nmrium, spc_type, invalid_molfile = generate_spectrum_data(params, is_regen) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lint/UselessAssignment: Useless assignment to variable - arr_nmrium
. Use _
or _arr_nmrium
as a variable name to indicate that it won't be used.
return generate_spectrum_from_nmrium | ||
end | ||
|
||
tmp_jcamp, tmp_img, arr_jcamp, arr_img, arr_csv, arr_nmrium, spc_type, invalid_molfile = generate_spectrum_data(params, is_regen) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Layout/LineLength: Line is too long. [133/120]
jcamp_att | ||
end | ||
end | ||
|
||
def edit_process(is_regen, orig_params) | ||
params = build_params(orig_params) | ||
tmp_jcamp, tmp_img, arr_jcamp, arr_img, arr_csv, spc_type = generate_spectrum_data(params, is_regen) | ||
tmp_jcamp, tmp_img, _, _, arr_csv, arr_nmrium, spc_type, invalid_molfile = generate_spectrum_data(params, is_regen) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Metrics/AbcSize: Assignment Branch Condition size for edit_process is too high. [<14, 22, 6> 26.76/25]
tmp_files_to_be_deleted.push(*arr_csv) | ||
delete_related_csv(csv_att) | ||
end | ||
|
||
# set_backup | ||
unless arr_nmrium.nil? || arr_nmrium.length == 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/NumericPredicate: Use arr_nmrium.length.zero?
instead of arr_nmrium.length == 0
.
tmp_files_to_be_deleted.push(*arr_csv) | ||
delete_related_csv(csv_att) | ||
end | ||
|
||
# set_backup | ||
unless arr_nmrium.nil? || arr_nmrium.length == 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/ZeroLengthPredicate: Use empty?
instead of length == 0
.
jcamp_att | ||
end | ||
|
||
def check_invalid_molfile(tmp_img, spc_type, tmp_jcamp) | ||
if tmp_img.nil? && spc_type.nil? && tmp_jcamp['invalid_molfile'] == true | ||
def check_invalid_molfile(invalid_molfile = false) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/OptionalBooleanParameter: Prefer keyword arguments for arguments with a boolean default value; use invalid_molfile: false
instead of invalid_molfile = false
.
def check_invalid_molfile(tmp_img, spc_type, tmp_jcamp) | ||
if tmp_img.nil? && spc_type.nil? && tmp_jcamp['invalid_molfile'] == true | ||
def check_invalid_molfile(invalid_molfile = false) | ||
if invalid_molfile == true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/GuardClause: Use a guard clause (return unless invalid_molfile == true
) instead of wrapping the code inside a conditional expression.
@@ -268,7 +316,11 @@ def check_invalid_molfile(tmp_img, spc_type, tmp_jcamp) | |||
end | |||
|
|||
def generate_spectrum_data(params, is_regen) | |||
tmp_jcamp, tmp_img, arr_jcamp, arr_img, arr_csv, spc_type = Tempfile.create('molfile') do |t_molfile| | |||
if params[:ext] == 'nmrium' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/IfUnlessModifier: Favor modifier if
usage when having a single-line body. Another good alternative is the usage of control flow &&
/||
.
return | ||
end | ||
|
||
tmp_jcamp, tmp_img, arr_jcamp, arr_img, arr_csv, arr_nmrium, spc_type, invalid_molfile = Tempfile.create('molfile') do |t_molfile| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lint/UselessAssignment: Useless assignment to variable - tmp_jcamp
. Use _
or _tmp_jcamp
as a variable name to indicate that it won't be used.
|
||
result = [] # result data | ||
base = {} # track current version data | ||
log_data.versions.each do |version| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Metrics/BlockLength: Block has too many lines. [42/25]
log_data.versions.each do |version| | ||
changes = version.changes # get changes for current version | ||
changes_comparison_hash = {} # hash for changes comparison | ||
changes.each do |key, value| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Metrics/BlockLength: Block has too many lines. [29/25]
o: old_value[key], | ||
n: new_value[key], | ||
l: label[key], | ||
k: kind[key] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/TrailingCommaInHashLiteral: Put a comma after the last item of a multiline hash.
o: old_value, | ||
n: new_value, | ||
l: version_label(key), # label for attribute, | ||
k: version_kind(key) # kind of attribute (numrange, date, string) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/TrailingCommaInHashLiteral: Put a comma after the last item of a multiline hash.
|
||
result << { | ||
'k' => version_entity, # record kind (sampe, reaction, ...) | ||
'n' => record_name, # record name (uses as default the name attribute but in case the model doesn't have a name field or you want to change it) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Layout/LineLength: Line is too long. [151/120]
moved to #1253 |
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