feat: data versioning #2068

PiTrem · 2024-08-12T09:43:02Z

github-actions · 2024-08-12T09:44:28Z

app/api/chemotion/version_api.rb

@@ -0,0 +1,98 @@
+require 'open-uri'


Style/FrozenStringLiteralComment: Missing frozen string literal comment.

app/api/modules/logidze_module.rb

github-actions · 2024-08-12T09:44:28Z

app/models/container_hierarchy.rb

@@ -0,0 +1,2 @@
+class ContainerHierarchy < ApplicationRecord


Style/FrozenStringLiteralComment: Missing frozen string literal comment.

app/services/versioning/fetcher.rb

github-actions · 2024-08-12T09:44:28Z

app/services/versioning/fetchers/reaction_fetcher.rb

@@ -0,0 +1,60 @@
+# frozen_string_literal: true
+
+class Versioning::Fetchers::ReactionFetcher


Style/ClassAndModuleChildren: Use nested module/class definitions instead of compact style.

app/services/versioning/fetchers/sample_fetcher.rb

github-actions · 2024-08-12T09:44:30Z

app/services/versioning/fetchers/screen_fetcher.rb

@@ -0,0 +1,34 @@
+# frozen_string_literal: true
+
+class Versioning::Fetchers::ScreenFetcher


Style/ClassAndModuleChildren: Use nested module/class definitions instead of compact style.

github-actions · 2024-08-12T09:44:30Z

app/services/versioning/fetchers/screen_fetcher.rb

+    new(**args).call
+  end
+
+  def call


Metrics/AbcSize: Assignment Branch Condition size for call is too high. [<14, 33, 4> 36.07/25]

github-actions · 2024-08-12T09:44:30Z

app/services/versioning/fetchers/screen_fetcher.rb

+      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}"])


Layout/LineLength: Line is too long. [138/120]

github-actions · 2024-08-12T09:44:32Z

app/services/versioning/fetchers/screen_fetcher.rb

+        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}"])


Layout/LineLength: Line is too long. [182/120]

github-actions · 2024-08-12T09:44:32Z

app/services/versioning/fetchers/screen_fetcher.rb

+    end
+
+    screen.research_plans.each do |research_plan|
+      versions += Versioning::Fetchers::ResearchPlanFetcher.call(research_plan: research_plan, prefix: "Research Plan: #{research_plan.name}")


Layout/LineLength: Line is too long. [142/120]

github-actions · 2024-08-12T09:44:33Z

app/services/versioning/merger.rb

@@ -0,0 +1,38 @@
+# frozen_string_literal: true
+
+class Versioning::Merger


Style/ClassAndModuleChildren: Use nested module/class definitions instead of compact style.

github-actions · 2024-08-12T09:44:33Z

app/services/versioning/merger.rb

+
+    merged_versions = []
+    grouped_versions.each_with_index do |(_, v), index|
+      changes = v.map do |v|


Lint/ShadowingOuterLocalVariable: Shadowing outer local variable - v.

app/services/versioning/reverter.rb

app/services/versioning/serializers/base_serializer.rb

github-actions · 2024-08-12T09:44:35Z

app/services/versioning/serializers/container_serializer.rb

@@ -0,0 +1,90 @@
+# frozen_string_literal: true
+
+class Versioning::Serializers::ContainerSerializer < Versioning::Serializers::BaseSerializer


Style/ClassAndModuleChildren: Use nested module/class definitions instead of compact style.

github-actions · 2024-08-12T09:44:35Z

app/services/versioning/serializers/container_serializer.rb

+    new(record: record, name: name).call
+  end
+
+  def field_definitions


Metrics/AbcSize: Assignment Branch Condition size for field_definitions is too high. [<14, 31, 5> 34.38/25]

JanCBrammer

I made a few suggestions to revert the changes to semantically unchanged files.
This would make the PR a bit smaller which would make working with the PR in the GitHub UI a bit easier. Atm, the UI is quite unresponsive due to to its size (large diffs in many files).

I'll continue the review over the coming days.

app/api/chemotion/sample_api.rb

app/assets/stylesheets/sample-details.scss

JanCBrammer · 2024-10-01T10:40:34Z

app/models/container_hierarchy.rb

Why do we need this stub?

app/models/reactions_product_sample.rb

app/models/sample.rb

public/klasses.json

public/jsmol

app/packs/src/stores/alt/actions/ElementActions.js

JanCBrammer · 2024-10-01T11:12:51Z

app/packs/src/apps/mydb/elements/details/VersionsTable.js

+import ScreensFetcher from 'src/fetchers/ScreensFetcher';
+import WellplatesFetcher from 'src/fetchers/WellplatesFetcher';
+
+export default class VersionsTable extends Component {


Maybe convert this to a functional component? I feel like introducing new components might be an opportunity to move the codebase away from class components.

app/packs/src/apps/mydb/elements/details/VersionsTable.js

app/packs/src/apps/mydb/elements/details/VersionsTableChanges.js

JanCBrammer · 2024-10-02T08:05:54Z

app/packs/src/apps/mydb/elements/details/VersionsTableChanges.js

+  const alertText = revertibleFields() < 0
+    ? 'You cannot undo changes. You have unsaved data which would be lost.'
+    : `You cannot undo these changes.
+       Either the changes are up to date, it is the first version or all changes are irreversible.`;


From a user perspective it would be great if this alert would be more specific, i.e., the three cases that are lumped together in the fallback of the ternary could be differentiated.

app/packs/src/apps/mydb/elements/details/VersionsTableModal.js

JanCBrammer · 2024-10-02T08:49:48Z

app/packs/src/apps/mydb/elements/details/reactions/ReactionDetails.js

Please revert the formatting changes, otherwise this file's diff is unnecessarily hard to review.

app/packs/src/apps/mydb/elements/details/samples/propertiesTab/SampleForm.js

JanCBrammer · 2024-10-02T09:04:58Z

app/packs/src/apps/mydb/elements/details/screens/ScreenDetails.js

Please revert the formatting changes, otherwise this file's diff is unnecessarily hard to review.

app/packs/src/fetchers/SamplesFetcher.js

app/packs/src/models/ResearchPlan.js

app/packs/src/models/Sample.js

github-actions · 2024-10-09T10:34:58Z

app/models/sample.rb

@@ -112,45 +113,59 @@ class Sample < ApplicationRecord
  pg_search_scope :search_by_cas, against: { xref: 'cas' }

  # scopes for suggestions
-  scope :by_residues_custom_info, ->(info, val) { joins(:residues).where("residues.custom_info -> '#{info}' ILIKE ?", "%#{sanitize_sql_like(val)}%")}
+  scope :by_residues_custom_info, lambda { |info, val|
+                                    joins(:residues).where("residues.custom_info -> '#{info}' ILIKE ?", "%#{sanitize_sql_like(val)}%")


Layout/LineLength: Line is too long. [134/120]

github-actions · 2024-10-09T10:34:58Z

app/models/sample.rb

  scope :by_name, ->(query) { where('name ILIKE ?', "%#{sanitize_sql_like(query)}%") }
  scope :by_sample_xref_cas,
        ->(query) { where("xref ? 'cas'").where("xref ->> 'cas' ILIKE ?", "%#{sanitize_sql_like(query)}%") }
-  scope :by_exact_name, ->(query) { where('lower(name) ~* lower(?) or lower(external_label) ~* lower(?)', "^([a-zA-Z0-9]+-)?#{sanitize_sql_like(query)}(-?[a-zA-Z])$", "^([a-zA-Z0-9]+-)?#{sanitize_sql_like(query)}(-?[a-zA-Z])$") }
+  scope :by_exact_name, lambda { |query|
+                          where('lower(name) ~* lower(?) or lower(external_label) ~* lower(?)', "^([a-zA-Z0-9]+-)?#{sanitize_sql_like(query)}(-?[a-zA-Z])$", "^([a-zA-Z0-9]+-)?#{sanitize_sql_like(query)}(-?[a-zA-Z])$")


Layout/LineLength: Line is too long. [217/120]

github-actions · 2024-10-09T10:34:58Z

app/models/sample.rb

-  scope :by_reaction_solvent_ids,  ->(ids) { joins(:reactions_solvent_samples).where('reactions_samples.reaction_id in (?)', ids) }
-  scope :by_reaction_ids,          ->(ids) { joins(:reactions_samples).where('reactions_samples.reaction_id in (?)', ids) }
+  scope :by_reaction_reactant_ids, lambda { |ids|
+                                     joins(:reactions_reactant_samples).where('reactions_samples.reaction_id in (?)', ids)


Rails/WhereEquals: Use where(reactions_samples: { reaction_id: ids }) instead of manually constructing SQL.

github-actions · 2024-10-09T10:34:59Z

app/models/sample.rb

-  scope :by_reaction_solvent_ids,  ->(ids) { joins(:reactions_solvent_samples).where('reactions_samples.reaction_id in (?)', ids) }
-  scope :by_reaction_ids,          ->(ids) { joins(:reactions_samples).where('reactions_samples.reaction_id in (?)', ids) }
+  scope :by_reaction_reactant_ids, lambda { |ids|
+                                     joins(:reactions_reactant_samples).where('reactions_samples.reaction_id in (?)', ids)


Layout/LineLength: Line is too long. [122/120]

github-actions · 2024-10-09T10:34:59Z

app/models/sample.rb

+                                     joins(:reactions_reactant_samples).where('reactions_samples.reaction_id in (?)', ids)
+                                   }
+  scope :by_reaction_product_ids,  lambda { |ids|
+                                     joins(:reactions_product_samples).where('reactions_samples.reaction_id in (?)', ids)


Rails/WhereEquals: Use where(reactions_samples: { reaction_id: ids }) instead of manually constructing SQL.

github-actions · 2024-10-09T10:35:00Z

app/models/sample.rb

-    self.creator.increment_counter 'samples'
+    return unless /^#{creator.name_abbreviation}-\d+$/.match?(short_label)
+
+    creator.increment_counter 'samples'


Rails/SkipsModelValidations: Avoid using increment_counter because it skips validations.

github-actions · 2024-10-09T10:35:01Z

app/models/wellplate.rb

@@ -132,7 +133,7 @@ def create_subwellplate(user, collection_ids, _copy_ea = false)
        readouts: w.readouts,
        additive: w.additive,
        position_x: w.position_x,
-        position_y: w.position_y
+        position_y: w.position_y,


Metrics/AbcSize: Assignment Branch Condition size for create_subwellplate is too high. [<11, 30, 4> 32.2/25]

github-actions · 2024-10-09T10:35:01Z

app/models/wellplate.rb

@@ -132,7 +133,7 @@ def create_subwellplate(user, collection_ids, _copy_ea = false)
        readouts: w.readouts,
        additive: w.additive,
        position_x: w.position_x,
-        position_y: w.position_y
+        position_y: w.position_y,


Metrics/MethodLength: Method has too many lines. [31/30]

github-actions · 2024-10-09T10:35:01Z

app/services/versioning/reverter.rb

+    end
+
+    def call
+      changes.each do |change|


Metrics/BlockLength: Block has too many lines. [26/25]

github-actions · 2024-10-09T10:35:01Z

app/services/versioning/reverters/well_reverter.rb

@@ -0,0 +1,7 @@
+# frozen_string_literal: true
+
+class Versioning::Reverters::WellReverter < Versioning::Reverters::BaseReverter


Style/ClassAndModuleChildren: Use nested module/class definitions instead of compact style.

github-actions · 2024-10-09T10:35:03Z

app/services/versioning/reverters/wellplate_reverter.rb

@@ -0,0 +1,7 @@
+# frozen_string_literal: true
+
+class Versioning::Reverters::WellplateReverter < Versioning::Reverters::BaseReverter


Style/ClassAndModuleChildren: Use nested module/class definitions instead of compact style.

github-actions · 2024-10-09T10:35:03Z

app/services/versioning/serializers/container_serializer.rb

+    new(record: record, name: name).call
+  end
+
+  def field_definitions


Metrics/MethodLength: Method has too many lines. [73/30]

github-actions · 2024-10-09T10:35:04Z

app/services/versioning/serializers/container_serializer.rb

+            JSON.parse(result).join("\n")
+          },
+          revert: %i[extended_metadata.hyperlinks],
+          revertible_value_formatter: ->(key, value) { JSON.parse(jsonb_formatter('hyperlinks').call(key, value) || '[]') },


Layout/LineLength: Line is too long. [124/120]

github-actions · 2024-10-09T10:35:04Z

app/services/versioning/serializers/elemental_composition_serializer.rb

@@ -0,0 +1,33 @@
+# frozen_string_literal: true
+
+class Versioning::Serializers::ElementalCompositionSerializer < Versioning::Serializers::BaseSerializer


Style/ClassAndModuleChildren: Use nested module/class definitions instead of compact style.

github-actions · 2024-10-09T10:35:04Z

app/services/versioning/serializers/reaction_serializer.rb

@@ -0,0 +1,88 @@
+# frozen_string_literal: true
+
+class Versioning::Serializers::ReactionSerializer < Versioning::Serializers::BaseSerializer


Style/ClassAndModuleChildren: Use nested module/class definitions instead of compact style.

github-actions · 2024-10-09T10:35:05Z

app/services/versioning/serializers/research_plan_serializer.rb

+    end
+  end
+
+  def lookups


Metrics/PerceivedComplexity: Perceived complexity for lookups is too high. [10/8]

github-actions · 2024-10-09T10:35:05Z

app/services/versioning/serializers/residue_serializer.rb

@@ -0,0 +1,68 @@
+# frozen_string_literal: true
+
+class Versioning::Serializers::ResidueSerializer < Versioning::Serializers::BaseSerializer


Style/ClassAndModuleChildren: Use nested module/class definitions instead of compact style.

github-actions · 2024-10-09T10:35:05Z

app/services/versioning/serializers/residue_serializer.rb

+    new(record: record, name: name).call
+  end
+
+  def field_definitions


Metrics/MethodLength: Method has too many lines. [44/30]

github-actions · 2024-10-09T10:35:05Z

app/services/versioning/serializers/screen_serializer.rb

@@ -0,0 +1,41 @@
+# frozen_string_literal: true
+
+class Versioning::Serializers::ScreenSerializer < Versioning::Serializers::BaseSerializer


Style/ClassAndModuleChildren: Use nested module/class definitions instead of compact style.

github-actions · 2024-10-09T10:35:05Z

app/services/versioning/serializers/screen_serializer.rb

+    new(record: record, name: name).call
+  end
+
+  def field_definitions


Metrics/MethodLength: Method has too many lines. [31/30]

app/packs/src/apps/mydb/elements/details/VersionsTableChanges.js

JanCBrammer · 2024-10-15T05:51:45Z

app/packs/src/apps/mydb/elements/details/VersionsTableChanges.js

+  const isRevertible = revertibleFields();
+  let alertText = 'You cannot undo changes. ';
+  if (isRevertible === 0) {
+    alertText += 'Every change is either up to date or irreversible';


Now the first and last conditional branch both contain the "all changes are irreversible" case.

github-actions · 2024-10-16T11:44:48Z

LCOV of commit `7c8ac70` during Continuous Integration #4020

Summary coverage rate:
  lines......: 61.6% (14094 of 22897 lines)
  functions..: no data found
  branches...: no data found

Files changed coverage rate: n/a

github-actions · 2024-10-22T14:01:57Z

LCOV of commit `89dd152` during Continuous Integration #4048

Summary coverage rate:
  lines......: 61.6% (14081 of 22876 lines)
  functions..: no data found
  branches...: no data found

Files changed coverage rate: n/a

JanCBrammer

This feature needs tests 🧐

JanCBrammer

Tests look ok. Nice that there's at least some basic coverage now.

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 · 2024-11-08T09:25:53Z

LCOV of commit `ab6c437` during Continuous Integration #4128

Summary coverage rate:
  lines......: 64.8% (14411 of 22227 lines)
  functions..: no data found
  branches...: no data found

Files changed coverage rate: n/a

PiTrem marked this pull request as draft August 12, 2024 09:43

github-actions bot reviewed Aug 12, 2024

View reviewed changes

headri force-pushed the versioning branch from 9106529 to d7a8b37 Compare September 19, 2024 14:12

headri marked this pull request as ready for review September 26, 2024 14:37

headri requested a review from JanCBrammer September 26, 2024 14:37

JanCBrammer requested changes Oct 1, 2024

View reviewed changes

JanCBrammer requested changes Oct 2, 2024

View reviewed changes

github-actions bot reviewed Oct 9, 2024

View reviewed changes

JanCBrammer reviewed Oct 15, 2024

View reviewed changes

headri force-pushed the versioning branch 2 times, most recently from afdcee9 to 89dd152 Compare October 22, 2024 13:38

JanCBrammer requested changes Oct 23, 2024

View reviewed changes

PiTrem changed the title ~~Versioning~~ feat: data versioning Nov 7, 2024

PiTrem marked this pull request as draft November 7, 2024 10:38

PiTrem added this to the v2.0.0 milestone Nov 7, 2024

JanCBrammer approved these changes Nov 8, 2024

View reviewed changes

mschneider85 and others added 10 commits November 8, 2024 08:52

Add svg formatter and json

1741ea7

Add revert button to versioned field reset colum information re-create schema update solvent label

refactor: various files

e176e2a

refactored Form and removed css

109768a

feat: Added Version Table to Sample as All Versions Tab

78db23e

added author to all_versions

b447648

feat: versions tab syncs with properties - reloads on click

369f0f5

feat: added fields to sample serializer

e199277

feat: added versioning to reactions

cab0fc9

Adrian Herrmann added 12 commits November 8, 2024 08:53

feat: added versions to research_plan

df6106e

feat: added versions to screen

4440258

fix: mistakes from rebase

1c3d0a6

feat: added setState after revert to screens and research_plans

48541ad

feat: added wellplate to versioning

b83d32f

feat: added wells to versioning

f2d7bf2

review findings: reverted some refactorings & specified alert text

13f2dd2

reverted refactoring in sample.rb

67f153a

refactor: changed BootstrapTable to AgGridReact

e7b55dd

fix: rebase on bootstrap update

ac70bcb

sample detail rebase fix

161d0f4

added tests for version_api

ab6c437

headri force-pushed the versioning branch from e048bea to ab6c437 Compare November 8, 2024 09:03

headri marked this pull request as ready for review November 8, 2024 09:15

PiTrem mentioned this pull request Nov 12, 2024

Feature/versioning enhanced #1253

Closed

10 tasks

		@@ -0,0 +1,60 @@
		# frozen_string_literal: true

		class Versioning::Fetchers::ReactionFetcher

		@@ -0,0 +1,34 @@
		# frozen_string_literal: true

		class Versioning::Fetchers::ScreenFetcher

		@@ -0,0 +1,38 @@
		# frozen_string_literal: true

		class Versioning::Merger

		@@ -0,0 +1,90 @@
		# frozen_string_literal: true

		class Versioning::Serializers::ContainerSerializer < Versioning::Serializers::BaseSerializer

		@@ -0,0 +1,7 @@
		# frozen_string_literal: true

		class Versioning::Reverters::WellReverter < Versioning::Reverters::BaseReverter

		@@ -0,0 +1,7 @@
		# frozen_string_literal: true

		class Versioning::Reverters::WellplateReverter < Versioning::Reverters::BaseReverter

		@@ -0,0 +1,33 @@
		# frozen_string_literal: true

		class Versioning::Serializers::ElementalCompositionSerializer < Versioning::Serializers::BaseSerializer

		@@ -0,0 +1,88 @@
		# frozen_string_literal: true

		class Versioning::Serializers::ReactionSerializer < Versioning::Serializers::BaseSerializer

		@@ -0,0 +1,68 @@
		# frozen_string_literal: true

		class Versioning::Serializers::ResidueSerializer < Versioning::Serializers::BaseSerializer

		@@ -0,0 +1,41 @@
		# frozen_string_literal: true

		class Versioning::Serializers::ScreenSerializer < Versioning::Serializers::BaseSerializer

feat: data versioning #2068

Are you sure you want to change the base?

feat: data versioning #2068

Conversation

PiTrem commented Aug 12, 2024 • edited by JanCBrammer Loading

github-actions bot Aug 12, 2024

Choose a reason for hiding this comment

github-actions bot Aug 12, 2024

Choose a reason for hiding this comment

github-actions bot Aug 12, 2024

Choose a reason for hiding this comment

github-actions bot Aug 12, 2024

Choose a reason for hiding this comment

github-actions bot Aug 12, 2024

Choose a reason for hiding this comment

github-actions bot Aug 12, 2024

Choose a reason for hiding this comment

github-actions bot Aug 12, 2024

Choose a reason for hiding this comment

github-actions bot Aug 12, 2024

Choose a reason for hiding this comment

github-actions bot Aug 12, 2024

Choose a reason for hiding this comment

github-actions bot Aug 12, 2024

Choose a reason for hiding this comment

github-actions bot Aug 12, 2024

Choose a reason for hiding this comment

github-actions bot Aug 12, 2024

Choose a reason for hiding this comment

JanCBrammer left a comment

Choose a reason for hiding this comment

JanCBrammer Oct 1, 2024

Choose a reason for hiding this comment

JanCBrammer Oct 1, 2024

Choose a reason for hiding this comment

JanCBrammer Oct 2, 2024

Choose a reason for hiding this comment

JanCBrammer Oct 2, 2024

Choose a reason for hiding this comment

JanCBrammer Oct 2, 2024

Choose a reason for hiding this comment

github-actions bot Oct 9, 2024

Choose a reason for hiding this comment

github-actions bot Oct 9, 2024

Choose a reason for hiding this comment

github-actions bot Oct 9, 2024

Choose a reason for hiding this comment

github-actions bot Oct 9, 2024

Choose a reason for hiding this comment

github-actions bot Oct 9, 2024

Choose a reason for hiding this comment

github-actions bot Oct 9, 2024

Choose a reason for hiding this comment

github-actions bot Oct 9, 2024

Choose a reason for hiding this comment

github-actions bot Oct 9, 2024

Choose a reason for hiding this comment

github-actions bot Oct 9, 2024

Choose a reason for hiding this comment

github-actions bot Oct 9, 2024

Choose a reason for hiding this comment

github-actions bot Oct 9, 2024

Choose a reason for hiding this comment

github-actions bot Oct 9, 2024

Choose a reason for hiding this comment

github-actions bot Oct 9, 2024

Choose a reason for hiding this comment

github-actions bot Oct 9, 2024

Choose a reason for hiding this comment

github-actions bot Oct 9, 2024

Choose a reason for hiding this comment

github-actions bot Oct 9, 2024

Choose a reason for hiding this comment

github-actions bot Oct 9, 2024

Choose a reason for hiding this comment

github-actions bot Oct 9, 2024

Choose a reason for hiding this comment

github-actions bot Oct 9, 2024

Choose a reason for hiding this comment

github-actions bot Oct 9, 2024

PiTrem commented Aug 12, 2024 •

edited by JanCBrammer

Loading

LCOV of commit `7c8ac70` during Continuous Integration #4020

LCOV of commit `89dd152` during Continuous Integration #4048

LCOV of commit `ab6c437` during Continuous Integration #4128