codeant-ai-for-open-source[bot] commented on code in PR #37037:
URL: https://github.com/apache/superset/pull/37037#discussion_r2680521761


##########
superset-frontend/src/features/alerts/AlertReportModal.tsx:
##########
@@ -502,7 +502,7 @@ const AlertReportModal: 
FunctionComponent<AlertReportModalProps> = ({
 
   const [isScreenshot, setIsScreenshot] = useState<boolean>(false);
   useEffect(() => {
-    setIsScreenshot(reportFormat === 'PNG');
+    setIsScreenshot(reportFormat === 'PNG' || reportFormat === 'PDF');

Review Comment:
   **Suggestion:** The comparison is case-sensitive and will fail to detect 
formats if `reportFormat` comes from the backend or elsewhere in lowercase or 
with surrounding whitespace (e.g. 'pdf', ' Pdf '). Normalize and trim 
`reportFormat` before comparing to avoid missing PDF/PNG values. [logic error]
   
   **Severity Level:** Minor ⚠️
   ```suggestion
       const fmt = String(reportFormat || '').trim().toUpperCase();
       setIsScreenshot(fmt === 'PNG' || fmt === 'PDF');
   ```
   <details>
   <summary><b>Why it matters? ⭐ </b></summary>
   
   Normalizing/trimming the format string before comparing fixes a real, 
low-risk logic hole: if reportFormat ever contains extra whitespace or 
lowercase values (e.g. 'pdf', ' Pdf '), the existing strict comparison will 
incorrectly return false. The improved snippet is small, correct, and safe in 
this component's context.
   </details>
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** superset-frontend/src/features/alerts/AlertReportModal.tsx
   **Line:** 505:505
   **Comment:**
        *Logic Error: The comparison is case-sensitive and will fail to detect 
formats if `reportFormat` comes from the backend or elsewhere in lowercase or 
with surrounding whitespace (e.g. 'pdf', ' Pdf '). Normalize and trim 
`reportFormat` before comparing to avoid missing PDF/PNG values.
   
   Validate the correctness of the flagged issue. If correct, How can I resolve 
this? If you propose a fix, implement it and please make it concise.
   ```
   </details>



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