ncbo / ontologies_api

Hypermedia API for NCBO's ontology-related projects
http://data.bioontology.org
Other
25 stars 10 forks source link

Feature: Add the slices CRUD endpoints #87

Closed syphax-bouazzouni closed 10 months ago

syphax-bouazzouni commented 2 years ago

Forked from #19

This PR adds the following endpoints :

Our long-term vision behind slices can be found here; In brief gives the possibility the users to create their slices, with their ontologies, colors, and logo, .... Making Ontoportal a sort a Portal provider for people that don't want or don't have the resources to deploy there own instances (and with portal federation, everything will be also connected).

The biggest blocker here, about the slice features is that we can't can't dynamically create a DSN configuration for each of the slices created by a user.

alexskr commented 1 year ago

should slice creation/update be restricted to users with administrative privileges?

alexskr commented 1 year ago

The biggest blocker here, about the slice features is that we can't can't dynamically create a DSN configuration for each of the slices created by a user.

it can be done with wildcard subdomains.

syphax-bouazzouni commented 1 year ago

Should slice creation/update be restricted to users with administrative privileges? Now that I have checked again my own code (a year) later, it is true that with this version anyone can create and delete a slice there is no ownership in the current model.

For now, I added a security check on each of the slice write operations. That prevents no admin users from editing them.

 error 403, "Access denied" unless current_user && current_user.admin?
alexskr commented 12 months ago

@syphax-bouazzouni, thanks for adding security check. Would you please add necessary unit tests for the new functionality.

alexskr commented 12 months ago

it would also be a good idea to restrict the use of underscores (_) in the slice names.
Underscores are generally allowed in subdomain names according to the DNS specification. However, it's important to note that while they are technically allowed, they may not be supported or accepted by all DNS servers, web browsers, software applications and certificate authorities.

syphax-bouazzouni commented 10 months ago

@syphax-bouazzouni, thanks for adding security check. Would you please add necessary unit tests for the new functionality.

Added here https://github.com/ncbo/ontologies_api/pull/87/commits/6dc238dd96b0da8b035737a0f173a63de3f6c289

it would also be a good idea to restrict the use of underscores (_) in the slice names. Underscores are generally allowed in subdomain names according to the DNS specification. However, it's important to note that while they are technically allowed, they may not be supported or accepted by all DNS servers, web browsers, software applications and certificate authorities.

Added https://github.com/ncbo/ontologies_linked_data/pull/176

codecov-commenter commented 10 months ago

Codecov Report

Merging #87 (8489f2b) into master (9743042) will increase coverage by 0.01%. Report is 49 commits behind head on master. The diff coverage is 69.76%.

@@            Coverage Diff             @@
##           master      #87      +/-   ##
==========================================
+ Coverage   71.84%   71.85%   +0.01%     
==========================================
  Files          52       52              
  Lines        2845     2878      +33     
==========================================
+ Hits         2044     2068      +24     
- Misses        801      810       +9     
Flag Coverage Δ
unittests 71.85% <69.76%> (+0.01%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
controllers/metrics_controller.rb 58.69% <100.00%> (-1.73%) :arrow_down:
controllers/ontology_analytics_controller.rb 100.00% <100.00%> (ø)
controllers/ontology_submissions_controller.rb 73.07% <100.00%> (+3.17%) :arrow_up:
helpers/access_control_helper.rb 83.33% <ø> (-1.29%) :arrow_down:
helpers/application_helper.rb 90.33% <100.00%> (+0.42%) :arrow_up:
controllers/slices_controller.rb 66.66% <59.37%> (-33.34%) :arrow_down:
alexskr commented 10 months ago

@syphax-bouazzouni, Looks great!

mdorf commented 10 months ago

@syphax-bouazzouni, we want to merge this PR, but it contains a dependency on a specific branch of 'ontoportal-lirmm/ontologies_linked_data':

gem 'ontologies_linked_data', github: 'ontoportal-lirmm/ontologies_linked_data', branch: 'pr/feature/extend-slice-acronym-validator' 

We definitely don't want to add this dependency to our codebase. This PR doesn't make it clear what functionality/code exactly it requires from that 'ontoportal-lirmm/ontologies_linked_data' branch. Is there another PR that contains that functionality?

mdorf commented 10 months ago

If we merge the PR ncbo/ontologies_linked_data#176, can we then remove the dependency on ontoportal-lirmm, or are there other changes that need to be merged first?

syphax-bouazzouni commented 10 months ago

If we merge the PR ncbo/ontologies_linked_data#176, can we then remove the dependency on ontoportal-lirmm, or are there other changes that need to be merged first?

Yes, this change in the Gemfile, was just to pass the tests.