From c6897f86dad4f7b089eadef9d6bf6f9af780e393 Mon Sep 17 00:00:00 2001 From: Jennifer Vendetti Date: Thu, 22 Aug 2024 14:15:28 -0700 Subject: [PATCH 1/6] Add include all param where necessary Recent change to the find_by_acronym method in ontologies_api_client result in a lesser set of attributes returned from the triplestore. Also disable cache_refresh_all on updates. --- app/controllers/ontologies_controller.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/app/controllers/ontologies_controller.rb b/app/controllers/ontologies_controller.rb index d61b1bfa59..4c0847c5f1 100644 --- a/app/controllers/ontologies_controller.rb +++ b/app/controllers/ontologies_controller.rb @@ -188,7 +188,7 @@ def create def edit # Note: find_by_acronym includes ontology views - @ontology = LinkedData::Client::Models::Ontology.find_by_acronym(params[:id]).first + @ontology = LinkedData::Client::Models::Ontology.find_by_acronym(params[:id], include: 'all').first redirect_to_home unless session[:user] && @ontology.administeredBy.include?(session[:user].id) || session[:user].admin? @categories = LinkedData::Client::Models::Category.all @user_select_list = LinkedData::Client::Models::User.all.map {|u| [u.username, u.id]} @@ -254,9 +254,9 @@ def show end # Note: find_by_acronym includes ontology views - @ontology = LinkedData::Client::Models::Ontology.find_by_acronym(params[:ontology]).first + @ontology = LinkedData::Client::Models::Ontology.find_by_acronym(params[:ontology], include: 'all').first not_found if @ontology.nil? - + # Handle the case where an ontology is converted to summary only. # See: https://github.com/ncbo/bioportal_web_ui/issues/133. if @ontology.summaryOnly && params[:p].present? @@ -313,7 +313,7 @@ def show end def submit_success - @ontology = LinkedData::Client::Models::Ontology.find_by_acronym(params[:id]).first + @ontology = LinkedData::Client::Models::Ontology.find_by_acronym(params[:id], include: 'all').first render 'submit_success' end @@ -345,9 +345,9 @@ def update return end # Note: find_by_acronym includes ontology views - @ontology = LinkedData::Client::Models::Ontology.find_by_acronym(params[:ontology][:acronym] || params[:id]).first + @ontology = LinkedData::Client::Models::Ontology.find_by_acronym(params[:id]).first @ontology.update_from_params(ontology_params) - error_response = @ontology.update + error_response = @ontology.update(cache_refresh_all: false) if response_error?(error_response) @categories = LinkedData::Client::Models::Category.all @user_select_list = LinkedData::Client::Models::User.all.map {|u| [u.username, u.id]} From 59ffa48ef6067b3ee55d5fbeefa5319ba86b90c9 Mon Sep 17 00:00:00 2001 From: Jennifer Vendetti Date: Thu, 22 Aug 2024 14:19:54 -0700 Subject: [PATCH 2/6] Replace find_by_acronym with more performant get Also refactor the update method to prevent an unnecessary fetch of all users and ontologies on successful updates. --- app/controllers/projects_controller.rb | 45 ++++++-------------------- 1 file changed, 10 insertions(+), 35 deletions(-) diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 7de8f345f7..973fa9f641 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -17,18 +17,11 @@ def index end def show - projects = LinkedData::Client::Models::Project.find_by_acronym(params[:id]) - if projects.blank? - flash.alert = "Project not found: #{params[:id]}" - redirect_to projects_path - return - end - - @project = projects.first + @project = LinkedData::Client::Models::Project.get(params[:id]) @ontologies_used = [] onts_used = @project.ontologyUsed onts_used.each do |ont_used| - ont = LinkedData::Client::Models::Ontology.find(ont_used) + ont = LinkedData::Client::Models::Ontology.get(ont_used, include: 'name,acronym') @ontologies_used << Hash['name', ont.name, 'acronym', ont.acronym] unless ont.nil? end @ontologies_used.sort_by! { |o| o['name'].downcase } @@ -45,13 +38,7 @@ def new end def edit - projects = LinkedData::Client::Models::Project.find_by_acronym(params[:id]) - if projects.blank? - flash.alert = "Project not found: #{params[:id]}" - redirect_to projects_path - return - end - @project = projects.first + @project = LinkedData::Client::Models::Project.get(params[:id]) @user_select_list = LinkedData::Client::Models::User.all.map { |u| [u.username, u.id] } @user_select_list.sort! { |a, b| a[1].downcase <=> b[1].downcase } @usedOntologies = @project.ontologyUsed || [] @@ -84,21 +71,15 @@ def create end def update - projects = LinkedData::Client::Models::Project.find_by_acronym(params[:id]) - if projects.blank? - flash.alert = "Project not found: #{params[:id]}" - redirect_to projects_path - return - end - @project = projects.first + @project = LinkedData::Client::Models::Project.get(params[:id]) @project.update_from_params(project_params) - @user_select_list = LinkedData::Client::Models::User.all.map { |u| [u.username, u.id] } - @user_select_list.sort! { |a, b| a[1].downcase <=> b[1].downcase } - @usedOntologies = @project.ontologyUsed || [] - @ontologies = LinkedData::Client::Models::Ontology.all - error_response = @project.update + error_response = @project.update(cache_refresh_all: false) if response_error?(error_response) @errors = response_errors(error_response) + @user_select_list = LinkedData::Client::Models::User.all.map { |u| [u.username, u.id] } + @user_select_list.sort! { |a, b| a[1].downcase <=> b[1].downcase } + @usedOntologies = @project.ontologyUsed || [] + @ontologies = LinkedData::Client::Models::Ontology.all render :edit else flash[:notice] = 'Project successfully updated' @@ -107,13 +88,7 @@ def update end def destroy - projects = LinkedData::Client::Models::Project.find_by_acronym(params[:id]) - if projects.blank? - flash.alert = "Project not found: #{params[:id]}" - redirect_to projects_path - return - end - @project = projects.first + @project = LinkedData::Client::Models::Project.get(params[:id]) error_response = @project.delete if response_error?(error_response) @errors = response_errors(error_response) From ca3d6dda93070d5069a3648dceb99fc62252f125 Mon Sep 17 00:00:00 2001 From: Jennifer Vendetti Date: Thu, 22 Aug 2024 14:22:18 -0700 Subject: [PATCH 3/6] Simplify code for fetching an ontology --- app/controllers/submissions_controller.rb | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/app/controllers/submissions_controller.rb b/app/controllers/submissions_controller.rb index 042c3e4365..9280fdfb6f 100644 --- a/app/controllers/submissions_controller.rb +++ b/app/controllers/submissions_controller.rb @@ -5,15 +5,8 @@ class SubmissionsController < ApplicationController before_action :authorize_and_redirect, only: [:edit, :update, :create, :new] def new - begin - # REVIEW: do we really need this double attempt to locate an ontology? I think find_by_acronym (below) should - # be sufficient. It's not evident that we call the new method with a full URI anymore. - @ontology = LinkedData::Client::Models::Ontology.get(CGI.unescape(params[:ontology_id])) - rescue MultiJson::ParseError - nil - end - - @ontology ||= LinkedData::Client::Models::Ontology.find_by_acronym(params[:ontology_id]).first + # NOTE: find_by_acronym includes ontology views + @ontology = LinkedData::Client::Models::Ontology.find_by_acronym(params[:ontology_id]).first @submission = @ontology.explore.latest_submission @submission ||= LinkedData::Client::Models::OntologySubmission.new end From c65fc5a38beb6e79390d7670b53d6e1da2038026 Mon Sep 17 00:00:00 2001 From: Jennifer Vendetti Date: Thu, 22 Aug 2024 14:24:15 -0700 Subject: [PATCH 4/6] Replace find with more performant get Also disable cache_refresh_all on updates, and redirect to the user show page after successful creates. Redirecting to the (slow to load) ontology browse page isn't a good user experience. --- app/controllers/users_controller.rb | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 5d54443047..531b4fd6ff 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -16,11 +16,7 @@ def index end def show - @user = if session[:user].admin? && params.has_key?(:id) - LinkedData::Client::Models::User.find_by_username(params[:id]).first - else - LinkedData::Client::Models::User.find(session[:user].id) - end + @user = LinkedData::Client::Models::User.get(params[:id], include: 'all') @all_ontologies = LinkedData::Client::Models::Ontology.all(ignore_custom_ontologies: true) @user_ontologies = @user.customOntology @@ -41,8 +37,7 @@ def new end def edit - @user = LinkedData::Client::Models::User.find(params[:id]) - @user ||= LinkedData::Client::Models::User.find_by_username(params[:id]).first + @user = LinkedData::Client::Models::User.get(params[:id], include: 'all') end def create @@ -58,7 +53,7 @@ def create else flash[:notice] = 'Account was successfully created' session[:user] = LinkedData::Client::Models::User.authenticate(@user.username, @user.password) - redirect_to_browse + redirect_to user_path(@user.username) end else render 'new' @@ -66,8 +61,7 @@ def create end def update - @user = LinkedData::Client::Models::User.find(params[:id]) - @user = LinkedData::Client::Models::User.find_by_username(params[:id]).first if @user.nil? + @user = LinkedData::Client::Models::User.get(params[:id], include: 'all') @errors = validate_update(user_params) if @errors.empty? @@ -78,7 +72,7 @@ def update end @user.update_from_params(user_params.merge!(role: user_roles)) - error_response = @user.update + error_response = @user.update(cache_refresh_all: false) if response_error?(error_response) @errors = response_errors(error_response) @@ -99,12 +93,10 @@ def update def destroy response = { errors: String.new(''), success: String.new('') } - @user = LinkedData::Client::Models::User.find(params[:id]) - @user = LinkedData::Client::Models::User.find_by_username(params[:id]).first if @user.nil? + @user = LinkedData::Client::Models::User.get(params[:id]) if session[:user].admin? @user.delete response[:success] << 'User deleted successfully ' - else response[:errors] << 'Not permitted ' end From 4f071d6da52f54429a6c317f6ae0f38ad314c310 Mon Sep 17 00:00:00 2001 From: Jennifer Vendetti Date: Thu, 22 Aug 2024 14:25:14 -0700 Subject: [PATCH 5/6] Don't make REST calls to get a username from an ID --- app/helpers/application_helper.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 4ad4a0de29..95d08d8ff7 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -38,9 +38,7 @@ def clean_id(string) end def get_username(user_id) - user = LinkedData::Client::Models::User.find(user_id) - username = user.nil? ? user_id : user.username - username + user_id.split('/').last end def current_user_admin? From 7cbdb87e62b76dc7dae278e54b59252623602b64 Mon Sep 17 00:00:00 2001 From: Jennifer Vendetti Date: Wed, 4 Sep 2024 11:47:53 -0700 Subject: [PATCH 6/6] Bump ontologies_api_client to v2.4.0 tag --- Gemfile | 2 +- Gemfile.lock | 8 +++----- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/Gemfile b/Gemfile index a913445ceb..215fecf35e 100644 --- a/Gemfile +++ b/Gemfile @@ -57,7 +57,7 @@ gem 'iso-639', '~> 0.3.6' gem 'multi_json' gem 'mysql2', '0.5.5' gem 'oj' -gem 'ontologies_api_client', github: 'ncbo/ontologies_api_ruby_client', tag: 'v2.3.0' +gem 'ontologies_api_client', github: 'ncbo/ontologies_api_ruby_client', tag: 'v2.4.0' gem 'open_uri_redirections' gem 'pry' gem 'psych', '< 4' diff --git a/Gemfile.lock b/Gemfile.lock index 35cd0a8a87..bcd80c4489 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,9 +1,9 @@ GIT remote: https://github.com/ncbo/ontologies_api_ruby_client.git - revision: 027c749f5de3f644b0392197e9482be51738cbe8 - tag: v2.3.0 + revision: f589b13dfbbc133ea67cbae1a8f92b41ea85c14b + tag: v2.4.0 specs: - ontologies_api_client (2.2.5) + ontologies_api_client (2.4.0) activesupport (= 7.0.8) addressable (~> 2.8) excon @@ -13,7 +13,6 @@ GIT lz4-ruby multi_json oj - spawnling (= 2.1.5) GEM remote: https://rubygems.org/ @@ -352,7 +351,6 @@ GEM tilt select2-rails (4.0.13) sexp_processor (4.17.0) - spawnling (2.1.5) sprockets (4.2.1) concurrent-ruby (~> 1.0) rack (>= 2.2.4, < 4)