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

PTV-1315 Fix AlignmentSet viewer #2501

Open
wants to merge 57 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
57 commits
Select commit Hold shift + click to select a range
6ca7e5c
Initial fix for PTV-1315
eapearson Nov 12, 2019
c918671
Merge remote-tracking branch 'upstream/develop' into fix-PTV-1315
eapearson Sep 30, 2021
acb152e
add AMD support for React; tried to use the commented-out shim, but c…
eapearson Sep 30, 2021
91ae02c
port preact components to react
eapearson Sep 30, 2021
bd57026
remove commented-out preact from page
eapearson Sep 30, 2021
bb54451
remove unused variables, move some styles to stylesheet, refactor pro…
eapearson Oct 5, 2021
d1885db
update npm deps (med security warning), upgrade testing chrome to 93,…
eapearson Oct 5, 2021
fb5e493
address code styling issues
eapearson Oct 5, 2021
d505ca6
move mswUtils to a better home, and adjust import paths
eapearson Oct 5, 2021
0bfc5a9
silly tabs
eapearson Oct 5, 2021
2b80522
add direct suppport for react and react-dom, rather than using global…
eapearson Oct 6, 2021
e4a5e2b
refactor set viewer code
eapearson Oct 6, 2021
8f6bb25
tests simple react component, stub test for another
eapearson Oct 6, 2021
7d49f12
add peer dependency stylelint; change name of install-npm -> install-…
eapearson Oct 6, 2021
cd58cf5
move styles to stylesheet
eapearson Oct 7, 2021
af4524a
typo
eapearson Oct 8, 2021
eeaa4bf
move test utils and adjust, and make absolute, imports
eapearson Oct 8, 2021
cff7f08
add more tests, test data, and light refactoring for testability
eapearson Oct 8, 2021
abd0057
add shebang to shell script
eapearson Oct 8, 2021
de8fc7e
refactor naming, keep truckin' on docs
eapearson Oct 9, 2021
5dc8ef6
update usage of list_objects to use list of workspaces; tests pass ag…
eapearson Oct 10, 2021
11e1f54
removed comments, have been moved to a README
eapearson Oct 10, 2021
505b59f
use safer setState
eapearson Oct 10, 2021
6559f7b
remove now-unused arg
eapearson Oct 10, 2021
5ae7ca1
improve file comment
eapearson Oct 10, 2021
f3d16aa
codacy now uses "remark" for formatting, so switch to that and reform…
eapearson Oct 10, 2021
c79a888
improve select control display with very long labels
eapearson Oct 11, 2021
b6e0e90
fix error display, add "loading" text, remove data generation logging
eapearson Oct 11, 2021
0412355
add tests for all viewer states;
eapearson Oct 11, 2021
8364701
add prop-types, refactor components to suit, refactor tests to suit t…
eapearson Oct 11, 2021
5b4a457
make codacy happier through simplification?
eapearson Oct 11, 2021
f7cc9dd
codacy formatting changes
eapearson Oct 11, 2021
4a382e4
replace node-module installer shell script with javascript script
eapearson Oct 11, 2021
7ae5fc8
move type to module mapping into shared function, remove commented ou…
eapearson Oct 12, 2021
687a147
improve KBaseSets viewer README
eapearson Oct 14, 2021
4c3e873
rename components
eapearson Oct 14, 2021
0369e62
backport list_objects fix from truss branch
eapearson Oct 15, 2021
e30fc6b
more backporting list_workspaces changes (fixes to the previous chang…
eapearson Oct 15, 2021
86fa014
change name of install-node-modules to copy-node-modules-to-ext-modules
eapearson Oct 15, 2021
c9feac9
Add specific error message for unsupported type and object; add more …
eapearson Oct 15, 2021
cb89ffe
tidying up tests.
eapearson Oct 15, 2021
a819699
add tsets for SetTypeResolver
eapearson Oct 15, 2021
43ee556
refactor react component rendering into utils
eapearson Oct 15, 2021
c241c8a
update release notes
eapearson Oct 18, 2021
6170784
re-cherrypick list_objects changes in prep for merge from canonical repo
eapearson Oct 19, 2021
bab2335
Merge remote-tracking branch 'upstream/develop' into PTV-1315-Alignme…
eapearson Oct 19, 2021
d56e40e
udpate release notes
eapearson Oct 19, 2021
9ea17c4
styling
eapearson Oct 22, 2021
d1d796f
bootstrap select -> inline
eapearson Oct 22, 2021
a690d54
add more information to "unknown error"; should never occur
eapearson Oct 25, 2021
a8a0d79
Merge remote-tracking branch 'origin/develop' into PTV-1315-Alignment…
eapearson Oct 26, 2021
d2c482c
sync from origin, add back in what we need
eapearson Oct 26, 2021
b07e84d
catch up dependencies post-merge; I don't trust package.json post mer…
eapearson Oct 26, 2021
5a01d4a
move unit test utils into main amd module file tree to remove warning…
eapearson Oct 27, 2021
152e8a6
remove unit test utils pattern
eapearson Oct 27, 2021
4ce8d52
restore script name changed in sync from origin
eapearson Oct 27, 2021
a904655
make tab test more reliable
eapearson Oct 27, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .stylelintrc.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"extends": "stylelint-config-standard"
}
1 change: 1 addition & 0 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ RUN \
# install JS deps
npm install -g grunt-cli && \
npm install && \
npm run install-node-modules && \
./node_modules/.bin/bower install --allow-root --config.interactive=false && \
# Compile Javascript down into an itty-bitty ball unless SKIP_MINIFY is non-empty
echo Skip=$SKIP_MINIFY && \
Expand Down
3 changes: 3 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ run-dev-image:
install:
bash $(INSTALLER)

install-node-modules:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit misleadingly named as it actually moves kbase node modules elsewhere; it doesn't install npm modules if they aren't already there. Can you rename it something more appropriate?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. Had hoped it was clear that the "-node-modules" suffix implies that it installs stuff from "node_modules", rather than something like "-node-packages", which nom install does, but alas clearly not, and punning off of make install, which since it "installs" node modules into the narrative.
So, brand-new name: "copy-node-modules-into-ext-modules" captures that it is a "copy", that the subject is the "node modules" directory tree, and that they are being copied into the "ext_modules" directory, which is where node package UMD modules and related assets are installed, er, copied for usage in the front end.

sh scripts/install-node-modules.sh

# runs the installer to locally build the Narrative in a
# local venv.
build-travis-narrative:
Expand Down
2 changes: 1 addition & 1 deletion docs/developer/local-docker.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ KBASE_TEST_TOKEN=YOUR_TOKEN_HERE BASE_URL=https://ci.kbase.us make test-integrat
```

E.g.

```bash
SERVICE=selenium-standalone BROWSER=chrome HEADLESS=f BASE_URL=https://ci.kbase.us KBASE_TEST_TOKEN=TOKEN_HERE make test-integration
```
Expand Down Expand Up @@ -84,7 +85,6 @@ All of the testing options may be used

#### narrative-dev local container


#### next local container

#### prod local container
Expand Down
2 changes: 1 addition & 1 deletion docs/testing/unit-testing-with-msw.md
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ To support more advanced testing scenarios, involving authentication, search, et

## Files involved

- test/unit/util/mswUtils.js
- test/unit/utils/mswUtils.js
- test/unit/karma.conf.js
- test/unit/test-main.js
- package.json
Expand Down
3 changes: 0 additions & 3 deletions kbase-extension/kbase_templates/page.html
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,6 @@
<!-- End Google Tag Manager -->

<script src="{{static_url('components/es6-promise/promise.min.js')}}" type="text/javascript" charset="utf-8"></script>
<!-- <script src="{{static_url('components/preact/index.js')}}" type="text/javascript"></script>
<script src="{{static_url('components/proptypes/index.js')}}" type="text/javascript"></script>
<script src="{{static_url('components/preact-compat/index.js')}}" type="text/javascript"></script> -->
<script src="{{static_url('components/react/react.production.min.js')}}" type="text/javascript"></script>
<script src="{{static_url('components/react/react-dom.production.min.js')}}" type="text/javascript"></script>
<script src="{{static_url('components/create-react-class/index.js')}}" type="text/javascript"></script>
Expand Down
48 changes: 48 additions & 0 deletions kbase-extension/static/kbase/js/react_components/ErrorMessage.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
define([
'react'
], (
React
) => {
'use strict';

const { createElement: e, Component } = React;

/**
* A class to display an error message.
*
* This is a simple error viewer. It does not show extended information like
* stacktrace, or richer data from Service clients, like the error code.
* It should handle service client errors, any error based on the Error class,
* any object which has a "message" property, and a simple string.
*/
class ErrorMessage extends Component {
render() {
const error = this.props.error;
let message;
if (typeof error === 'object' && error !== null) {
if (error.error && error.error.message) {
// handle errors thrown by kbase service clients
message = error.error.message;
} else if (error.message) {
// Standard Error objects or descendants, or those which act like one in this regard.
message = error.message;
} else {
message = 'Unknown Error';
}
} else if (typeof error === 'string') {
message = error;
} else {
message = 'Unknown Error';
}

return e('div', {
className: 'alert alert-danger'
}, [
'Error: ',
message
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you could remove both the elses and have the default text for message be Unknown Error -- save a bit of space and repetition

]);
}
}

return ErrorMessage;
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,211 @@
/*
kbaseGenericSetViewer

A widget which can display a data object visualization for several types of objects
in the "KBaseSets" type module.

The primary task of this viewer is to look up the object type it is provided, and, based
on the type of the object, either dispatch to the set viewer if it is supported, or
display a warning that the type is not supported if it isn't.

Support is indicated in the module-level `SET_TYPE_TO_MODULE_MAPPING`.

At present just ReadsAlignmentSet and AssemblySet are supported; others should display
an error message, or are handled by other viewers.

Note that this widget is a wrapper dispatching to the specific viewer, most of the
implementation is in the React components referred two in the AMD dependencies. The only
UI supported herein is the loading indicator and an error message, if necessary.

The original intention of this viewer was to support all of the types implemented in KBaseSets,
but that work has not yet been completed:

Also, see: https://github.com/kbase/NarrativeViewers/blob/dd1eeeba0ba1faacd6c3596a9413109aeb82e32e/ui/narrative/methods/view_generic_set/spec.json#L21

Below is the status of each KBaseSet type:

Implemented in this viewer:

KBaseSets.ReadsAlignmentSet
SetAPI.get_reads_alignment_set_v1

KBaseSets.AssemblySet
SetAPI.get_assembly_set_v1

Implemented in other viewers:

KBaseSets.DifferentialExpressionMatrixSet
SetAPI.get_differential_expression_matrix_set_v1
Implemented by:
https://github.com/kbase/NarrativeViewers/blob/dd1eeeba0ba1faacd6c3596a9413109aeb82e32e/ui/narrative/methods/view_differential_expression_matrix_set/spec.json
and this viewer does not work

KBaseSets.ExpressionSet
SetAPI.get_expression_set_v1
Implemented by:
https://github.com/kbase/NarrativeViewers/blob/dd1eeeba0ba1faacd6c3596a9413109aeb82e32e/ui/narrative/methods/view_rnaseq_sample_expression/spec.json
and that viewer doesn't work

KBaseSets.ReadsSet
SetAPI.get_reads_set_v1
Implemented by:
https://github.com/kbase/NarrativeViewers/blob/dd1eeeba0ba1faacd6c3596a9413109aeb82e32e/ui/narrative/methods/view_reads_set/spec.json
and the current viewer works

KBaseSets.SampleSet
SetAPI.sample_set_to_samples_info (I think)
Implemented by:
https://github.com/kbase/NarrativeViewers/tree/master/ui/narrative/methods/view_sample_set
but not that this viewer does not utilize the SetAPI, which was implemented against the search, which
has never been fully implemented for search. Also Samples and RNASeqSamples seem to be conflated
a bit in SetAPI.

Not implemented anywhere:

KBaseSets.FeatureSetSet
SetAPI.get_feature_set_set_v1
can't find any data, or any app which outputs this type, or accepts as input (according to the app browser)

KBaseSets.GenomeSet
SetAPI.get_genome_set_v1

*/
define([
'react',
'kb_common/jsonRpc/genericClient',
'kb_service/utils',
'widgets/function_output/KBaseSets/types/ReadsAlignmentSet',
'widgets/function_output/KBaseSets/types/AssemblySet',
'react_components/ErrorMessage',
'widgets/function_output/KBaseSets/SetLoader',

// For effect
'bootstrap'
], (
React,
ServiceClient,
ServiceUtils,
ReadsAlignmentSet,
AssemblySet,
ErrorMessage,
SetLoader
) => {
'use strict';

const { createElement: e, Component } = React;

// Note that any modules referenced must be imported above, and that such modules
// must be components implemented as defined in `react_components/genericSets`.
const SET_TYPE_TO_MODULE_MAPPING = {
'KBaseSets.ReadsAlignmentSet': {
module: ReadsAlignmentSet,
method: 'get_reads_alignment_set_v1'
},
'KBaseSets.AssemblySet': {
module: AssemblySet,
method: 'get_assembly_set_v1'
}
};

/*
Implements a React component which primarily dispatches to the component which
matches the type of object provided in the prop 'objectRef'.
*/
class Dispatcher extends Component {
constructor(props) {
super(props);
this.state = {
status: null,
// setType: null,
// error: null
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rm comments

};
}

async componentDidMount() {
this.setState({
status: 'loading'
});

try {
const workspace = new ServiceClient({
url: this.props.workspaceURL,
module: 'Workspace',
token: this.props.token
});

const [infos] = await workspace.callFunc('get_object_info_new', [{
objects: [{
ref: this.props.objectRef
}]
}]);

// Get a nicer object info (an object rather than array)
const objectInfo = ServiceUtils.objectInfoToObject(infos[0]);

// Make a versionless object type id for lookup in the set of supported
// set types defined above.
const objectType = [objectInfo.typeModule, objectInfo.typeName].join('.');

const mapping = SET_TYPE_TO_MODULE_MAPPING[objectType];
if (!mapping) {
this.setState({
status: 'error',
error: new Error(`Unsupported set type: ${this.state.setType}`)
});
return;
}

this.setState({
status: 'loaded',
setType: objectType,
module: mapping.module,
method: mapping.method
});
} catch (error) {
console.error('Error getting object info', error);
this.setState({
status: 'error',
error
});
}
}

renderError() {
return e(ErrorMessage, {
error: this.state.error
});
}

renderLoaded() {
return e(SetLoader, {
token: this.props.token,
workspaceURL: this.props.workspaceURL,
serviceWizardURL: this.props.serviceWizardURL,
objectRef: this.props.objectRef,
method: this.state.method,
module: this.state.module
});

// return e(this.state.module, {
// token: this.props.token,
// workspaceURL: this.props.workspaceURL,
// serviceWizardURL: this.props.serviceWizardURL,
// objectRef: this.props.objectRef
// });
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rm comments

}

render() {
switch (this.state.status) {
case null:
case 'loading':
return e('div', null, 'Loading...');
case 'error':
return this.renderError();
case 'loaded':
return this.renderLoaded();
}
}
}

return Dispatcher;
});
Loading