You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "mistercrunch (via GitHub)" <gi...@apache.org> on 2023/06/12 21:58:18 UTC
[GitHub] [superset] mistercrunch opened a new pull request, #24368: feat: make data tables support html
mistercrunch opened a new pull request, #24368:
URL: https://github.com/apache/superset/pull/24368
### SUMMARY
allowing links and other basic HTML to render in data tables safely
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<img width="460" alt="Screen Shot 2023-06-12 at 2 23 41 PM" src="https://github.com/apache/superset/assets/487433/3df03a4b-da97-4ea4-adf4-cc2cd469a48c">
<img width="857" alt="Screen Shot 2023-06-12 at 2 23 26 PM" src="https://github.com/apache/superset/assets/487433/63f64efb-ce02-4bc8-bbdf-35ac8a55babd">
### TESTING INSTRUCTIONS
* create an expression or calculated dim
* render in SQL Lab, samples, result set view
### ADDITIONAL INFORMATION
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] worker24h commented on pull request #24368: feat: make data tables support html
Posted by "worker24h (via GitHub)" <gi...@apache.org>.
worker24h commented on PR #24368:
URL: https://github.com/apache/superset/pull/24368#issuecomment-1594347890
> > The feature is very best，I need it too.
> > But, I hope to that, **Add a config item** by Web UI ，rather than **CONCAT In sql** ，
> > What are your ideas ？ @mistercrunch
> > 
>
> I think the concat is nice in that it allows you to re-use the columnar data anywhere needed in the link. For example, a Github issue ID is used as the linked text AND a particular spot in the URL. If we were to add a link to the UI, we would need some form of syntax (e.g. handlebars/jinja) that would allow you to construct the link in the correct way... and that can have security-related concerns.
>
> I love the idea of it being in the UI, but I'm having a hard time thinking of a way to do it that's both simple and secure.
YES, I got it, thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] rusackas commented on a diff in pull request #24368: feat: make data tables support html
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on code in PR #24368:
URL: https://github.com/apache/superset/pull/24368#discussion_r1228362829
##########
superset-frontend/packages/superset-ui-core/src/utils/html.tsx:
##########
@@ -0,0 +1,53 @@
+import React from 'react';
+import { FilterXSS, getDefaultWhiteList } from 'xss';
+
+const xssFilter = new FilterXSS({
+ whiteList: {
Review Comment:
"allowList" would be better :)
Also, wondering if we can leverage the config.py's HTML_SANITIZATION_SCHEMA_EXTENSIONS here to keep things DRY. I guess that depends on whether or not people are going to want to start punching holes in this config.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] worker24h commented on pull request #24368: feat: make data tables support html
Posted by "worker24h (via GitHub)" <gi...@apache.org>.
worker24h commented on PR #24368:
URL: https://github.com/apache/superset/pull/24368#issuecomment-1590493590
The feature is very best，I need it too.
But, I hope to that, **Add config item** by Web UI ，rather than ** CONCAT In sql **
What are your ideas ？ @mistercrunch
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] nytai commented on pull request #24368: feat: make data tables support html
Posted by "nytai (via GitHub)" <gi...@apache.org>.
nytai commented on PR #24368:
URL: https://github.com/apache/superset/pull/24368#issuecomment-1592105910
Hmm, I was just looking into adding this support to the pivot tables too.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] alexandrafetterman commented on pull request #24368: feat: make data tables support html
Posted by "alexandrafetterman (via GitHub)" <gi...@apache.org>.
alexandrafetterman commented on PR #24368:
URL: https://github.com/apache/superset/pull/24368#issuecomment-1613585464
Will this work in just regular Table charts? Like by applying custom SQL to fields I have in the table using concatenate? Or just SQLlab?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] mistercrunch merged pull request #24368: feat: make data tables support html
Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch merged PR #24368:
URL: https://github.com/apache/superset/pull/24368
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] commented on pull request #24368: feat: make data tables support html
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #24368:
URL: https://github.com/apache/superset/pull/24368#issuecomment-1590497946
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/24368?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#24368](https://app.codecov.io/gh/apache/superset/pull/24368?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (009951e) into [master](https://app.codecov.io/gh/apache/superset/commit/a3aacf2527086fac010fdd3f1feb5e9eab3c7562?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (a3aacf2) will **decrease** coverage by `0.06%`.
> The diff coverage is `93.24%`.
> :exclamation: Current head 009951e differs from pull request most recent head f94dd98. Consider uploading reports for the commit f94dd98 to get more accurate results
```diff
@@ Coverage Diff @@
## master #24368 +/- ##
==========================================
- Coverage 69.05% 68.99% -0.06%
==========================================
Files 1903 1904 +1
Lines 74530 74144 -386
Branches 8105 8120 +15
==========================================
- Hits 51464 51157 -307
+ Misses 20955 20875 -80
- Partials 2111 2112 +1
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `55.65% <90.00%> (+0.03%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://app.codecov.io/gh/apache/superset/pull/24368?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...acy-preset-chart-deckgl/src/components/Tooltip.tsx](https://app.codecov.io/gh/apache/superset/pull/24368?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9sZWdhY3ktcHJlc2V0LWNoYXJ0LWRlY2tnbC9zcmMvY29tcG9uZW50cy9Ub29sdGlwLnRzeA==) | `0.00% <0.00%> (ø)` | |
| [...lugins/plugin-chart-table/src/utils/formatValue.ts](https://app.codecov.io/gh/apache/superset/pull/24368?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtdGFibGUvc3JjL3V0aWxzL2Zvcm1hdFZhbHVlLnRz) | `66.66% <0.00%> (-3.93%)` | :arrow_down: |
| [...c/components/Chart/DrillDetail/DrillDetailPane.tsx](https://app.codecov.io/gh/apache/superset/pull/24368?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvQ2hhcnQvRHJpbGxEZXRhaWwvRHJpbGxEZXRhaWxQYW5lLnRzeA==) | `75.32% <ø> (ø)` | |
| [...end/src/components/Datasource/DatasourceEditor.jsx](https://app.codecov.io/gh/apache/superset/pull/24368?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRGF0YXNvdXJjZS9EYXRhc291cmNlRWRpdG9yLmpzeA==) | `65.35% <ø> (ø)` | |
| [...-frontend/src/components/TableCollection/index.tsx](https://app.codecov.io/gh/apache/superset/pull/24368?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvVGFibGVDb2xsZWN0aW9uL2luZGV4LnRzeA==) | `100.00% <ø> (ø)` | |
| [superset-frontend/src/dashboard/actions/hydrate.js](https://app.codecov.io/gh/apache/superset/pull/24368?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9hY3Rpb25zL2h5ZHJhdGUuanM=) | `2.04% <ø> (ø)` | |
| [...mponents/DataTablesPane/components/SamplesPane.tsx](https://app.codecov.io/gh/apache/superset/pull/24368?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9EYXRhVGFibGVzUGFuZS9jb21wb25lbnRzL1NhbXBsZXNQYW5lLnRzeA==) | `97.67% <ø> (ø)` | |
| [...ataTablesPane/components/SingleQueryResultPane.tsx](https://app.codecov.io/gh/apache/superset/pull/24368?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9EYXRhVGFibGVzUGFuZS9jb21wb25lbnRzL1NpbmdsZVF1ZXJ5UmVzdWx0UGFuZS50c3g=) | `85.71% <ø> (ø)` | |
| [...erset-frontend/src/profile/components/fixtures.tsx](https://app.codecov.io/gh/apache/superset/pull/24368?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3Byb2ZpbGUvY29tcG9uZW50cy9maXh0dXJlcy50c3g=) | `100.00% <ø> (ø)` | |
| [superset/models/sql\_lab.py](https://app.codecov.io/gh/apache/superset/pull/24368?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvbW9kZWxzL3NxbF9sYWIucHk=) | `79.51% <0.00%> (+0.94%)` | :arrow_up: |
| ... and [16 more](https://app.codecov.io/gh/apache/superset/pull/24368?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | |
... and [8 files with indirect coverage changes](https://app.codecov.io/gh/apache/superset/pull/24368/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] rusackas commented on a diff in pull request #24368: feat: make data tables support html
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on code in PR #24368:
URL: https://github.com/apache/superset/pull/24368#discussion_r1228365407
##########
superset-frontend/packages/superset-ui-core/src/utils/html.tsx:
##########
@@ -0,0 +1,53 @@
+import React from 'react';
+import { FilterXSS, getDefaultWhiteList } from 'xss';
+
+const xssFilter = new FilterXSS({
+ whiteList: {
Review Comment:
Sorry, didn't realize this was legacy code being moved over... it just kinda jumped out at me :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] mistercrunch commented on a diff in pull request #24368: feat: make data tables support html
Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch commented on code in PR #24368:
URL: https://github.com/apache/superset/pull/24368#discussion_r1228369730
##########
superset-frontend/packages/superset-ui-core/src/utils/html.tsx:
##########
@@ -0,0 +1,53 @@
+import React from 'react';
+import { FilterXSS, getDefaultWhiteList } from 'xss';
+
+const xssFilter = new FilterXSS({
+ whiteList: {
Review Comment:
yeah I wonder if a newer version of the `xss` lib has alternative semantics
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] rusackas commented on pull request #24368: feat: make data tables support html
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #24368:
URL: https://github.com/apache/superset/pull/24368#issuecomment-1591858100
> The feature is very best，I need it too.
>
> But, I hope to that, **Add a config item** by Web UI ，rather than **CONCAT In sql** ，
>
> What are your ideas ？ @mistercrunch
>
> 
I think the concat is nice in that it allows you to re-use the columnar data anywhere needed in the link. For example, a Github issue ID is used as the linked text AND a particular spot in the URL. If we were to add a link to the UI, we would need some form of syntax (e.g. handlebars/jinja) that would allow you to construct the link in the correct way... and that can have security-related concerns.
I love the idea of it being in the UI, but I'm having a hard time thinking of a way to do it that's both simple and secure.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org