Vitor-Avila commented on code in PR #29530:
URL: https://github.com/apache/superset/pull/29530#discussion_r1672316209


##########
superset/security/manager.py:
##########
@@ -170,32 +170,37 @@ def query_context_modified(query_context: "QueryContext") 
-> bool:
     if form_data.get("slice_id") != stored_chart.id:
         return True
 
-    # compare form_data
-    requested_metrics = {
-        freeze_metric(metric) for metric in form_data.get("metrics") or []
-    }
-    stored_metrics = {
-        freeze_metric(metric)
-        for metric in stored_chart.params_dict.get("metrics") or []
-    }
-    if not requested_metrics.issubset(stored_metrics):
-        return True
+    stored_query_context = (
+        json.loads(cast(str, stored_chart.query_context))
+        if stored_chart.query_context
+        else None
+    )
 
-    # compare queries in query_context
-    queries_metrics = {
-        freeze_metric(metric)
-        for query in query_context.queries
-        for metric in query.metrics or []
-    }
+    # compare columns and metrics in form_data with stored values
+    for key in ["metrics", "columns"]:

Review Comment:
   I think some viz types might have the columns listed as `groupby` in the 
`form_data`:
   
   
![image](https://github.com/apache/superset/assets/96086495/3d74e982-69e4-4efc-b679-6b0b04b2b677)



-- 
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