Feature/versioning enhanced #1253
Conversation
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
Make revert function work (#780) - 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 Add missing versioning for Screens, Wellplates, and Research plans Change the way versions are displayed in the version table. Previously we had one version per database update. Now we group versions by the request id. These changes are now merged to make it more intuitive for the user. Implement revert functionality /w fetchers, reverters, and serializers Implement a new revert UI with one button per version and one modal to select revertible values. Delete old revert button. Co-authored-by: Martin Schneider <[email protected]> Co-authored-by: VadimKeller <[email protected]>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -0,0 +1,98 @@ | |||
| require 'open-uri' | |||
There was a problem hiding this comment.
Style/FrozenStringLiteralComment: Missing frozen string literal comment.
| Logidze.clear_responsible! if @logidze_meta_set.present? | ||
| end | ||
| end | ||
| end |
There was a problem hiding this comment.
Layout/TrailingEmptyLines: Final newline missing.
| @@ -0,0 +1,2 @@ | |||
| class ContainerHierarchy < ApplicationRecord | |||
There was a problem hiding this comment.
Style/FrozenStringLiteralComment: Missing frozen string literal comment.
| @@ -53,6 +53,7 @@ | |||
| # | |||
|
|
|||
| class Sample < ApplicationRecord | |||
| has_logidze | |||
There was a problem hiding this comment.
Metrics/ClassLength: Class has too many lines. [455/200]
| @@ -0,0 +1,30 @@ | |||
| # frozen_string_literal: true | |||
|
|
|||
| class Versioning::Fetcher | |||
There was a problem hiding this comment.
Style/ClassAndModuleChildren: Use nested module/class definitions instead of compact style.
| new(**args).call | ||
| end | ||
|
|
||
| def call |
There was a problem hiding this comment.
Metrics/AbcSize: Assignment Branch Condition size for call is too high. [<14, 38, 5> 40.8/25]
| versions += Versioning::Serializers::ContainerSerializer.call(analysis, ["Analysis: #{analysis.name}"]) | ||
|
|
||
| analysis.children.with_deleted.with_log_data.each do |dataset| | ||
| versions += Versioning::Serializers::ContainerSerializer.call(dataset, ["Analysis: #{analysis.name}", "Dataset: #{dataset.name}"]) |
There was a problem hiding this comment.
Layout/LineLength: Line is too long. [138/120]
| versions += Versioning::Serializers::ContainerSerializer.call(dataset, ["Analysis: #{analysis.name}", "Dataset: #{dataset.name}"]) | ||
|
|
||
| versions += dataset.attachments.with_log_data.flat_map do |attachment| | ||
| Versioning::Serializers::AttachmentSerializer.call(attachment, ["Analysis: #{analysis.name}", "Dataset: #{dataset.name}", "Attachment: #{attachment.filename}"]) |
There was a problem hiding this comment.
Layout/LineLength: Line is too long. [170/120]
| @@ -0,0 +1,34 @@ | |||
| # frozen_string_literal: true | |||
|
|
|||
| class Versioning::Fetchers::ScreenFetcher | |||
There was a problem hiding this comment.
Style/ClassAndModuleChildren: Use nested module/class definitions instead of compact style.
| new(**args).call | ||
| end | ||
|
|
||
| def call |
There was a problem hiding this comment.
Metrics/AbcSize: Assignment Branch Condition size for call is too high. [<14, 33, 4> 36.07/25]
| versions += Versioning::Serializers::ContainerSerializer.call(analysis, ["Analysis: #{analysis.name}"]) | ||
|
|
||
| analysis.children.with_deleted.with_log_data.each do |dataset| | ||
| versions += Versioning::Serializers::ContainerSerializer.call(dataset, ["Analysis: #{analysis.name}", "Dataset: #{dataset.name}"]) |
There was a problem hiding this comment.
Layout/LineLength: Line is too long. [138/120]
| versions += Versioning::Serializers::ContainerSerializer.call(dataset, ["Analysis: #{analysis.name}", "Dataset: #{dataset.name}"]) | ||
|
|
||
| dataset.attachments.with_log_data.each do |attachment| | ||
| versions += Versioning::Serializers::AttachmentSerializer.call(attachment, ["Analysis: #{analysis.name}", "Dataset: #{dataset.name}", "Attachment: #{attachment.filename}"]) |
There was a problem hiding this comment.
Layout/LineLength: Line is too long. [182/120]
| end | ||
|
|
||
| screen.research_plans.each do |research_plan| | ||
| versions += Versioning::Fetchers::ResearchPlanFetcher.call(research_plan: research_plan, prefix: "Research Plan: #{research_plan.name}") |
There was a problem hiding this comment.
Layout/LineLength: Line is too long. [142/120]
| @@ -0,0 +1,38 @@ | |||
| # frozen_string_literal: true | |||
|
|
|||
| class Versioning::Merger | |||
There was a problem hiding this comment.
Style/ClassAndModuleChildren: Use nested module/class definitions instead of compact style.
|
|
||
| merged_versions = [] | ||
| grouped_versions.each_with_index do |(_, v), index| | ||
| changes = v.map do |v| |
There was a problem hiding this comment.
Lint/ShadowingOuterLocalVariable: Shadowing outer local variable - v.
|
|
||
| attr_accessor :record, :name | ||
|
|
||
| def call |
There was a problem hiding this comment.
Metrics/PerceivedComplexity: Perceived complexity for call is too high. [24/8]
| attr_accessor :record, :name | ||
|
|
||
| def call | ||
| Rails.cache.fetch("versions/#{record.cache_key}", version: record.cache_version) do |
There was a problem hiding this comment.
Metrics/BlockLength: Block has too many lines. [50/25]
|
|
||
| return [] unless log_data | ||
|
|
||
| log_data.versions.group_by { |version| version.data.dig('m', 'uuid') }.each do |uuid, versions| |
There was a problem hiding this comment.
Metrics/BlockLength: Block has too many lines. [44/25]
| result << { | ||
| db_id: record.id, | ||
| klass_name: klass_name, # record class (sampe, reaction, ...) | ||
| name: 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.
Layout/LineLength: Line is too long. [145/120]
| @@ -0,0 +1,90 @@ | |||
| # frozen_string_literal: true | |||
|
|
|||
| class Versioning::Serializers::ContainerSerializer < Versioning::Serializers::BaseSerializer | |||
There was a problem hiding this comment.
Style/ClassAndModuleChildren: Use nested module/class definitions instead of compact style.
|
moved to #2068 |
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