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

fix/HIV app and Guppy #558

Merged
merged 10 commits into from
Jul 29, 2019
Merged

fix/HIV app and Guppy #558

merged 10 commits into from
Jul 29, 2019

Conversation

mfshao
Copy link
Collaborator

@mfshao mfshao commented Jul 26, 2019

Let NIAID's HIV app support Guppy

Please make sure to follow the DEV guidelines before asking for review.

New Features

  • Replace Arranger by Guppy for HIV apps

Breaking Changes

Bug Fixes

Improvements

Dependency updates

Deployment changes

@@ -38,6 +38,10 @@ class ExplorerFilter extends React.Component {
}
}
}
return null;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

to address a warning found in console while testing using dev.html.
Warning: ExplorerFilter.getSnapshotBeforeUpdate(): A snapshot value (or null) must be returned. You have returned undefined.

return null;
}

componentDidUpdate() {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

to address a warning found in console while testing using dev.html.
Warning: ExplorerFilter: getSnapshotBeforeUpdate() should be used with componentDidUpdate(). This component defines getSnapshotBeforeUpdate() only.

@mfshao mfshao requested review from abgeorge7 and ZakirG July 26, 2019 17:26
@mfshao mfshao marked this pull request as ready for review July 26, 2019 17:27
@mfshao
Copy link
Collaborator Author

mfshao commented Jul 26, 2019

deployed in https://qa-niaid.planx-pla.net

Copy link
Contributor

@abgeorge7 abgeorge7 left a comment

Choose a reason for hiding this comment

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

When I run this locally (qa-niaid) I get: SyntaxError: Unexpected token u in JSON at position 0

but maybe my local config is out of date? Is it deployed to qa-niaid where we can test there?

@mfshao
Copy link
Collaborator Author

mfshao commented Jul 26, 2019

When I run this locally (qa-niaid) I get: SyntaxError: Unexpected token u in JSON at position 0

but maybe my local config is out of date? Is it deployed to qa-niaid where we can test there?

hmmm... yeah it is deployed on QA-NIAID now

Copy link
Contributor

@abgeorge7 abgeorge7 left a comment

Choose a reason for hiding this comment

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

Looks good to me but I'd default to @ZakirG

body: JSON.stringify({
query: queryString,
variables: JSON.parse(variableString),
Copy link
Contributor

Choose a reason for hiding this comment

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

In the case where we use arranger, variableString will be undefined right? And I think that will lead to the error Uncaught SyntaxError: Unexpected token u in JSON at position 0

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

nice catch! fixed in 1dd7906

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants