deprecate LEGACY_REST_URL variable

alexskr commented 2 years ago

According to the sample config file LEGACY_REST_URL is 'Legacy REST core service address (BioPortal v3.x and lower)' BioPortal v3.x rest service was deprecated a while ago but this variable still in use: https://github.com/ncbo/bioportal_web_ui/blob/b33ca410d8922beb1c51ccb373262e9563d03166/app/controllers/ajax_proxy_controller.rb#L28 https://github.com/ncbo/bioportal_web_ui/blob/6f1fb336eb27e57c444750c6c187bf23cc60a3b8/lib/log.rb#L30

syphax-bouazzouni commented 2 years ago

In addition, there are 3 more places where it's used, see the screenshot below.

alexskr commented 2 years ago

flexviz is another deprecated adobe flash based visualization which needs to be purged as well

alexskr commented 2 years ago

can we remove these files?

https://github.com/ncbo/bioportal_web_ui/blob/6f1fb336eb27e57c444750c6c187bf23cc60a3b8/app/views/concepts/_software.html.erb https://github.com/ncbo/bioportal_web_ui/blob/6f1fb336eb27e57c444750c6c187bf23cc60a3b8/app/views/concepts/_images.rhtmlold https://github.com/ncbo/bioportal_web_ui/blob/6f1fb336eb27e57c444750c6c187bf23cc60a3b8/app/views/concepts/_flexviz.html.erb https://github.com/ncbo/bioportal_web_ui/blob/6f1fb336eb27e57c444750c6c187bf23cc60a3b8/app/views/concepts/_images.html.erb

syphax-bouazzouni commented 2 years ago

in addition (two), the LEGACY_REST_URL is called in jsonp and remote methods, where they themselves are not used anywhere.

jsonp

remote

remote is called inside https://github.com/ncbo/bioportal_web_ui/blob/6f1fb336eb27e57c444750c6c187bf23cc60a3b8/lib/log.rb#L14

But will never be called because the add function is called nowhere in the code with request argument

my (personnel) conclusion

we can remove jsonp and its route
we can remove remote and its call in add
we can also remove $REMOTE_LOGGING in the sample config file, because it was only used in remote

syphax-bouazzouni commented 2 years ago

Finally, I have some time ago a list of the unused view (that you can see below), stay to list all the related unused code to these views, and have a second verification of another person.

https://github.com/ncbo/bioportal_web_ui/issues/219

alexskr commented 2 years ago

If i understand this correctly log.rb includes remote logging functionality if we set REMOTE_LOGGING. It was previously used for UI to log events to obsolete/non-existing v3 rest API. I think this can be safely removed.

jvendetti commented 2 years ago

I wasn't able to look at all 37 of the view templates that are listed as unused, but at first glance there are at least a few that are still employed by BioPortal.

layouts/angular.html.erb

This is the layout used by the ontology Browse page:

Started GET "/ontologies" for 127.0.0.1 at 2022-09-07 10:52:24 -0700
Processing by OntologiesController#index as HTML
  Rendering ontologies/browse.html.erb within layouts/angular

layouts/_ontology.html.haml

This layout is referenced inside of the virtual_show method in the notes controller:

https://github.com/ncbo/bioportal_web_ui/blob/6f1fb336eb27e57c444750c6c187bf23cc60a3b8/app/controllers/notes_controller.rb#L35

The method can be triggered by navigating to the top-level Notes tab for any ontology with notes, then clicking on any note subject:

Started GET "/ontologies/BRO/notes/https%3A%2F%2Fdata.bioontology.org%2Fnotes%2Fa5809cba-e4dd-4553-9b24-c010f9d27b50" for 127.0.0.1 at 2022-09-07 14:19:57 -0700
Processing by NotesController#virtual_show as */*
  Parameters: {"ontology"=>"BRO", "noteid"=>"https://data.bioontology.org/notes/a5809cba-e4dd-4553-9b24-c010f9d27b50"}
  Rendered notes/_thread.html.haml (Duration: 1610.9ms | Allocations: 3108739)
Completed 200 OK in 15722ms (Views: 1611.3ms | ActiveRecord: 0.0ms | Allocations: 3473884)

... though it wasn't immediately clear what would trigger the elseif and else clauses inside this method that reference the ontology layout partial.

application/_footer_appliance.html.haml

Used to render a different footer when running the application in appliance mode:

https://github.com/ncbo/bioportal_web_ui/blob/6f1fb336eb27e57c444750c6c187bf23cc60a3b8/app/views/layouts/appliance.html.haml#L35

notes/_list.html.haml

Used to render the notes associated with a particular class:

Started GET "/ajax_concepts/BRO/?conceptid=http%3A%2F%2Fbioontology.org%2Fontologies%2FResearchArea.owl%23Area_of_Research&callback=load" for 127.0.0.1 at 2022-09-07 14:00:14 -0700
Processing by ConceptsController#show as */*
  Parameters: {"conceptid"=>"http://bioontology.org/ontologies/ResearchArea.owl#Area_of_Research", "callback"=>"load", "ontology"=>"BRO"}
  Ontology Load (0.3ms)  SELECT `ontologies`.* FROM `ontologies` WHERE `ontologies`.`acronym` = 'BRO' LIMIT 1
  ↳ app/helpers/application_helper.rb:367:in `ontolobridge_instructions_template'
  Rendered concepts/_biomixer.html.erb (Duration: 0.1ms | Allocations: 42)
  Rendered concepts/_details.html.haml (Duration: 775.1ms | Allocations: 7617)
  Rendered notes/_list.html.haml (Duration: 6957.3ms | Allocations: 363722)

syphax-bouazzouni commented 2 years ago

layouts/angular.html.erb

This is the layout used by the ontology Browse page:

Started GET "/ontologies" for 127.0.0.1 at 2022-09-07 10:52:24 -0700
Processing by OntologiesController#index as HTML
  Rendering ontologies/browse.html.erb within layouts/angular

Yeah, for the angular layout indeed it is used, I don't remember why I listed it in my notes. Maybe just to remember to remove angular-js and update the browse page.

layouts/_ontology.html.haml

This layout is referenced inside of the virtual_show method in the notes controller:

https://github.com/ncbo/bioportal_web_ui/blob/6f1fb336eb27e57c444750c6c187bf23cc60a3b8/app/controllers/notes_controller.rb#L35

The method can be triggered by navigating to the top-level Notes tab for any ontology with notes, then clicking on any note subject:
Started GET "/ontologies/BRO/notes/https%3A%2F%2Fdata.bioontology.org%2Fnotes%2Fa5809cba-e4dd-4553-9b24-c010f9d27b50" for 127.0.0.1 at 2022-09-07 14:19:57 -0700
Processing by NotesController#virtual_show as */*
  Parameters: {"ontology"=>"BRO", "noteid"=>"https://data.bioontology.org/notes/a5809cba-e4dd-4553-9b24-c010f9d27b50"}
  Rendered notes/_thread.html.haml (Duration: 1610.9ms | Allocations: 3108739)
Completed 200 OK in 15722ms (Views: 1611.3ms | ActiveRecord: 0.0ms | Allocations: 3473884)
... though it wasn't immediately clear what would trigger the elseif and else clauses inside this method that reference the ontology layout partial.

Yes, but I think that the elseif and else clauses inside will never be fulfilled (not 100% sure)

application/_footer_appliance.html.haml

Used to render a different footer when running the application in appliance mode:

https://github.com/ncbo/bioportal_web_ui/blob/6f1fb336eb27e57c444750c6c187bf23cc60a3b8/app/views/layouts/appliance.html.haml#L35

Ah yes, i found that its only our local code that does no more is it.

notes/_list.html.haml

Used to render the notes associated with a particular class:

Started GET "/ajax_concepts/BRO/?conceptid=http%3A%2F%2Fbioontology.org%2Fontologies%2FResearchArea.owl%23Area_of_Research&callback=load" for 127.0.0.1 at 2022-09-07 14:00:14 -0700
Processing by ConceptsController#show as */*
  Parameters: {"conceptid"=>"http://bioontology.org/ontologies/ResearchArea.owl#Area_of_Research", "callback"=>"load", "ontology"=>"BRO"}
  Ontology Load (0.3ms)  SELECT `ontologies`.* FROM `ontologies` WHERE `ontologies`.`acronym` = 'BRO' LIMIT 1
  ↳ app/helpers/application_helper.rb:367:in `ontolobridge_instructions_template'
  Rendered concepts/_biomixer.html.erb (Duration: 0.1ms | Allocations: 42)
  Rendered concepts/_details.html.haml (Duration: 775.1ms | Allocations: 7617)
  Rendered notes/_list.html.haml (Duration: 6957.3ms | Allocations: 363722)

Indeed,

I updated the list with your remarks, thanks @jvendetti.

ncbo / bioportal_web_ui