Reintroduce detail level system (#943)

* Create infrastructure to configure Entities for different Detail Levels

* Introduce ElementDetailLevelCalculator

This class reimplements the logic from the ElementPermissionProxy.
As the name suggests, it only calculates the detail levels (more efficiently I think).
It does explicitly NOT include functionality to serialize elements.
This should be done separately (see ApplicationEntity#entity_for_level).

* Fix some lint issues

* Fix internal datastructure in ElementDetailLevelCalculator

* Fix bugs in ElementDetailLevelCalculator

* Add expose! method with additional features

- anonymize_below: X -> anonymizes the field with a default string if the current detail level of the object is lower than X
- anonymize_with: Y -> uses Y as a replacement when a value gets anonymized

* Some fixes

* Fix usage of with_options

* Fix specs

* Add specs for calculator and application entity

* Use with_options to make SampleEntity a little more readable

* Remove analyses field from sample entity as it is not used in the frontend and the anonymization code was broken anyway

* Introduce detail levels into WellplateEntity

* Introduce detail levels to WellEntity

* Fix well entity

* Fix anonymization bugs in SampleEntity

* Use expose! in WellplateEntity and fix some anonymizations

* Use expose! in WellEntity and fix double expose of readouts

* Rework ResearchPlanEntity a little, but it does not use anonymization anyway

* Introduce detail levels into Screen

* Fix syntax bug in ResearchPlanEntity

* Fix ElementDetailLevelCalculatorSpec

* Remove detail_level method from ElementDetailLevelCalculator

As we discussed, we will only use the simpler logic of nested_detail_levels,
based on the maximum configured level over all accessible collections.
This removes a lot of special cases (that were not required anyway).

Further optimization will be possible when the shared and synced collections
feature gets merged, so we only have one of those left.

* Reintroduce anonymization to ElementAPI

* Reintroduce anonymization to SampleAPI

* Reintroduce anonymization to ScreenAPI

* Reintroduce anonymization to SearchAPI

* Reintroduce anonymization to WellplateAPI

* Reintroduce anonymization to ResearchPlanAPI

* Reintroduce anonymization into ReactionAPI

* WIP find a better workflow to test entities

* Fix logic bug in ApplicationEntity that made anonymization options within with_options not work properly

* Add final spec for sample entity

* Add specs for 2 special cases

* Add specs for well entity

* Add specs for wellplate entity

* Fix some linter issues

* Fix more rubocop issues

* Fix rubocop issues in classes

* Add specs for screen entity

* Update ResearchPlanEntity

* Add anonymization to SampleReportEntity

* Refactor LiteratureEntity

* Make literatures method in SampleReportEntity private

* Fix invocation of ElementDetailLevelCalculator in ReactionAPI

* Inntroduce anonymization to ReactionEntity

* Add default for detail_levels in ElementDetailLevelCalculator

* Fix specs that broke after introducing anonymization

* Use expose! in ReactionMaterialEntity

* Introduce anonymization and analyses into ReactionMaterialReportEntity

* Fix syntax issue in SampleReportEntity

* Fix filenames of several spec files

* Prevent deprecation warnings about unsafe SQL strings

* Fix reporter specs for csv and html

* Raise exception when inkscape is not installed

* Rework ReactionReportEntity to include anonymization and inheritance from ReactionEntity

* Introduce anonymization into ElementEntity

* Reintroduce detail levels into GenericElementAPI

* Make PurificationSolvents available for DetailLevel0

* Add specs for research plan entity

* WIP Add specs for reaction

* Make usage of ApplicationEntity#detail_levels less error-prone

When no detail levels are configured, 10 is returned for every requested class.
When detail levels are configured, every request for an unconfigured class returns 0, configured classes return their respective configuration.

This allows us to use simple hashes in our specs, instead of relying on the result of the ElementDetailLevelCalculator.

* Unify usage of expose! in ReactionMaterialEntity

* Set Reporter::Xlsx::ReactionList spec to pending

The spec tries to parse a Xlsx file generated by Axlsx using Roo.
The currently used Roo gem has a parser implementation that does not properly understand
the valid xml structure Axlsx generates, so the spec fails.

The newer version Roo 2.9.0 appears to have addressed this bug but requires Ruby 2.7 to run.
Until we complete the Rails 6 Upgrade, this spec is pending

* Add reaction entity spec

* Add spec for sample report spec

* Reintroduce Detail levels to some more codepaths

* Fix reaction material Entity to display analyses properly

* Remove fcontext and binding.pry

* Add ci image with inkscape dependency

* Fix ci image again

* Add a working ci image

* Fix spec

* Disable spec with missing dependency

Co-authored-by: Johannes Haubold <jh@megorei.com>

.github/workflows/testacceptance.yml

@@ -18,7 +18,7 @@ jobs:
         pg_database_test: [chemotion_test]
         pg_password: [123456]
     container:
-      image: complat/complat-ubuntu-runner:development-5.c44d0c2d4
+      image: complat/complat-ubuntu-runner:development-5.0b920674f8
     env:
       HOME: /home/gitlab-runner
 

.github/workflows/testjs.yml

@@ -15,7 +15,7 @@ jobs:
         pg_database: [chemotion_test]
         pg_password: [123456]
     container:
-      image: complat/complat-ubuntu-runner:development-5.c44d0c2d4
+      image: complat/complat-ubuntu-runner:development-5.0b920674f8
     env:
       HOME: /home/gitlab-runner
 

.github/workflows/testrb.yml

@@ -14,7 +14,7 @@ jobs:
         pg_role: [chemotion_test]
         pg_database: [chemotion_test]
         pg_password: [123456]
-    container: complat/complat-ubuntu-runner:development-5.c44d0c2d4
+    container: complat/complat-ubuntu-runner:development-5.0b920674f8
     env:
       HOME: /home/gitlab-runner
 

.gitlab-ci.yml

@@ -1,4 +1,4 @@
-image: "complat/complat-ubuntu-runner:development-5.c44d0c2d4"
+image: "complat/complat-ubuntu-runner:development-5.0b920674f8"
 
 services:
   - postgres:12-alpine

.rubocop.yml

@@ -40,6 +40,13 @@ Style/TrailingCommaInHashLiteral:
   Enabled: true
   EnforcedStyleForMultiline: comma
 
+Style/TrailingCommaInArguments:
+  Enabled: true
+  EnforcedStyleForMultiline: comma
+
 Metrics/BlockLength:
   Exclude:
   - 'spec/**/*_spec.rb'
+
+RSpec/ExampleLength:
+  Enabled: false

app/api/chemotion/element_api.rb

@@ -97,12 +97,24 @@ class ElementAPI < Grape::API
       post do
         result = { 'samples' => [], 'reactions' => [] }
 
-        # TODO: optimize includes for performance
+        # TODO: optimize for performance
+        #       The calculator is not really a good way to check for large amounts of elements,
+        #       as it will fetch all containing collections for EACH element.
+        #       The more elements are selected, the more SQL queries get executed.
         if params['sample'][:checkedAll] || params['sample'][:checkedIds].any?
-          result['samples'] = Entities::SampleEntity.represent(@collection.samples.by_ui_state(params['sample']))
+          result['samples'] = @collection.samples.by_ui_state(params['sample']).map do |sample|
+            calculator = ElementDetailLevelCalculator.new(user: current_user, element: sample)
+
+            Entities::SampleEntity.represent(sample, detail_levels: calculator.detail_levels)
+          end
         end
+
         if params['reaction'][:checkedAll] || params['reaction'][:checkedIds].any?
-          result['reactions'] = Entities::ReactionEntity.represent(@collection.reactions.by_ui_state(params['reaction']))
+          result['reactions'] = @collection.reactions.by_ui_state(params['reaction']).map do |reaction|
+            calculator = ElementDetailLevelCalculator.new(user: current_user, element: reaction)
+
+            Entities::ReactionEntity.represent(reaction, detail_levels: calculator.detail_levels)
+          end
         end
 
         # TODO: fallback if sample are not in owned collection and currentCollection is missing
@@ -141,12 +153,20 @@ class ElementAPI < Grape::API
               :container, :products, :purification_solvents, :reactants, :segments, :solvents, :starting_materials, :tag
             )
             result['samples'] = samples.map do |sample|
-              serialized_element = Entities::SampleEntity.represent(sample).serializable_hash
+              detail_levels = ElementDetailLevelCalculator.new(user: current_user, element: sample).detail_levels
+              serialized_element = Entities::SampleEntity.represent(
+                sample,
+                detail_levels: detail_levels
+              ).serializable_hash
               serialized_element[:literatures] = citation_for_elements(sample.id, 'Sample')
               serialized_element
             end
             result['reactions'] = reactions.map do |reaction|
-              serialized_element = Entities::ReactionEntity.represent(reaction).serializable_hash
+              detail_levels = ElementDetailLevelCalculator.new(user: current_user, element: reaction).detail_levels
+              serialized_element = Entities::ReactionEntity.represent(
+                reaction,
+                detail_levels: detail_levels
+              ).serializable_hash
               serialized_element[:literatures] = citation_for_elements(reaction.id, 'Reaction')
               serialized_element
             end
@@ -157,14 +177,16 @@ class ElementAPI < Grape::API
               if sample_tags && sample.id.in?(sample_tags)
                 { id: sample.id, in_browser_memory: true }
               else
-                Entities::SampleEntity.represent(sample, displayed_in_list: true)
+                detail_levels = ElementDetailLevelCalculator.new(user: current_user, element: sample).detail_levels
+                Entities::SampleEntity.represent(sample, detail_levels: detail_levels, displayed_in_list: true)
               end
             end
             result['reactions'] = reactions.includes_for_list_display.map do |reaction|
               if reaction_tags && reaction.id.in?(reaction_tags)
                 { id: reaction.id, in_browser_memory: true }
               else
-                Entities::ReactionEntity.represent(reaction, displayed_in_list: true)
+                detail_levels = ElementDetailLevelCalculator.new(user: current_user, element: reaction).detail_levels
+                Entities::ReactionEntity.represent(reaction, detail_levels: detail_levels, displayed_in_list: true)
               end
             end
           end

app/api/chemotion/generic_element_api.rb

@@ -147,7 +147,13 @@ class GenericElementAPI < Grape::API
 
         reset_pagination_page(scope)
 
-        present paginate(scope), with: Entities::ElementEntity
+        paginate(scope).map do |element|
+          Entities::ElementEntity.represent(
+            element,
+            displayed_in_list: true,
+            detail_levels: ElementDetailLevelCalculator.new(user: current_user, element: element).detail_levels
+          )
+        end
       end
 
       desc 'Return serialized element by id'
@@ -162,7 +168,10 @@ class GenericElementAPI < Grape::API
         get do
           element = Element.find(params[:id])
           {
-            element: Entities::ElementEntity.represent(element),
+            element: Entities::ElementEntity.represent(
+              element,
+              detail_levels: ElementDetailLevelCalculator.new(user: current_user, element: element).detail_levels
+            ),
             attachments: Entities::AttachmentEntity.represent(element.attachments)
           }
         end
@@ -208,7 +217,12 @@ class GenericElementAPI < Grape::API
         element.save!
         element.save_segments(segments: params[:segments], current_user_id: current_user.id)
 
-        present element, with: Entities::ElementEntity, root: :element
+        present(
+          element,
+          with: Entities::ElementEntity,
+          root: :element,
+          detail_levels: ElementDetailLevelCalculator.new(user: current_user, element: element).detail_levels
+        )
       end
 
       desc 'Update element by id'
@@ -248,7 +262,10 @@ class GenericElementAPI < Grape::API
           element.save_segments(segments: params[:segments], current_user_id: current_user.id)
 
           {
-            element: Entities::ElementEntity.represent(element),
+            element: Entities::ElementEntity.represent(
+              element,
+              detail_levels: ElementDetailLevelCalculator.new(user: current_user, element: element).detail_levels
+            ),
             attachments: Entities::AttachmentEntity.represent(element.attachments)
           }
         end

app/api/chemotion/reaction_api.rb

@@ -57,7 +57,15 @@ class ReactionAPI < Grape::API
 
         reset_pagination_page(scope)
 
-        present paginate(scope), with: Entities::ReactionEntity, displayed_in_list: true, root: :reactions
+        reactions = paginate(scope).map do |reaction|
+          Entities::ReactionEntity.represent(
+            reaction,
+            displayed_in_list: true,
+            detail_levels: ElementDetailLevelCalculator.new(user: current_user, element: reaction).detail_levels
+          )
+        end
+
+        { reactions: reactions }
       end
 
       desc 'Return serialized reaction by id'
@@ -74,7 +82,11 @@ class ReactionAPI < Grape::API
           reaction = Reaction.find(params[:id])
 
           {
-            reaction: Entities::ReactionEntity.represent(reaction, policy: @element_policy),
+            reaction: Entities::ReactionEntity.represent(
+              reaction,
+              policy: @element_policy,
+              detail_levels: ElementDetailLevelCalculator.new(user: current_user, element: reaction).detail_levels
+            ),
             literatures: Entities::LiteratureEntity.represent(citation_for_elements(params[:id], 'Reaction'))
           }
         end
@@ -153,7 +165,13 @@ class ReactionAPI < Grape::API
           kinds = reaction.container&.analyses&.pluck(Arel.sql("extended_metadata->'kind'"))
           recent_ols_term_update('chmo', kinds) if kinds&.length&.positive?
 
-          present reaction, with: Entities::ReactionEntity, root: :reaction, policy: @element_policy
+          present(
+            reaction,
+            with: Entities::ReactionEntity,
+            detail_levels: ElementDetailLevelCalculator.new(user: current_user, element: reaction).detail_levels,
+            root: :reaction,
+            policy: @element_policy
+          )
         end
       end
 
@@ -261,7 +279,12 @@ class ReactionAPI < Grape::API
           kinds = reaction.container&.analyses&.pluck(Arel.sql("extended_metadata->'kind'"))
           recent_ols_term_update('chmo', kinds) if kinds&.length&.positive?
 
-          present reaction, with: Entities::ReactionEntity, root: :reaction
+          present(
+            reaction,
+            with: Entities::ReactionEntity,
+            detail_levels: ElementDetailLevelCalculator.new(user: current_user, element: reaction).detail_levels,
+            root: :reaction
+          )
         end
       end
     end

app/api/chemotion/research_plan_api.rb

@@ -48,7 +48,15 @@ class ResearchPlanAPI < Grape::API
 
         reset_pagination_page(scope)
 
-        present paginate(scope), with: Entities::ResearchPlanEntity, displayed_in_list: true, root: :research_plans
+        research_plans = paginate(scope).map do |research_plan|
+          Entities::ResearchPlanEntity.represent(
+            research_plan,
+            displayed_in_list: true,
+            detail_levels: ElementDetailLevelCalculator.new(user: current_user, element: research_plan).detail_levels
+          )
+        end
+
+        { research_plans: research_plans }
       end
 
       desc 'Create a research plan'
@@ -147,7 +155,10 @@ class ResearchPlanAPI < Grape::API
             subject: ''
           ) if research_plan.research_plan_metadata.nil?
           {
-            research_plan: Entities::ResearchPlanEntity.represent(research_plan),
+            research_plan: Entities::ResearchPlanEntity.represent(
+              research_plan,
+              detail_levels: ElementDetailLevelCalculator.new(user: current_user, element: research_plan).detail_levels
+            ),
             attachments: Entities::AttachmentEntity.represent(research_plan.attachments),
           }
         end
@@ -175,9 +186,10 @@ class ResearchPlanAPI < Grape::API
           if research_plan = ResearchPlan.find(params[:id])
             research_plan.update!(attributes)
             research_plan.save_segments(segments: params[:segments], current_user_id: current_user.id)
-
           end
-          present research_plan, with: Entities::ResearchPlanEntity, root: :research_plan
+
+          detail_levels = ElementDetailLevelCalculator.new(user: current_user, element: research_plan).detail_levels
+          present research_plan, with: Entities::ResearchPlanEntity, detail_levels: detail_levels, root: :research_plan
         end
       end
 
@@ -296,7 +308,9 @@ class ResearchPlanAPI < Grape::API
       route_param :id do
         before do
           error!('401 Unauthorized', 401) unless ElementPolicy.new(current_user, ResearchPlan.find(params[:id])).update?
-          error!('401 Unauthorized', 401) unless ElementPolicy.new(current_user, Wellplate.find(params[:wellplate_id])).read?
+          error!('401 Unauthorized', 401) unless ElementPolicy.new(
+            current_user, Wellplate.find(params[:wellplate_id])
+          ).read?
         end
 
         post 'import_wellplate/:wellplate_id' do
@@ -307,7 +321,12 @@ class ResearchPlanAPI < Grape::API
             exporter.execute!
 
             {
-              research_plan: Entities::ResearchPlanEntity.represent(research_plan),
+              research_plan: Entities::ResearchPlanEntity.represent(
+                research_plan,
+                detail_levels: ElementDetailLevelCalculator.new(
+                  user: current_user, element: research_plan
+                ).detail_levels
+              ),
               attachments: Entities::AttachmentEntity.represent(research_plan.attachments),
             }
           rescue StandardError => e
@@ -335,7 +354,12 @@ class ResearchPlanAPI < Grape::API
             exporter.execute!
 
             {
-              research_plan: Entities::ResearchPlanEntity.represent(research_plan),
+              research_plan: Entities::ResearchPlanEntity.represent(
+                research_plan,
+                detail_levels: ElementDetailLevelCalculator.new(
+                  user: current_user, element: research_plan
+                ).detail_levels
+              ),
               attachments: Entities::AttachmentEntity.represent(research_plan.attachments),
             }
           rescue StandardError => e

app/api/chemotion/sample_api.rb

@@ -213,20 +213,26 @@ class SampleAPI < Grape::API
         if params[:molecule_sort] == 1
           molecule_scope = Molecule
                            .where(id: (sample_scope.pluck :molecule_id))
-                           .order("LENGTH(SUBSTRING(sum_formular, 'C\\d+'))")
+                           .order(Arel.sql("LENGTH(SUBSTRING(sum_formular, 'C\\d+'))"))
                            .order(:sum_formular)
           reset_pagination_page(molecule_scope)
           paginate(molecule_scope).each do |molecule|
             samplesGroup = sample_scope.select {|v| v.molecule_id == molecule.id}
             samplesGroup = samplesGroup.sort { |x, y| y.updated_at <=> x.updated_at }
             samplesGroup.each do |sample|
-              samplelist.push(Entities::SampleEntity.represent(sample, displayed_in_list: true))
+              detail_levels = ElementDetailLevelCalculator.new(user: current_user, element: sample).detail_levels
+              samplelist.push(
+                Entities::SampleEntity.represent(sample, detail_levels: detail_levels, displayed_in_list: true)
+              )
             end
           end
         else
           sample_scope = sample_scope.order('updated_at DESC')
           paginate(sample_scope).each do |sample|
-            samplelist.push(Entities::SampleEntity.represent(sample, displayed_in_list: true))
+            detail_levels = ElementDetailLevelCalculator.new(user: current_user, element: sample).detail_levels
+            samplelist.push(
+              Entities::SampleEntity.represent(sample, detail_levels: detail_levels, displayed_in_list: true)
+            )
           end
         end
 
@@ -249,8 +255,13 @@ class SampleAPI < Grape::API
         get do
           sample = Sample.includes(:molecule, :residues, :elemental_compositions, :container)
                         .find(params[:id])
-
-          present sample, with: Entities::SampleEntity, policy: @element_policy, root: :sample
+          present(
+            sample,
+            with: Entities::SampleEntity,
+            detail_levels: ElementDetailLevelCalculator.new(user: current_user, element: sample).detail_levels,
+            policy: @element_policy,
+            root: :sample
+          )
         end
       end
 
@@ -373,12 +384,13 @@ class SampleAPI < Grape::API
           kinds = @sample.container&.analyses&.pluck(Arel.sql("extended_metadata->'kind'"))
           recent_ols_term_update('chmo', kinds) if kinds&.length&.positive?
 
-          var_detail_level = db_exec_detail_level_for_sample(current_user.id, @sample.id)
-          nested_detail_levels = {}
-          nested_detail_levels[:sample] = var_detail_level[0]['detail_level_sample'].to_i
-          nested_detail_levels[:wellplate] = [var_detail_level[0]['detail_level_wellplate'].to_i]
-
-          present @sample, with: Entities::SampleEntity, policy: @element_policy, root: :sample
+          present(
+            @sample,
+            with: Entities::SampleEntity,
+            detail_levels: ElementDetailLevelCalculator.new(user: current_user, element: @sample).detail_levels,
+            policy: @element_policy,
+            root: :sample
+          )
         end
       end