-
Notifications
You must be signed in to change notification settings - Fork 54
Feature/data library #1686
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
base: master
Are you sure you want to change the base?
Feature/data library #1686
Conversation
Please find the detailed integration test report here Please find the ci env pod logs here |
Please find the detailed integration test report here Please find the ci env pod logs here |
Please find the detailed integration test report here Please find the ci env pod logs here |
|
Please find the ci env pod logs here |
|
|
||
| .discovery-action-section { | ||
| display: flex; | ||
| flex-direction: row; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need this? there is only one element in that div
| if (currentList) { | ||
| saveToList(currentList.label, currentList.value); | ||
| } else { | ||
| saveToList(currentList.label); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
currentList might be null or undefined here?
| const mapStateToProps = (state) => ({ | ||
| user: state.user, | ||
| discovery: state.discovery, | ||
| systemPopupActivated: !!state.popups?.systemUseWarnPopup, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why we need this props for data library?
| disabled={error !== null || loading || data?.length === 0 || currentList === undefined || selectedResources.length === 0 || notLoggedIn} | ||
| onClick={() => { | ||
| if (currentList) { | ||
| saveToList(currentList.label, currentList.value); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we call fetchLists() after this? Otherwise if I'm creating a new list, the list ID will remains undefined until I refresh the page 🤔
| @@ -0,0 +1,183 @@ | |||
| import React, { | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wish we could have a story for this component in storybook
| - https://*.quicksight.aws.amazon.com for loading AWS Quicksight dashboards into COVID-19 Home page | ||
| --> | ||
| <meta http-equiv="Content-Security-Policy" content="default-src 'self' https://login.bionimbus.org https://wayf.incommonfederation.org; child-src blob:; img-src 'self' <%= htmlWebpackPlugin.options.imgSrc %> https://opendata.datacommons.io https://static.planx-pla.net data: https://*.s3.amazonaws.com blob:; script-src 'self' 'unsafe-eval' <%= htmlWebpackPlugin.options.scriptSrc %>; worker-src 'self' blob:; style-src 'self' 'unsafe-inline' https://fonts.googleapis.com; object-src 'none'; font-src 'self' data: https://fonts.googleapis.com https://fonts.gstatic.com; connect-src 'self' https://login.bionimbus.org https://wayf.incommonfederation.org https://opendata.datacommons.io https://static.planx-pla.net https://*.zendesk.com <%= htmlWebpackPlugin.options.connectSrc %>; frame-src <%= htmlWebpackPlugin.options.connectSrc %> 'self' https://auspice.planx-pla.net https://auspice.pandemicresponsecommons.org https://*.quicksight.aws.amazon.com;"> | ||
| <meta http-equiv="Content-Security-Policy" content="default-src 'self' http://localhost:8000 https://login.bionimbus.org https://wayf.incommonfederation.org; child-src blob:; img-src 'self' <%= htmlWebpackPlugin.options.imgSrc %> http://localhost:8000 https://opendata.datacommons.io https://static.planx-pla.net data: https://*.s3.amazonaws.com blob:; script-src 'self' 'unsafe-eval' <%= htmlWebpackPlugin.options.scriptSrc %>; worker-src 'self' blob:; style-src 'self' 'unsafe-inline' https://fonts.googleapis.com; object-src 'none'; font-src 'self' data: https://fonts.googleapis.com https://fonts.gstatic.com; connect-src 'self' https://login.bionimbus.org https://wayf.incommonfederation.org https://opendata.datacommons.io https://static.planx-pla.net https://*.zendesk.com <%= htmlWebpackPlugin.options.connectSrc %>; frame-src <%= htmlWebpackPlugin.options.connectSrc %> 'self' https://auspice.planx-pla.net https://auspice.pandemicresponsecommons.org https://*.quicksight.aws.amazon.com;"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this for local testing? if so I suggest moving them to dev.html and only use them from there
| scriptSrcURLs.push(...['https://www.google-analytics.com', 'https://ssl.google-analytics.com', 'https://www.googletagmanager.com']); | ||
| connectSrcURLs.push(...['https://www.google-analytics.com', 'https://*.analytics.google.com', 'https://analytics.google.com', 'https://*.g.doubleclick.net']); | ||
| scriptSrcURLs.push(...['http://localhost:8000', 'http://www.google-analytics.com','https://www.google-analytics.com', 'https://ssl.google-analytics.com', 'https://www.googletagmanager.com']); | ||
| connectSrcURLs.push(...['http://localhost:8000', 'https://*.analytics.google.com', 'https://analytics.google.com', 'https://*.g.doubleclick.net']); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think you need to do them here with google analytics
| <Button | ||
| loading={loading} | ||
| type='primary' | ||
| disabled={error !== null || loading || data?.length === 0 || currentList === undefined || selectedResources.length === 0 || notLoggedIn} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
data?.length === 0 seems to prevent any initial list creation. It does not seem to increase when a user's first list is initially entered (after creation, but before a study is Saved to list), and subsequently can't be Saved to List as the button remains disabled. I have tested removing this and it resolves this issue
Please find the detailed integration test report here Login here Please find the ci env pod logs here |
Please find the detailed integration test report here (login here first) |
a81f1d8 to
e7192e9
Compare
Please find the detailed integration test report here (login here first) |
add notifications with icons add doc for enableDataLibrary
e7192e9 to
bd45089
Compare
Please find the detailed integration test report here |
Please find the detailed integration test report here Please find the Github Action logs here |
Link to JIRA ticket if there is one:
New Features
Breaking Changes
Bug Fixes
Improvements
Dependency updates
Deployment changes