-
Notifications
You must be signed in to change notification settings - Fork 18
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
Download option for visualisations and dashboards (connect #952) #1586
Download option for visualisations and dashboards (connect #952) #1586
Conversation
finnfiddle
commented
Aug 1, 2018
- Update release notes if necessary
…shboards-#952 * develop: Add more details about how retrieving values Refactor: Fix indentation Use `concat` instead of `||` in sql update. Relates to #1517 Add documentation on how to create new tenant #1514 Add documentation on how to create new tenant #1514 [#1408] Fix bug in raster import failure case [#1408] Clean up unsuccessful raster imports better [#1408] Client par of delete pending rasters WIP [#1408] Delete pending datasets WIP # Conflicts: # client/package-lock.json
Adapt backend env vars
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.
Looks like we miss translation strings, seeing:
[React Intl] Missing message: "save_your_visualisation_before_exporting" for locale: "en"
@@ -107,6 +107,7 @@ | |||
:sentry-backend-dsn sentry-backend-dsn | |||
:flow-api-url flow-api-url | |||
:windshaft-url "http://localhost:4000" | |||
:exporter-api-url "http://localhost:3001" |
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 use bound symbol from config.clj
backend/src/akvo/lumen/config.clj
Outdated
@@ -24,6 +24,7 @@ | |||
'email-password (:lumen-email-password env) | |||
'email-user (:lumen-email-user env) | |||
'encryption-key (:lumen-encryption-key env) | |||
'exporter-api-url "http://localhost:3001" |
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 use env var or fallback to default as in (:exporter-api-url env "http://localhost:3001")
… and 'Download-option-for-visualisations-and-dashboards-#952' of github.com:akvo/akvo-lumen into Download-option-for-visualisations-and-dashboards-#952 * 'Download-option-for-visualisations-and-dashboards-#952' of github.com:akvo/akvo-lumen: Fix backend env vars * 'Download-option-for-visualisations-and-dashboards-#952' of github.com:akvo/akvo-lumen: Fix backend env vars
exporter/Dockerfile
Outdated
@@ -0,0 +1,14 @@ | |||
FROM zenato/puppeteer |
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.
Can we have a more specific version instead of latest?
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'm not sure there are any other tags, see https://hub.docker.com/r/zenato/puppeteer/tags/
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 see there are plenty of puppeteer images. See https://hub.docker.com/search/?isAutomated=0&isOfficial=0&page=1&pullCount=0&q=puppeteer&starCount=0.
The most popular one has tags.
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 first went with the most popular one alekzonder/puppeteer
but it uses an outdated version of puppeteer. Hence my choice. But the 2nd one could work because it uses the latest puppeteer - https://hub.docker.com/r/buildkite/puppeteer/. Will try now
… of github.com:akvo/akvo-lumen into Download-option-for-visualisations-and-dashboards-#952 * 'Download-option-for-visualisations-and-dashboards-#952' of github.com:akvo/akvo-lumen: [#952] Silent cypress npm install [#952] Upgrade Cypress and quiet install output [#952] Enabled first Cypress tests [#952] Temporary disable cypress test
Since this feature uses a new exporter service we need to make sure to deploy that in the k8s environment. |
Todo:
|
First cut on not initialising a Keycloak session using token & refresh token but using the given token.
…shboards-#952 * develop: (55 commits) Disable *again* update for datasets coming from Data files [#1562] Fix query [#1638] Drop dataset source column [#1638] Select data_source.spec instead of dataset.source [#1638] [#1642] Upgrade to latest Windshaft image Revert isUpdatable logic Maps show small on shared link #1632 #1576 fixed rich text styling issues rich text editor fixes #1576 Use proper visualizable metadata in legend [client] [#1579] More translations [i18n] Set state.error=null on 200 http status Improve UI styling and translations Improvements [#1626] && Manage 404 backend http response Add "unit" to caddisfly "name" response Add aws url to caddisfly images [#1571] Tidy up multiple-column details response [#1571] inc z-index over maximum found in lumen css [#1561] Avoid add-to-collection opt in case of failing dataset [#1546] Disable update for datasets coming from Data files [#1562] ... # Conflicts: # backend/resources/akvo/lumen/system.edn # client/e2e-test/Dockerfile # client/package-lock.json # client/src/components/dashboard/DashboardEditor.jsx # client/src/translations/en.json # client/src/translations/es.json
Adding a viz to a Dashboard generates the following browser console warning:
|
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.
Looks good to me now
backend/src/akvo/lumen/config.clj
Outdated
@@ -10,6 +10,7 @@ | |||
(assert (:lumen-keycloak-client-secret env) | |||
(error-msg "LUMEN_KEYCLOAK_CLIENT_SECRET")) | |||
(assert (:lumen-keycloak-url env) (error-msg "LUMEN_KEYCLOAK_URL")) | |||
#_(assert (:exporter-api-url env) (error-msg "EXPORTER_API_URL")) |
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.
maybe we can remove definitely this line
…shboards-#952 * develop: (37 commits) Hovering over charts shows different colour in shared visuals and dashboards #1667 #1673 fixed pivot table data caching bug when switch vis type Fix harcoded url in dev environment (#1688) Bar chart Show label toggle turned on for existing bars #1685 Bar chart value labels do not fit in the bar #1682 #1673 fixed linting Change visualisation type #1673 Add Show labels toggle for bar charts #1655 Changing dataset on existing chart crashes app #1679 bar labels toggle fixes 4 #1655 bar labels toggle fixes 3 #1655 Improve derive/parse-row-object-references impl and test coverage Align translations Support to row names with hyphens in update dataset #1655 bar labels toggle fixes Pie chart label not shown #1558 Add Show labels toggle for bar charts #1655 Add transactional support to update dataset Dataset initial view has a right blank empty rectangle #1658 #1573 added missing translations #1573 removed unused dep ... # Conflicts: # client/src/components/charts/PivotTable.jsx # client/src/components/charts/SimpleBarChart.jsx # client/src/components/visualisation/VisualisationPreview.jsx # client/src/containers/Visualisation.jsx # client/src/translations/en.json # client/src/translations/fr.json