aminghadersohi commented on code in PR #36933:
URL: https://github.com/apache/superset/pull/36933#discussion_r2891592817
##########
superset/commands/chart/delete.py:
##########
@@ -68,3 +69,16 @@ def validate(self) -> None:
security_manager.raise_for_ownership(model)
except SupersetSecurityException as ex:
raise ChartForbiddenError() from ex
+
+
+class DeleteEmbeddedChartCommand(BaseCommand):
+ def __init__(self, chart: Slice):
+ self._chart = chart
+
+ @transaction(on_error=partial(on_error,
reraise=ChartDeleteEmbeddedFailedError))
+ def run(self) -> None:
+ self.validate()
+ return EmbeddedChartDAO.delete(self._chart.embedded)
+
+ def validate(self) -> None:
+ pass
Review Comment:
Fixed in a8b866e. Added existence, embedded-record, and ownership validation
to `DeleteEmbeddedChartCommand.validate()`.
##########
superset/config.py:
##########
@@ -559,6 +559,8 @@ class D3TimeFormat(TypedDict, total=False):
# This feature flag is stil in beta and is not recommended for production
use.
"GLOBAL_ASYNC_QUERIES": False,
"EMBEDDED_SUPERSET": False,
+ # Enables the "Embed code" and "Embed chart" options in the Share menu
+ "EMBEDDABLE_CHARTS": True,
Review Comment:
Fixed in a8b866e. Changed `EMBEDDABLE_CHARTS` to default to `False` so the
feature is opt-in.
##########
embed-demo.html:
##########
@@ -0,0 +1,110 @@
+<!DOCTYPE html>
+<html>
+<head>
+ <title>Superset Embedded Chart Demo</title>
+ <style>
+ body {
+ font-family: -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto,
sans-serif;
+ max-width: 1200px;
+ margin: 0 auto;
+ padding: 40px;
+ background: #f5f5f5;
+ }
+ h1 { color: #333; }
+ .input-section {
+ background: white;
+ border-radius: 8px;
+ padding: 20px;
+ box-shadow: 0 2px 8px rgba(0,0,0,0.1);
+ margin-bottom: 20px;
+ }
+ textarea {
+ width: 100%;
+ height: 150px;
+ font-family: monospace;
+ font-size: 12px;
+ padding: 10px;
+ border: 1px solid #ddd;
+ border-radius: 4px;
+ resize: vertical;
+ box-sizing: border-box;
+ }
+ button {
+ background: #20a7c9;
+ color: white;
+ border: none;
+ padding: 12px 24px;
+ font-size: 16px;
+ border-radius: 4px;
+ cursor: pointer;
+ margin-top: 10px;
+ }
+ button:hover {
+ background: #1a8fa8;
+ }
+ .chart-container {
+ background: white;
+ border-radius: 8px;
+ padding: 20px;
+ box-shadow: 0 2px 8px rgba(0,0,0,0.1);
+ min-height: 450px;
+ }
+ label {
+ font-weight: 600;
+ display: block;
+ margin-bottom: 8px;
+ }
+ </style>
+</head>
+<body>
+ <h1>Superset Embedded Chart Demo</h1>
+
+ <div class="input-section">
+ <label for="iframe-input">Paste iframe_html response here:</label>
+ <textarea id="iframe-input" placeholder="Paste the iframe_html value from
get_embeddable_chart response..."></textarea>
+ <button onclick="embedChart()">Embed Chart</button>
+ </div>
+
+ <div class="chart-container" id="chart-container">
+ <p style="color: #999; text-align: center; margin-top: 200px;">Chart will
appear here</p>
+ </div>
+
+ <script>
+ function embedChart() {
+ const input = document.getElementById('iframe-input').value.trim();
+ const container = document.getElementById('chart-container');
+
+ if (!input) {
+ alert('Please paste the iframe_html first');
+ return;
+ }
+
+ // Insert the iframe HTML
+ container.innerHTML = input;
Review Comment:
Fixed in a8b866e. Added URL scheme validation (only `http://` or `https://`
allowed) and `sandbox` attribute to the created iframe to prevent javascript:
URI injection.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]