Fix: display contact for get submissions (#45)

* add get submission all including all properties test

* extract and use submission_include_params where we use submission.bring

* use retrieve_submissions helper in the :acronym/submissions endpoint

Gemfile.lock

@@ -1,6 +1,6 @@
 GIT
   remote: https://github.com/ncbo/ncbo_ontology_recommender.git
-  revision: 83e835de368bc9f19da800a477982e0ad770900d
+  revision: f440ae855a217807fead1d20629a0f187997b973
   branch: master
   specs:
     ncbo_ontology_recommender (0.0.1)
@@ -11,7 +11,7 @@ GIT
 
 GIT
   remote: https://github.com/ontoportal-lirmm/goo.git
-  revision: 1d78bde5a711d05475da0459308c7db074af5e21
+  revision: 0c0dba92e28fd8c8261db1b3183946e0cf183b53
   branch: development
   specs:
     goo (0.0.2)
@@ -53,7 +53,7 @@ GIT
 
 GIT
   remote: https://github.com/ontoportal-lirmm/ontologies_linked_data.git
-  revision: 4c89c8346766d23e09b24c8e29750bf3a91e6b53
+  revision: 1204ede68ed0a5af5e3fb355172496d5e0134544
   branch: development
   specs:
     ontologies_linked_data (0.0.1)
@@ -173,10 +173,9 @@ GEM
       retriable (>= 2.0, < 4.a)
       rexml
       webrick
-    googleauth (1.7.0)
+    googleauth (1.8.0)
       faraday (>= 0.17.3, < 3.a)
       jwt (>= 1.4, < 3.0)
-      memoist (~> 0.16)
       multi_json (~> 1.11)
       os (>= 0.9, < 2.0)
       signet (>= 0.16, < 2.a)
@@ -205,7 +204,6 @@ GEM
       net-imap
       net-pop
       net-smtp
-    memoist (0.16.2)
     method_source (1.0.0)
     mime-types (3.5.1)
       mime-types-data (~> 3.2015)
@@ -231,7 +229,7 @@ GEM
       net-protocol
     net-ssh (7.2.0)
     netrc (0.11.0)
-    newrelic_rpm (9.4.2)
+    newrelic_rpm (9.5.0)
     oj (2.18.5)
     omni_logger (0.1.4)
       logger
@@ -248,7 +246,7 @@ GEM
       rack (>= 0.4)
     rack-attack (6.6.1)
       rack (>= 1.0, < 3)
-    rack-cache (1.14.0)
+    rack-cache (1.13.0)
       rack (>= 0.4)
     rack-cors (1.0.6)
       rack (>= 1.6.0)
@@ -294,7 +292,7 @@ GEM
     rubyzip (2.3.2)
     rufus-scheduler (2.0.24)
       tzinfo (>= 0.3.22)
-    signet (0.17.0)
+    signet (0.18.0)
       addressable (~> 2.8)
       faraday (>= 0.17.5, < 3.a)
       jwt (>= 1.5, < 3.0)
@@ -326,7 +324,7 @@ GEM
       net-ssh (>= 2.8.0)
     systemu (2.6.5)
     temple (0.10.2)
-    tilt (2.2.0)
+    tilt (2.3.0)
     timeout (0.4.0)
     trailblazer-option (0.1.2)
     tzinfo (2.0.6)

controllers/admin_controller.rb

@@ -68,7 +68,7 @@ class AdminController < ApplicationController
       latest = ont.latest_submission(status: :any)
       error 404, "Ontology #{params["acronym"]} contains no submissions" if latest.nil?
       check_last_modified(latest)
-      latest.bring(*OntologySubmission.goo_attrs_to_load(includes_param))
+      latest.bring(*submission_include_params)
       NcboCron::Models::OntologySubmissionParser.new.queue_submission(latest, actions)
       halt 204
     end
@@ -84,7 +84,7 @@ class AdminController < ApplicationController
         latest = ont.latest_submission(status: :any)
       end
       check_last_modified(latest) if latest
-      latest.bring(*OntologySubmission.goo_attrs_to_load(includes_param)) if latest
+      latest.bring(*submission_include_params) if latest
       reply(latest || {})
     end
 

controllers/ontologies_controller.rb

@@ -38,22 +38,12 @@ class OntologiesController < ApplicationController
       else
         latest = ont.latest_submission(status: :any)
       end
-      check_last_modified(latest) if latest
-      # When asking to display all metadata, we are using bring_remaining which is more performant than including all metadata (remove this when the query to get metadata will be fixed)
-      if latest
-        if includes_param.first == :all
-          # Bring what we need to display all attr of the submission
-          latest.bring_remaining
-          latest.bring(*submission_attributes_all)
-        else
-          includes = OntologySubmission.goo_attrs_to_load(includes_param)
-
-          includes << {:contact=>[:name, :email]} if includes.find{|v| v.is_a?(Hash) && v.keys.first.eql?(:contact)}
 
-          latest.bring(*includes)
-        end
+      if latest
+        check_last_modified(latest)
+        latest.bring(*submission_include_params)
       end
-      #remove the whole previous if block and replace by it: latest.bring(*OntologySubmission.goo_attrs_to_load(includes_param)) if latest
+
       reply(latest || {})
     end
 
@@ -63,7 +53,7 @@ class OntologiesController < ApplicationController
     patch '/:acronym/latest_submission' do
       ont = Ontology.find(params["acronym"]).first
       error 422, "You must provide an existing `acronym` to patch" if ont.nil?
-      
+
       submission = ont.latest_submission(status: :any)
 
       submission.bring(*OntologySubmission.attributes)

controllers/ontology_submissions_controller.rb

@@ -29,17 +29,14 @@ class OntologySubmissionsController < ApplicationController
       error 422, "Ontology #{params["acronym"]} does not exist" unless ont
       check_last_modified_segment(LinkedData::Models::OntologySubmission, [ont.acronym])
       check_access(ont)
-      if includes_param.first == :all
-        # When asking to display all metadata, we are using bring_remaining which is more performant than including all metadata (remove this when the query to get metadata will be fixed)
-        ont.bring(submission_attributes_all)
-
-        ont.submissions.each do |sub|
-          sub.bring_remaining
-        end
-      else
-        ont.bring(submissions: OntologySubmission.goo_attrs_to_load(includes_param))
-      end
-      reply ont.submissions.sort {|a,b| b.submissionId.to_i <=> a.submissionId.to_i }  # descending order of submissionId
+      options = {
+        also_include_views: params["also_include_views"],
+        status: (params["include_status"] || "ANY"),
+        ontology: params["acronym"]
+      }
+      subs = retrieve_submissions(options)
+
+      reply subs.sort {|a,b| b.submissionId.to_i <=> a.submissionId.to_i }  # descending order of submissionId
     end
 
     ##
@@ -58,7 +55,7 @@ class OntologySubmissionsController < ApplicationController
       ont.bring(:submissions)
       ont_submission = ont.submission(params["ontology_submission_id"])
       error 404, "`submissionId` not found" if ont_submission.nil?
-      ont_submission.bring(*OntologySubmission.goo_attrs_to_load(includes_param))
+      ont_submission.bring(*submission_include_params)
       reply ont_submission
     end
 

helpers/application_helper.rb

@@ -363,8 +363,6 @@ def retrieve_latest_submissions(options = {})
 
         latest_submissions = page? ? submissions : {} # latest_submission doest not work with pagination
         submissions.each do |sub|
-          # To retrieve all metadata, but slow when a lot of ontologies
-          sub.bring_remaining   if includes_param.first == :all
           unless page?
             next if include_ready?(options) && !sub.ready?
             next if sub.ontology.nil?

helpers/submission_helper.rb

@@ -3,6 +3,18 @@
 module Sinatra
   module Helpers
     module SubmissionHelper
+      def submission_include_params
+        # When asking to display all metadata, we are using bring_remaining on each submission. Slower but best way to retrieve all attrs
+        includes = OntologySubmission.goo_attrs_to_load(includes_param)
+        if includes.find{|v| v.is_a?(Hash) && v.keys.include?(:ontology)}
+          includes << {:ontology=>[:administeredBy, :acronym, :name, :viewingRestriction, :group, :hasDomain,:notes, :reviews, :projects,:acl, :viewOf]}
+        end
+
+        if includes.find{|v| v.is_a?(Hash) && v.keys.include?(:contact)}
+          includes << {:contact=>[:name, :email]}
+        end
+        includes
+      end
 
       def submission_attributes_all
         out = [LinkedData::Models::OntologySubmission.embed_values_hash]
@@ -16,14 +28,17 @@ def submission_attributes_all
       def retrieve_submissions(options)
         status = (options[:status] || "RDF").to_s.upcase
         status = "RDF" if status.eql?("READY")
+        ontology_acronym = options[:ontology]
         any = status.eql?("ANY")
         include_views = options[:also_include_views] || false
         includes, page, size, order_by, _ = settings_params(LinkedData::Models::OntologySubmission)
         includes << :submissionStatus unless includes.include?(:submissionStatus)
 
         submissions_query = LinkedData::Models::OntologySubmission
+        submissions_query = submissions_query.where(ontology: [acronym: ontology_acronym]) if ontology_acronym
+
         if any
-          submissions_query = submissions_query.where
+          submissions_query = submissions_query.where unless ontology_acronym
         else
           submissions_query = submissions_query.where({ submissionStatus: [code: status] })
         end
@@ -32,24 +47,8 @@ def retrieve_submissions(options)
         submissions_query = submissions_query.filter(Goo::Filter.new(ontology: [:viewOf]).unbound) unless include_views
         submissions_query = submissions_query.filter(filter) if filter?
 
-        # When asking to display all metadata, we are using bring_remaining on each submission. Slower but best way to retrieve all attrs
-        if includes_param.first == :all
-          includes = [:submissionId, { :contact => [:name, :email],
-                                       :ontology => [:administeredBy, :acronym, :name, :summaryOnly, :ontologyType, :viewingRestriction, :acl,
-                                                     :group, :hasDomain, :views, :viewOf, :flat, :notes, :reviews, :projects],
-                                       :submissionStatus => [:code], :hasOntologyLanguage => [:acronym], :metrics => [:classes, :individuals, :properties] },
-                      :submissionStatus]
-        else
-          if includes.find { |v| v.is_a?(Hash) && v.keys.include?(:ontology) }
-            includes << { :ontology => [:administeredBy, :acronym, :name, :viewingRestriction, :group, :hasDomain, :notes, :reviews, :projects, :acl, :viewOf] }
-          end
-
-          if includes.find { |v| v.is_a?(Hash) && v.keys.include?(:contact) }
-            includes << { :contact => [:name, :email] }
-          end
-        end
 
-        submissions = submissions_query.include(includes)
+        submissions = submissions_query.include(submission_include_params)
         if page?
           submissions.page(page, size).all
         else

test/controllers/test_ontology_submissions_controller.rb

@@ -39,6 +39,12 @@ def self._create_onts
     ont.save
   end
 
+  def setup
+    delete_ontologies_and_submissions
+    ont = Ontology.new(acronym: @@acronym, name: @@name, administeredBy: [@@user])
+    ont.save
+  end
+
   def test_submissions_for_given_ontology
     num_onts_created, created_ont_acronyms = create_ontologies_and_submissions(ont_count: 1)
     ontology = created_ont_acronyms.first
@@ -196,7 +202,7 @@ def test_download_acl_only
   end
 
   def test_submissions_pagination
-    num_onts_created, created_ont_acronyms = create_ontologies_and_submissions(ont_count: 2, submission_count: 2)
+    num_onts_created, created_ont_acronyms, ontologies = create_ontologies_and_submissions(ont_count: 2, submission_count: 2)
 
     get "/submissions"
     assert last_response.ok?
@@ -210,4 +216,114 @@ def test_submissions_pagination
     submissions = MultiJson.load(last_response.body)
     assert_equal 1, submissions["collection"].length
   end
+
+
+  def test_submissions_default_includes
+    ontology_count = 5
+    num_onts_created, created_ont_acronyms, ontologies = create_ontologies_and_submissions(ont_count: ontology_count, submission_count: 1, submissions_to_process: [])
+
+    submission_default_attributes = LinkedData::Models::OntologySubmission.hypermedia_settings[:serialize_default].map(&:to_s)
+
+    get("/submissions?display_links=false&display_context=false&include_status=ANY")
+    assert last_response.ok?
+    submissions = MultiJson.load(last_response.body)
+
+    assert_equal ontology_count, submissions.size
+    assert(submissions.all? { |sub| submission_default_attributes.eql?(submission_keys(sub)) })
+
+    get("/ontologies/#{created_ont_acronyms.first}/submissions?display_links=false&display_context=false")
+
+    assert last_response.ok?
+    submissions = MultiJson.load(last_response.body)
+    assert_equal 1, submissions.size
+    assert(submissions.all? { |sub| submission_default_attributes.eql?(submission_keys(sub)) })
+  end
+
+  def test_submissions_all_includes
+    ontology_count = 5
+    num_onts_created, created_ont_acronyms, ontologies = create_ontologies_and_submissions(ont_count: ontology_count, submission_count: 1, submissions_to_process: [])
+    def submission_all_attributes
+      attrs = OntologySubmission.goo_attrs_to_load([:all])
+      embed_attrs = attrs.select { |x| x.is_a?(Hash) }.first
+
+      attrs.delete_if { |x| x.is_a?(Hash) }.map(&:to_s) + embed_attrs.keys.map(&:to_s)
+    end
+    get("/submissions?include=all&display_links=false&display_context=false")
+
+    assert last_response.ok?
+    submissions = MultiJson.load(last_response.body)
+    assert_equal ontology_count, submissions.size
+
+    assert(submissions.all? { |sub| submission_all_attributes.sort.eql?(submission_keys(sub).sort) })
+    assert(submissions.all? { |sub| sub["contact"] && (sub["contact"].first.nil? || sub["contact"].first.keys.eql?(%w[name email id])) })
+
+    get("/ontologies/#{created_ont_acronyms.first}/submissions?include=all&display_links=false&display_context=false")
+
+    assert last_response.ok?
+    submissions = MultiJson.load(last_response.body)
+    assert_equal 1, submissions.size
+
+    assert(submissions.all? { |sub| submission_all_attributes.sort.eql?(submission_keys(sub).sort) })
+    assert(submissions.all? { |sub| sub["contact"] && (sub["contact"].first.nil? || sub["contact"].first.keys.eql?(%w[name email id])) })
+
+    get("/ontologies/#{created_ont_acronyms.first}/latest_submission?include=all&display_links=false&display_context=false")
+    assert last_response.ok?
+    sub = MultiJson.load(last_response.body)
+
+    assert(submission_all_attributes.sort.eql?(submission_keys(sub).sort))
+    assert(sub["contact"] && (sub["contact"].first.nil? || sub["contact"].first.keys.eql?(%w[name email id])))
+
+    get("/ontologies/#{created_ont_acronyms.first}/submissions/1?include=all&display_links=false&display_context=false")
+    assert last_response.ok?
+    sub = MultiJson.load(last_response.body)
+
+    assert(submission_all_attributes.sort.eql?(submission_keys(sub).sort))
+    assert(sub["contact"] && (sub["contact"].first.nil? || sub["contact"].first.keys.eql?(%w[name email id])))
+  end
+
+  def test_submissions_custom_includes
+    ontology_count = 5
+    num_onts_created, created_ont_acronyms, ontologies = create_ontologies_and_submissions(ont_count: ontology_count, submission_count: 1, submissions_to_process: [])
+    include = 'ontology,contact,submissionId'
+
+    get("/submissions?include=#{include}&display_links=false&display_context=false")
+
+    assert last_response.ok?
+    submissions = MultiJson.load(last_response.body)
+    assert_equal ontology_count, submissions.size
+    assert(submissions.all? { |sub| include.split(',').eql?(submission_keys(sub)) })
+    assert(submissions.all? { |sub| sub["contact"] && (sub["contact"].first.nil? || sub["contact"].first.keys.eql?(%w[name email id])) })
+
+    get("/ontologies/#{created_ont_acronyms.first}/submissions?include=#{include}&display_links=false&display_context=false")
+
+    assert last_response.ok?
+    submissions = MultiJson.load(last_response.body)
+    assert_equal 1, submissions.size
+    assert(submissions.all? { |sub| include.split(',').eql?(submission_keys(sub)) })
+    assert(submissions.all? { |sub| sub["contact"] && (sub["contact"].first.nil? || sub["contact"].first.keys.eql?(%w[name email id])) })
+
+    get("/ontologies/#{created_ont_acronyms.first}/latest_submission?include=#{include}&display_links=false&display_context=false")
+    assert last_response.ok?
+    sub = MultiJson.load(last_response.body)
+    assert(include.split(',').eql?(submission_keys(sub)))
+    assert(sub["contact"] && (sub["contact"].first.nil? || sub["contact"].first.keys.eql?(%w[name email id])))
+
+    get("/ontologies/#{created_ont_acronyms.first}/submissions/1?include=#{include}&display_links=false&display_context=false")
+    assert last_response.ok?
+    sub = MultiJson.load(last_response.body)
+    assert(include.split(',').eql?(submission_keys(sub)))
+    assert(sub["contact"] && (sub["contact"].first.nil? || sub["contact"].first.keys.eql?(%w[name email id])))
+  end
+
+  def test_submissions_param_include
+    skip('only for local development regrouping a set of tests')
+    test_submissions_default_includes
+    test_submissions_all_includes
+    test_submissions_custom_includes
+  end
+
+  private
+  def submission_keys(sub)
+    sub.to_hash.keys - %w[@id @type id]
+  end
 end