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.

Sign up for GitHub

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

Jump to bottom

1272 update UI for CV and CDS layouts on chemspectra #1273

Merged
merged 1 commit into from
May 9, 2023
Merged

1272 update UI for CV and CDS layouts on chemspectra #1273

merged 1 commit into from
May 9, 2023

Conversation

baolanlequang
Copy link
Member

@baolanlequang baolanlequang commented Apr 14, 2023 •
edited

  • 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

@baolanlequang baolanlequang linked an issue Apr 14, 2023 that may be closed by this pull request
Update UI for CV layout and add CDS layout on Chemspectra #1272
Closed
github-actions[bot]
github-actions bot reviewed Apr 14, 2023
View reviewed changes
app/models/concerns/attachment_jcamp_aasm.rb Show resolved Hide resolved
app/models/concerns/attachment_jcamp_aasm.rb Show resolved Hide resolved
app/models/concerns/attachment_jcamp_aasm.rb Show resolved Hide resolved
app/models/concerns/attachment_jcamp_aasm.rb Show resolved Hide resolved
app/models/concerns/attachment_jcamp_aasm.rb Show resolved Hide resolved
spec/models/attachment_spec.rb Show resolved Hide resolved
spec/models/attachment_spec.rb Outdated Show resolved Hide resolved
spec/models/attachment_spec.rb Show resolved Hide resolved
spec/models/attachment_spec.rb Show resolved Hide resolved
spec/models/attachment_spec.rb Outdated Show resolved Hide resolved
@github-actions
Copy link

LCOV of commit b28a8dc during Continuous Integration #833

Summary coverage rate:
  lines......: 59.9% (11756 of 19633 lines)
  functions..: no data found
  branches...: no data found

Files changed coverage rate: n/a

@baolanlequang baolanlequang force-pushed the 1272-update-ui-for-cv-layout-on-chemspectra branch from b28a8dc to 944a04e Compare April 17, 2023 15:34
@github-actions
Copy link

LCOV of commit 944a04e during Continuous Integration #843

Summary coverage rate:
  lines......: 59.9% (11758 of 19633 lines)
  functions..: no data found
  branches...: no data found

Files changed coverage rate: n/a

@github-actions
Copy link

LCOV of commit c51b66e during Continuous Integration #845

Summary coverage rate:
  lines......: 59.9% (11757 of 19634 lines)
  functions..: no data found
  branches...: no data found

Files changed coverage rate: n/a

@baolanlequang baolanlequang force-pushed the 1272-update-ui-for-cv-layout-on-chemspectra branch from c51b66e to d81768f Compare April 19, 2023 13:12
@github-actions
Copy link

LCOV of commit d81768f during Continuous Integration #853

Summary coverage rate:
  lines......: 59.9% (11757 of 19633 lines)
  functions..: no data found
  branches...: no data found

Files changed coverage rate: n/a

@baolanlequang baolanlequang force-pushed the 1272-update-ui-for-cv-layout-on-chemspectra branch from d81768f to 3187829 Compare April 20, 2023 09:16
@github-actions
Copy link

LCOV of commit 3187829 during Continuous Integration #860

Summary coverage rate:
  lines......: 59.9% (11758 of 19632 lines)
  functions..: no data found
  branches...: no data found

Files changed coverage rate: n/a

@baolanlequang baolanlequang force-pushed the 1272-update-ui-for-cv-layout-on-chemspectra branch from 3187829 to ab5c3e3 Compare April 24, 2023 09:05
@github-actions
Copy link

LCOV of commit ab5c3e3 during Continuous Integration #884

Summary coverage rate:
  lines......: 60.1% (11790 of 19623 lines)
  functions..: no data found
  branches...: no data found

Files changed coverage rate: n/a

@github-actions
Copy link

LCOV of commit 6b59ce1 during Continuous Integration #885

Summary coverage rate:
  lines......: 60.1% (11788 of 19623 lines)
  functions..: no data found
  branches...: no data found

Files changed coverage rate: n/a

@github-actions
Copy link

LCOV of commit d89df5f during Continuous Integration #886

Summary coverage rate:
  lines......: 60.1% (11788 of 19623 lines)
  functions..: no data found
  branches...: no data found

Files changed coverage rate: n/a

@baolanlequang baolanlequang force-pushed the 1272-update-ui-for-cv-layout-on-chemspectra branch from d89df5f to ecf48ba Compare April 24, 2023 10:19
@github-actions
Copy link

LCOV of commit ecf48ba during Continuous Integration #887

Summary coverage rate:
  lines......: 60.1% (11788 of 19623 lines)
  functions..: no data found
  branches...: no data found

Files changed coverage rate: n/a

@baolanlequang baolanlequang force-pushed the 1272-update-ui-for-cv-layout-on-chemspectra branch from ecf48ba to 7780d09 Compare April 24, 2023 10:44
@baolanlequang baolanlequang marked this pull request as ready for review April 24, 2023 10:54
@baolanlequang
Copy link
Member Author

  • Demo video for CV layout
Screencast.2023-04-18.10.38.21.mp4
  • Demo video for CDS layout
Screencast.2023-04-18.10.40.01.mp4

@github-actions
Copy link

LCOV of commit 7780d09 during Continuous Integration #888

Summary coverage rate:
  lines......: 60.1% (11788 of 19623 lines)
  functions..: no data found
  branches...: no data found

Files changed coverage rate: n/a

@PiTrem PiTrem changed the title 1272 update UI for cv layout on chemspectra 1272 update UI for CV and CDS layout on chemspectra Apr 25, 2023
@PiTrem PiTrem changed the title 1272 update UI for CV and CDS layout on chemspectra 1272 update UI for CV and CDS layouts on chemspectra Apr 25, 2023
@baolanlequang baolanlequang force-pushed the 1272-update-ui-for-cv-layout-on-chemspectra branch from 7780d09 to b06472e Compare April 25, 2023 11:55
@github-actions
Copy link

LCOV of commit b06472e during Continuous Integration #899

Summary coverage rate:
  lines......: 60.1% (11789 of 19622 lines)
  functions..: no data found
  branches...: no data found

Files changed coverage rate: n/a

PiTrem
PiTrem reviewed May 9, 2023
View reviewed changes
app/models/concerns/attachment_jcamp_aasm.rb
if attachable&.root_element.is_a?(Sample)
params[:mass] = attachable&.root_element&.molecule&.exact_molecular_weight || 0.0
params[:sample_id] = attachable&.root_element.id

attachable.ancestors.each do |ancestor|
Copy link
Member

@PiTrem PiTrem May 9, 2023 •
edited

Choose a reason for hiding this comment

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

find ?

 params[:analysis_id] = attachable.ancestors.find {|container| container.container_type == 'analysis' }&.id

or what if there is several analyses? (not sure how the attachable is ordered..

@PiTrem PiTrem merged commit 2b2bd33 into main May 9, 2023
2 checks passed
@PiTrem PiTrem added this to the v1.6 milestone May 9, 2023
dpatelev pushed a commit that referenced this pull request May 10, 2023
@baolanlequang @dpatelev
save metadata into csv for CV layout and add CDS layout (#1273)
2eae05b 
Co-authored-by: Lan Le <lan.le@kit.edu>
mekkyz pushed a commit that referenced this pull request Sep 21, 2023
@baolanlequang @mekkyz
save metadata into csv for CV layout and add CDS layout (#1273)
d682fb9 
Co-authored-by: Lan Le <lan.le@kit.edu>
@baolanlequang baolanlequang mentioned this pull request Oct 24, 2023
Closed
baolanlequang added a commit that referenced this pull request Mar 5, 2024
@baolanlequang
save metadata into csv for CV layout and add CDS layout (#1273)
2d612d6 
Co-authored-by: Lan Le <lan.le@kit.edu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PiTrem PiTrem PiTrem left review comments

@github-actions github-actions[bot] github-actions left review comments

@TasnimMehzabin TasnimMehzabin TasnimMehzabin approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v1.6
Development

Successfully merging this pull request may close these issues.

Update UI for CV layout and add CDS layout on Chemspectra
3 participants
@baolanlequang @TasnimMehzabin @PiTrem