ideaconsult / Toxtree.js

http://ideaconsult.github.io/Toxtree.js/
2 stars 3 forks source link

errors when deployed as Tomcat root application #183

Open vedina opened 8 years ago

vedina commented 8 years ago

If the application runs in ToMCat root context, i.e. http://localhost:8080/ instead of http://localhost:8080//ambit2 it seems not all queries in jToxKit works correctly.

For example the Search substances tab in RA does not show substances and tries to send query http://localhost:8080/ui/substance? instead of http://localhost:8080/substance?

jquery-1.10.2.js:8706 GET http://localhost:8080/ui/substance?type=related&addDummySubstance=true&comp…1&bundle_uri=http%3A%2F%2Flocalhost%3A8080%2Fbundle%2F1&page=0&pagesize=10 405 (Method Not Allowed)
send @ jquery-1.10.2.js:8706
jQuery.extend.ajax @ jquery-1.10.2.js:8136window.jT.window.jToxKit.call @ jtoxkit.js:735
cls.queryEntries @ jtoxkit.js:3337
cls.querySubstance @ jtoxkit.js:3371
cls @ jtoxkit.js:3272preDetailedRow @ ui-matrix.js:1932ccLib.fireCallback @ jtoxkit.js:61fnShowDetails @ jtoxkit.js:2239(anonymous function) @ jtoxkit.js:2346jQuery.event.dispatch @ jquery-1.10.2.js:5095elemData.handle @ jquery-1.10.2.js:4766
jtoxkit.js:4430 Error [6]: http://localhost:8080/ui/substance?type=related&addDummySubstance=true&comp…1&bundle_uri=http%3A%2F%2Flocalhost%3A8080%2Fbundle%2F1&page=0&pagesize=10
jtoxkit.js:3344 Uncaught TypeError: Cannot read property 'length' of undefined

Examples (see assessment id tab, collect structures tab, search substances tab @thejonan @gonzomir

vedina commented 8 years ago

Another example

thejonan commented 8 years ago

I'll check this one - I remember expecting certain path components in the queries.

thejonan commented 8 years ago

I've investigated the case and indeed, in the very core of jToxKit, deducing a baseUrl from a full URL expects one (static) component. But the solution is not that simple, because we don't have strict policy about URLs. Here's the explanation:

When I plan to do is this: Add a new data-simple-base setting, that will instruct the jToxKit to not try to look for additional component while automatically deducing baseUrl. Meanwhile, let me know if any of you have a better idea, cause I don't really like this one.

vedina commented 8 years ago

Wouldn't it be better not to deduce base url (and assuming particular structure of the path), but specifying the base URL explicitly.

Not sure why querySubstance needs to overwrite baseURL. It uses normally the same server. Let's discuss this tomorrow.

thejonan commented 8 years ago

If a full URL is passed, it is always used, but if, for example, a URL for substances is given, internally additional queries for composition and studies are made and they need to use some baseURL - it is the one deduced from the given substanceUri.

thejonan commented 8 years ago

Hm. I've come up to a better solution... I just need to implement it. Yes, you're right that baseUrl approach in this absolute manner is wrong. It should be relative, i.e. - treat as base the part of URL that is before some keyword, which is specific for the kit.

thejonan commented 8 years ago

This one should be fixes with the last master commit: 88c6e7644acc8e2a7c9b7cfabcd2463a478f67bc. However, baseUrl policies should be refined, because in some kits - like jToxCompound, for example - it is not really known what is the "normal" URL for that type of data, i.e. - what is the keyword, and so there is no way to determine what should be considered "base". In the very same kit, for example baseUrl is used to fetch some images.

vedina commented 8 years ago

Some q/ comments before testing:

vedina commented 8 years ago

btw, don't use http://apps.ideaconsult.net:8080/ , use https://apps.ideaconsult.net/ instead

vedina commented 8 years ago

I'm afraid this is not yet solved. Please test and tell what are the configuration for these.

The commit log don't show anything changed in the read across files. Using the newest source on the trunk and adding data-base-url= to ra-matrix and ra-report query kit solves the problem for showing substances, but not for endpoints and users. The pages still issue queries prefixed with 'http://root/ui/" , where it should be "http://root/" only. Guess ui comes from the current url.

It could be simple kit configuration that I am missing. @gonzomir could help with testing RA pages.

@thejonan, please don't merge with the master. We have a little bit more complicated procedure since the last year, i.e. fork the repo and when ready do a pull request. For the facet-kit branch proceed with the branch, but for new things, including this one, pls go through a fork.

gonzomir commented 8 years ago

I tested the examples from above with a baseUrl, supplied as a GET parameter in the URL and everything works, so it should work when baseUrl is supplied in any other way (data- attribute or directly in settings) - it works locally too. But, I was thinking, wouldn't it be better if baseUrl is derived from bundleUri instead of UI's URL?

One more thing, on data.enanomapper.net the requests for users lists return uri-list instead of json.

gonzomir commented 8 years ago

OK, found the problem with endpoints. When initializing other kits in ui-matrix.js, we always pass the settings from the parent kit, but we don't do it when initializing jToxEndpoint. And this wouldn't be a problem if we passed absolute URL to the loadEndpoints method. But we pass relative URL there and things break.

gonzomir commented 8 years ago

Yep, the same with the POST to /bundle. Pull request #184 should fix these.

vedina commented 8 years ago

Thanks, will try in a minute. Indeed, the baseURL should not be derived from the UI's url, it should be an external setting, not derived. But we are likely coming to a happy end here :)

data.enanomapper.net uses different user management strategy , i.e. OpenTox AA and the user facilities used for RA are not really implemented (would need different backend implementation), though it would be good to work in consistent manner.

vedina commented 8 years ago

Added data-base-url="${ambit_root}" to all data-kit widgets in ui-matrix.ftl and ui-report.ftl. Confirmed this fixes the read across tabs issues. Let me know if there's anything wrong with this. Should we update uses/ui-matrix.html and uses/ui-report.html as well?

@thejonan or @gonzomir - can you look up the images issue search page, export tab

vedina commented 8 years ago

All working now, except for images.

vedina commented 8 years ago

Any news here for images?

vedina commented 8 years ago

Thanks, all fixed now.

vedina commented 8 years ago

Sorry, we need similar fix for images links in the read across tabs

image

and the dataset component export tab

image

gonzomir commented 8 years ago

I think setting the default baseUrl is flawed, it has two major issues:

  1. jToxKit assumes that the baseUrl always includes one element in it's path. This means that if the UI is located in http://example.com/ui.html (the path has one element - ui.html), the baseUrl will also be http://example.com/ui.html.
  2. The only way to be set is through a GET parameter. In jtoxkit.js lines 130 to 138 if a GET param for baseUrl isn't provided, it's derived from current document.location and later overwrites any other baseUrl, provided in other ways.

I think that not using document.location to add a default query param value should allow setting it in other ways. This will allow us to fix ui-matrix.js to derive baseUrl from bundleUri when the later is provided (as a default behavior), and to be explicitly set in configuration or by adding data-baseurl attribute.

vedina commented 8 years ago

See my comment https://github.com/ideaconsult/Toxtree.js/issues/183#issuecomment-205045296 - the base URL SHOULD NOT be deduced at all. It should be set. The server knows well which is the base URL and sets it through a Freemarker variable directly in the html.

thejonan commented 8 years ago

Good scanning! :-) Yes, that was not correct indeed.

In the latest commit (it is intentionally on another branch, which happens after the submodule fixes...), that place is fixed. More importantly the console logging upon deduction is also left in the code, so please - give it a try, and take a look at the console for lines like this:

Deduced base URL: ...

One we're sure we don't have such lines, or they produce whatever is expected - I'll remove the logging from the code.

thejonan commented 8 years ago

What is the final stage of this issue?