Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace hard-wired inclusions in meta-panel.controller #405

Open
brianritchie1312 opened this issue Feb 15, 2019 · 3 comments
Open

Replace hard-wired inclusions in meta-panel.controller #405

brianritchie1312 opened this issue Feb 15, 2019 · 3 comments

Comments

@brianritchie1312
Copy link
Contributor

meta-panel.controller uses a hard-wired list of inclusions for each entityType. This means that the meta-panel query always includes the same entities, not all of which may be required; it also means that the set of inclusions must be extended if new entity references are required (as happened recently with Study from Investigation).

The code extracts variable names from the meta-tab fields already; so it should be possible to replace the hard-wired .include()s by including the discovered variable names.

I am slightly hesitant because there may be other reasons why particular inclusions have been hard-wired; but this may be worth considering.

@brianritchie1312
Copy link
Contributor Author

I have tried replacing the hard-wired includes by including the extracted variable names; but it does not work. As a simple counter-example: for LILS investigations, the only extracted variable name is 'investigationUserPivot'. The hard-wired list does not include this, but does include 'user'. Replacing the hard-wired list with only 'investigationUserPivot' leads to an AngularJS "digest iterations" runaway. I am not sure why this happens, but it discourages me from trying further at the moment.

brianritchie1312 added a commit that referenced this issue Apr 18, 2019
See #405.

An attempt to replace the hard-wired inclusions by including the entityTypes discovered in the metatabs configuration.
This does not (always) work. In the current LILS configuration, metatabs for Investigation and Datafile are OK, but Dataset is not - the tabs do not complete properly, and sometimes there is a console error "Assertion failed: Input argument is not an HTMLInputElement".
I suspect that the hard-wired inclusions are still required. Whether also adding inclusions found in the configuration helps when "new" entity variables are mentioned is unknown.

Note that this change also adds logging for debug purposes.
brianritchie1312 added a commit that referenced this issue Apr 23, 2019
This now appears to work; I have not been able to reproduce the failures seen earlier (though the code should be functionally equivalent).
So I believe this change fixes #405.
@brianritchie1312
Copy link
Contributor Author

Fixes #217 (which I've just rediscovered!)

@brianritchie1312
Copy link
Contributor Author

I have found a counterexample which does not work with this change: a field expression such as:

datafileParameter[entity.type.valueType=='STRING'].stringValue

results in a red alert TypeError: entity.type is undefined. This is because though the new code detects the reference to datafileParameter and generates an inclusion for it, it does not detect the qualified expression, and cannot know that it should include datafileParameterType instead. The original code has the latter inclusion hard-wired (and it works).

Determining the correct set of inclusions from the configured metatab fields appears to be more difficult than anticipated. It is probably necessary to reuse or adapt the rather involved code used in the row calculations. I am not certain this is worthwhile.

Given that this counterexample is drawn from a production configuration (it is used by ISIS to display a metatab that shows all of a Datafile's parameters) I think that the branch that claims to fix this issue is not ready to be pulled into the master - and may never be! I am marking this issue as wontfix.

@stuartpullinger stuartpullinger added this to In Progress in icat.server May 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
icat.server
In Progress
Development

No branches or pull requests

1 participant