feat: element vessel api and UI #2235
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -221,6 +224,9 @@ class CollectionAPI < Grape::API | |||
| cell_lines = CelllineSample.by_collection_id(@cid) | |||
| .by_ui_state(params[:elements_filter][:cell_line]) | |||
| .for_user_n_groups(user_ids) | |||
| vessels = Vessel.by_collection_id(@cid) | |||
| .by_ui_state(params[:elements_filter][:vessel]) | |||
There was a problem hiding this comment.
Layout/MultilineMethodCallIndentation: Align .by_ui_state with .by_collection_id on line 227.
| @@ -221,6 +224,9 @@ class CollectionAPI < Grape::API | |||
| cell_lines = CelllineSample.by_collection_id(@cid) | |||
| .by_ui_state(params[:elements_filter][:cell_line]) | |||
| .for_user_n_groups(user_ids) | |||
| vessels = Vessel.by_collection_id(@cid) | |||
| .by_ui_state(params[:elements_filter][:vessel]) | |||
| .for_user_n_groups(user_ids) | |||
There was a problem hiding this comment.
Layout/MultilineMethodCallIndentation: Align .for_user_n_groups with .by_collection_id on line 227.
| @@ -134,6 +134,7 @@ class ProfileAPI < Grape::API | |||
| 'screen' => 4, | |||
| 'research_plan' => 5, | |||
| 'cell_line' => -1000, | |||
| 'vessel' => -1100, | |||
There was a problem hiding this comment.
Metrics/ClassLength: Class has too many lines. [213/200]
| # frozen_string_literal: true | ||
|
|
||
| module Chemotion | ||
| class VesselAPI < Grape::API |
There was a problem hiding this comment.
Metrics/ClassLength: Class has too many lines. [221/200]
| Entities::VesselInstanceEntity.represent( | ||
| vessel, | ||
| displayed_in_list: true, | ||
| # detail_levels: ElementDetailLevelCalculator.new(user: current_user, element: cell_line_sample).detail_levels, |
There was a problem hiding this comment.
Layout/LineLength: Line is too long. [124/120]
| @@ -0,0 +1,12 @@ | |||
|
|
|||
There was a problem hiding this comment.
Style/FrozenStringLiteralComment: Missing frozen string literal comment.
| @@ -0,0 +1,12 @@ | |||
|
|
|||
| module Entities | |||
There was a problem hiding this comment.
Layout/LeadingEmptyLines: Unnecessary blank line at the beginning of the source.
| @@ -30,6 +30,12 @@ class ProfileAPI < Grape::API | |||
| data[ll.to_s] = layout[ll] if layout[ll].present? && data[ll.to_s].nil? | |||
| end | |||
|
|
|||
| if (element_list = layout.dig(:layout)) | |||
There was a problem hiding this comment.
Metrics/ClassLength: Class has too many lines. [218/200]
| @@ -30,6 +30,12 @@ class ProfileAPI < Grape::API | |||
| data[ll.to_s] = layout[ll] if layout[ll].present? && data[ll.to_s].nil? | |||
| end | |||
|
|
|||
| if (element_list = layout.dig(:layout)) | |||
There was a problem hiding this comment.
Style/SingleArgumentDig: Use layout[:layout] instead of layout.dig(:layout).
| element_list.each do |element, sorting| | ||
| data['layout'][element.to_s] = sorting if data['layout'][element.to_s].nil? | ||
| end | ||
| end |
There was a problem hiding this comment.
Layout/EndAlignment: end at 37, 7 is not aligned with if at 33, 8.
ashwiniHerle
changed the title
ashwiniHerle
force-pushed
the
feat-add-new-element-vessel-api
branch
from
9514a1d to
e68fc13
Compare
ashwiniHerle
force-pushed
the
feat-add-new-element-vessel-api
branch
from
e68fc13 to
b8de641
Compare
| # frozen_string_literal: true | ||
|
|
||
| module Chemotion | ||
| class VesselAPI < Grape::API |
There was a problem hiding this comment.
Metrics/ClassLength: Class has too many lines. [238/200]
| requires :id, type: String, desc: 'id of vessel instance to load' | ||
| end | ||
| get ':id' do | ||
| if params[:id] == 'new' |
There was a problem hiding this comment.
Style/IfUnlessModifier: Favor modifier if usage when having a single-line body. Another good alternative is the usage of control flow &&/||.
| get ':id' do | ||
| if params[:id] == 'new' | ||
| return present Vessel.new, with: Entities::VesselInstanceEntity | ||
| end |
There was a problem hiding this comment.
Layout/EmptyLineAfterGuardClause: Add empty line after guard clause.
| ActiveRecord::Base.transaction do | ||
| vessel = Vessel.find_by(id: params[:vessel_id]) | ||
| error!('Vessel not found', 404) unless vessel | ||
|
|
There was a problem hiding this comment.
Layout/TrailingWhitespace: Trailing whitespace detected.
| short_label: params[:short_label], | ||
| }.compact | ||
| vessel.update!(vessel_params) if vessel_params.present? | ||
|
|
There was a problem hiding this comment.
Layout/TrailingWhitespace: Trailing whitespace detected.
| present vessel, with: Entities::VesselInstanceEntity | ||
| end | ||
| end | ||
|
|
There was a problem hiding this comment.
Layout/TrailingWhitespace: Trailing whitespace detected.
| end | ||
| end | ||
|
|
||
|
|
There was a problem hiding this comment.
Layout/TrailingWhitespace: Trailing whitespace detected.
| resource :names do | ||
| desc 'Returns all accessible vessel templates material names and their id' | ||
| get 'all' do | ||
| vessel_templates = VesselTemplate.select(:id, :name, :vessel_type, :material_type, :volume_amount, :volume_unit).distinct |
There was a problem hiding this comment.
Layout/LineLength: Line is too long. [131/120]
| @@ -39,6 +39,9 @@ def update_tag | |||
| when 'CollectionsCellline' | |||
| args = { collection_tag: true } | |||
| element = 'cellline_sample' | |||
| when 'CollectionsVessel' | |||
There was a problem hiding this comment.
Metrics/MethodLength: Method has too many lines. [31/30]
|
|
||
| belongs_to :vessel_template | ||
| belongs_to :creator, class_name: 'User', foreign_key: :user_id, inverse_of: :created_vessels | ||
|
|
||
| has_one :container, as: :containable |
There was a problem hiding this comment.
Rails/HasManyOrHasOneDependent: Specify a :dependent option.
| # frozen_string_literal: true | ||
|
|
||
| module Chemotion | ||
| class VesselAPI < Grape::API |
There was a problem hiding this comment.
Metrics/ClassLength: Class has too many lines. [248/200]
| volume_amount: params[:volume_amount], | ||
| volume_unit: params[:volume_unit], | ||
| weight_amount: params[:weight_amount], | ||
| weight_unit: params[:weight_unit] |
There was a problem hiding this comment.
Style/TrailingCommaInArguments: Put a comma after the last parameter of a multiline method call.
| weight_amount: params[:weight_amount], | ||
| weight_unit: params[:weight_unit] | ||
| ) | ||
|
|
There was a problem hiding this comment.
Layout/TrailingWhitespace: Trailing whitespace detected.
| ) | ||
|
|
||
| created_vessels = [] | ||
|
|
There was a problem hiding this comment.
Layout/TrailingWhitespace: Trailing whitespace detected.
| description: instance_params[:description], | ||
| bar_code: instance_params[:bar_code], | ||
| qr_code: instance_params[:qr_code], | ||
| short_label: params[:short_label] |
There was a problem hiding this comment.
Style/TrailingCommaInArguments: Put a comma after the last parameter of a multiline method call.
| qr_code: instance_params[:qr_code], | ||
| short_label: params[:short_label] | ||
| ) | ||
|
|
There was a problem hiding this comment.
Layout/TrailingWhitespace: Trailing whitespace detected.
| collection = current_user.collections.find_by(id: params[:collection_id]) | ||
| vessel.collections << collection if collection.present? | ||
| end | ||
|
|
There was a problem hiding this comment.
Layout/TrailingWhitespace: Trailing whitespace detected.
| error!("Container update failed: #{e.message}", 400) | ||
| end | ||
| end | ||
|
|
There was a problem hiding this comment.
Layout/TrailingWhitespace: Trailing whitespace detected.
| created_vessels << vessel | ||
| end | ||
| end | ||
|
|
There was a problem hiding this comment.
Layout/TrailingWhitespace: Trailing whitespace detected.
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