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]

Reply via email to