New feat: SAP packaging wizard #67
Conversation
github-actions
bot
added
documentation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughThis PR adds SAP Management functionality to SwitchCraft, enabling guided lifecycle management of SAP Installation Servers. It introduces a new multi-step wizard UI, backend service for tool detection and server customization, complete localization support, and documentation. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SapWizardView as SAP Wizard UI
participant SapService as SAP Service
participant FileSystem as File System
participant ShellUtils as Shell Utils
participant AdminTool as NwSapSetupAdmin.exe
User->>SapWizardView: Select server path (Step 1)
SapWizardView->>SapService: detect_admin_tool(server_path)
SapService->>FileSystem: Locate NwSapSetupAdmin.exe
FileSystem-->>SapService: Tool path
User->>SapWizardView: Add update files (Step 2)
SapWizardView->>SapService: merge_update(server_path, update_exe)
SapService->>ShellUtils: run_command(/UpdateServer)
ShellUtils->>AdminTool: Execute merge
AdminTool-->>ShellUtils: Success
User->>SapWizardView: Configure WebView2 & logo (Step 3)
SapWizardView->>SapService: customize_server(server_path, logo_path, use_webview2)
SapService->>FileSystem: Deploy logo (optional)
SapService->>FileSystem: Patch SapGuiSetup.xml
FileSystem-->>SapService: XML updated
User->>SapWizardView: Finalize & create SFU (Step 4)
SapWizardView->>SapService: create_single_file_installer(server_path, package_name, output_path)
SapService->>ShellUtils: run_command(/CreateSFU)
ShellUtils->>AdminTool: Execute SFU creation
AdminTool-->>ShellUtils: Installer path
ShellUtils-->>SapService: Output path
SapService-->>SapWizardView: Success
SapWizardView-->>User: Show result & completion
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/switchcraft/gui_modern/app.py (1)
236-260: Add SAP_WIZARD to the help URL mapping.The
_get_help_urlmapping doesn't includeNavIndex.SAP_WIZARD. When users click the help button from the SAP wizard, they'll be directed to the root documentation page instead of the SAP wizard docs.Proposed fix
NavIndex.EXCHANGE: "views/exchange", - NavIndex.SETTINGS_POLICIES: "views/policies" + NavIndex.SETTINGS_POLICIES: "views/policies", + NavIndex.SAP_WIZARD: "views/sap_wizard" }
🤖 Fix all issues with AI agents
In `@docs/SAP.md`:
- Around line 7-9: Fix the grammar in the "Logo Customization" bullet: replace
the phrase "by integrated them into the installation server" with "by
integrating them into the installation server" in the Logo Customization bullet
text so it reads correctly; locate and update the line that currently starts
"**Logo Customization**: Automatically distribute custom company logos..." and
apply the single-word change.
In `@docs/views/sap_wizard.md`:
- Line 5: The markdown references a missing image and uses the wrong VitePress
path: replace the existing reference string "! [SAP Wizard
Screenshot](/docs/public/img/sap_wizard.png)" by adding the actual
sap_wizard.png into docs/public/img/ (or your chosen docs public asset folder)
and update the markdown to use the VitePress-served path "/img/sap_wizard.png"
(i.e., change the image link to ""); ensure the file name matches exactly
(sap_wizard.png) and commit both the image and the updated
docs/views/sap_wizard.md line.
In `@src/switchcraft/assets/lang/de.json`:
- Around line 1098-1117: Several newly added localization keys (e.g.,
"sap_wizard_title", "sap_card_title", "sap_card_desc", "sap_step1_title"…
"btn_select_logo", "btn_apply_build") contain English or mixed-language strings;
replace them with fully German translations to ensure a consistent localized
UX—for example translate "SAP Management Wizard" and "Custom Logo wählen" into
proper German, and update all "sap_stepX_*", "sap_use_webview2",
"no_logo_selected", "label_*", and "btn_*" values to idiomatic German while
keeping the key names unchanged so callers continue to work.
In `@src/switchcraft/gui_modern/views/sap_wizard_view.py`:
- Around line 51-70: The FilePicker is being appended to self.app_page.overlay
each time _build_step_1 runs, causing duplicates; fix by creating a single
FilePicker once (e.g., initialize self.server_picker =
ft.FilePicker(on_result=...) in __init__) and replace local fp usage in
_build_step_1 with self.server_picker (append it to self.app_page.overlay only
once or ensure you remove any existing picker before appending). Ensure the
on_result handler updates self.server_path and path_text the same way and that
_build_step_1 references self.server_picker.get_directory_path() in the
ElevatedButton callback.
🧹 Nitpick comments (9)
tests/test_sap_service.py (1)
2-2: Remove unused import.
mock_openis imported but never used in the tests.Proposed fix
-from unittest.mock import MagicMock, patch, mock_open +from unittest.mock import MagicMock, patchsrc/switchcraft/gui_modern/views/sap_wizard_view.py (5)
3-4: Remove unused imports.
osandPathare imported but not used in this file.Proposed fix
import flet as ft import logging -import os -from pathlib import Path from switchcraft.gui_modern.utils.view_utils import ViewMixin
72-79: Step 2 update selection is not implemented.The "Add Update EXE" button shows a stub message and
self.update_filesis never populated. Consider adding a TODO comment or tracking this for follow-up.Do you want me to open an issue to track implementing the update file selection functionality?
81-90: Logo selection button has no handler.The "Select Custom Logo" button (line 87) lacks an
on_clickhandler, soself.logo_pathwill never be set. The step 4 summary will always show "Custom Logo: No".Do you want me to generate the logo picker implementation?
103-107: Consider disabling navigation buttons at boundaries.The Back/Next buttons remain enabled but do nothing when at step 1 or 4 respectively. Disabling them would provide clearer UX feedback.
109-119: Use i18n for user-facing messages.The error and success messages on lines 111 and 116 use hardcoded English strings. Consider using
i18n.get()for consistency with the rest of the wizard.Proposed fix
def _on_finalize(self, _): if not self.server_path: - self._show_snack("Please select a server path first.", color="RED") + self._show_snack(i18n.get("sap_error_no_path") or "Please select a server path first.", color="RED") return try: self.sap_service.customize_server(self.server_path, self.logo_path, self.use_webview2) - self._show_snack("SAP Server customized successfully!", color="GREEN") + self._show_snack(i18n.get("sap_success_customized") or "SAP Server customized successfully!", color="GREEN")src/switchcraft/services/sap_service.py (3)
1-8: Remove unused imports.
subprocess,List, andi18nare imported but never used.Proposed fix
import os import logging -import subprocess from pathlib import Path -from typing import List, Optional +from typing import Optional from switchcraft.utils.shell_utils import ShellUtils -from switchcraft.utils.i18n import i18n import xml.etree.ElementTree as ET
67-75: Moveimport shutilto top of file.The
shutilimport on line 70 is inside the method body. Move it to the top-level imports for consistency and to avoid repeated import overhead.Proposed fix
At top of file:
import os import logging from pathlib import Path from typing import Optional from switchcraft.utils.shell_utils import ShellUtils import xml.etree.ElementTree as ET +import shutilIn method:
if logo_path and os.path.exists(logo_path): target_logo = setup_dir / "custom_logo.png" - import shutil shutil.copy(logo_path, target_logo)
100-124: Consider verifying output file exists after creation.The method returns
output_pathbased solely on the command's return code. If the tool returns 0 but fails to create the file, callers may receive an invalid path.Proposed enhancement
result = ShellUtils.run_command(cmd) if result and result.returncode == 0: + if not Path(output_path).exists(): + logger.warning(f"SFU creation returned success but file not found: {output_path}") return output_path
| - **Automated Update Merging**: Easily integrate SAP GUI patches and add-ons using the `/UpdateServer` command-line interface. | ||
| - **Logo Customization**: Automatically distribute custom company logos to all clients by integrated them into the installation server. | ||
| - **Edge WebView2 Enforcement**: Force the use of Edge WebView2 as the default browser engine for SAP GUI transactions by patching the `SapGuiSetup.xml` configuration. |
There was a problem hiding this comment.
Minor grammar fix in Logo Customization bullet.
“by integrated them” → “by integrating them”.
✏️ Suggested edit
-- **Logo Customization**: Automatically distribute custom company logos to all clients by integrated them into the installation server.
+- **Logo Customization**: Automatically distribute custom company logos to all clients by integrating them into the installation server.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - **Automated Update Merging**: Easily integrate SAP GUI patches and add-ons using the `/UpdateServer` command-line interface. | |
| - **Logo Customization**: Automatically distribute custom company logos to all clients by integrated them into the installation server. | |
| - **Edge WebView2 Enforcement**: Force the use of Edge WebView2 as the default browser engine for SAP GUI transactions by patching the `SapGuiSetup.xml` configuration. | |
| - **Automated Update Merging**: Easily integrate SAP GUI patches and add-ons using the `/UpdateServer` command-line interface. | |
| - **Logo Customization**: Automatically distribute custom company logos to all clients by integrating them into the installation server. | |
| - **Edge WebView2 Enforcement**: Force the use of Edge WebView2 as the default browser engine for SAP GUI transactions by patching the `SapGuiSetup.xml` configuration. |
🤖 Prompt for AI Agents
In `@docs/SAP.md` around lines 7 - 9, Fix the grammar in the "Logo Customization"
bullet: replace the phrase "by integrated them into the installation server"
with "by integrating them into the installation server" in the Logo
Customization bullet text so it reads correctly; locate and update the line that
currently starts "**Logo Customization**: Automatically distribute custom
company logos..." and apply the single-word change.
docs/views/sap_wizard.md
Outdated
|
|
||
| The **SAP Management Wizard** is a guided tool designed to simplify the lifecycle management of SAP Installation Servers. | ||
|
|
||
|  |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if the image file exists
find docs -name "sap_wizard.png" -type f 2>/dev/null
echo "---"
# List the structure of docs/public if it exists
if [ -d "docs/public" ]; then
echo "docs/public directory structure:"
find docs/public -type f 2>/dev/null | head -20
else
echo "docs/public directory does not exist"
fi
echo "---"
# List the docs/views directory to confirm the file exists
echo "docs/views directory contents:"
ls -la docs/views/ 2>/dev/null || echo "docs/views not accessible"Repository: FaserF/SwitchCraft
Length of output: 1830
🏁 Script executed:
#!/bin/bash
# Check for VitePress config to understand path conventions
find . -maxdepth 2 -name "*vitepress*" -o -name "*.config.*" | grep -E "(vitepress|config)" | head -10
echo "---"
# Look for other image references in docs to see the pattern
rg "!\[.*\]\(/.*\.png" docs/ --type md | head -10Repository: FaserF/SwitchCraft
Length of output: 166
🏁 Script executed:
#!/bin/bash
# Check the exact content of the sap_wizard.md file
cat -n docs/views/sap_wizard.md 2>/dev/null || echo "File not found"Repository: FaserF/SwitchCraft
Length of output: 1211
The image file is missing from the repository and the path format is incorrect.
The sap_wizard.png file does not exist in docs/public/ or anywhere else in the codebase. Additionally, the path format is incorrect for VitePress—docs/public/ assets are served from the root, not as absolute paths including /docs/public/.
To resolve this:
- Add the
sap_wizard.pngimage file todocs/public/img/(or adjust the path if using a different location) - Update the markdown reference to
/img/sap_wizard.png
Proposed fix
-
+📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
|  | |
|  |
🤖 Prompt for AI Agents
In `@docs/views/sap_wizard.md` at line 5, The markdown references a missing image
and uses the wrong VitePress path: replace the existing reference string "! [SAP
Wizard Screenshot](/docs/public/img/sap_wizard.png)" by adding the actual
sap_wizard.png into docs/public/img/ (or your chosen docs public asset folder)
and update the markdown to use the VitePress-served path "/img/sap_wizard.png"
(i.e., change the image link to ""); ensure the file name matches exactly
(sap_wizard.png) and commit both the image and the updated
docs/views/sap_wizard.md line.
| "wine_missing_hint": "Diese Funktion erfordert eine Wine-Installation auf deinem Linux-System.", | ||
| "sap_wizard_title": "SAP Management Wizard", | ||
| "sap_card_title": "SAP Management", | ||
| "sap_card_desc": "SAP Install Server Werkzeug", | ||
| "sap_step1_title": "1. SAP Installations-Server wählen", | ||
| "sap_step1_desc": "Wähle den Stammordner deines SAP nwsetupadmin Servers aus.", | ||
| "sap_step2_title": "2. Updates & Add-ons hinzufügen (Optional)", | ||
| "sap_step2_desc": "Wähle .exe Dateien aus, die in den Installations-Server integriert werden sollen.", | ||
| "sap_step3_title": "3. Anpassung", | ||
| "sap_step3_desc": "Konfiguriere Logo und Browser-Einstellungen.", | ||
| "sap_step4_title": "4. Zusammenfassung & Paketierung", | ||
| "sap_use_webview2": "Edge WebView2 als Standard (Empfohlen)", | ||
| "no_logo_selected": "Kein Logo ausgewählt", | ||
| "label_server": "Server", | ||
| "label_custom_logo": "Benutzerdefiniertes Logo", | ||
| "label_webview2": "Edge WebView2", | ||
| "btn_browse_folder": "Serverordner durchsuchen", | ||
| "btn_add_update": "Update EXE hinzufügen", | ||
| "btn_select_logo": "Custom Logo wählen", | ||
| "btn_apply_build": "Anwenden & Paketierung erstellen" |
There was a problem hiding this comment.
Consider fully localizing the new SAP strings.
Several new values are English or mixed (e.g., “SAP Management Wizard”, “Custom Logo wählen”). If this is intentional, ignore; otherwise, a consistent German localization would improve UX.
💬 Optional wording adjustments
- "sap_wizard_title": "SAP Management Wizard",
+ "sap_wizard_title": "SAP-Verwaltungsassistent",
...
- "btn_select_logo": "Custom Logo wählen",
+ "btn_select_logo": "Benutzerdefiniertes Logo wählen",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "wine_missing_hint": "Diese Funktion erfordert eine Wine-Installation auf deinem Linux-System.", | |
| "sap_wizard_title": "SAP Management Wizard", | |
| "sap_card_title": "SAP Management", | |
| "sap_card_desc": "SAP Install Server Werkzeug", | |
| "sap_step1_title": "1. SAP Installations-Server wählen", | |
| "sap_step1_desc": "Wähle den Stammordner deines SAP nwsetupadmin Servers aus.", | |
| "sap_step2_title": "2. Updates & Add-ons hinzufügen (Optional)", | |
| "sap_step2_desc": "Wähle .exe Dateien aus, die in den Installations-Server integriert werden sollen.", | |
| "sap_step3_title": "3. Anpassung", | |
| "sap_step3_desc": "Konfiguriere Logo und Browser-Einstellungen.", | |
| "sap_step4_title": "4. Zusammenfassung & Paketierung", | |
| "sap_use_webview2": "Edge WebView2 als Standard (Empfohlen)", | |
| "no_logo_selected": "Kein Logo ausgewählt", | |
| "label_server": "Server", | |
| "label_custom_logo": "Benutzerdefiniertes Logo", | |
| "label_webview2": "Edge WebView2", | |
| "btn_browse_folder": "Serverordner durchsuchen", | |
| "btn_add_update": "Update EXE hinzufügen", | |
| "btn_select_logo": "Custom Logo wählen", | |
| "btn_apply_build": "Anwenden & Paketierung erstellen" | |
| "wine_missing_hint": "Diese Funktion erfordert eine Wine-Installation auf deinem Linux-System.", | |
| "sap_wizard_title": "SAP-Verwaltungsassistent", | |
| "sap_card_title": "SAP Management", | |
| "sap_card_desc": "SAP Install Server Werkzeug", | |
| "sap_step1_title": "1. SAP Installations-Server wählen", | |
| "sap_step1_desc": "Wähle den Stammordner deines SAP nwsetupadmin Servers aus.", | |
| "sap_step2_title": "2. Updates & Add-ons hinzufügen (Optional)", | |
| "sap_step2_desc": "Wähle .exe Dateien aus, die in den Installations-Server integriert werden sollen.", | |
| "sap_step3_title": "3. Anpassung", | |
| "sap_step3_desc": "Konfiguriere Logo und Browser-Einstellungen.", | |
| "sap_step4_title": "4. Zusammenfassung & Paketierung", | |
| "sap_use_webview2": "Edge WebView2 als Standard (Empfohlen)", | |
| "no_logo_selected": "Kein Logo ausgewählt", | |
| "label_server": "Server", | |
| "label_custom_logo": "Benutzerdefiniertes Logo", | |
| "label_webview2": "Edge WebView2", | |
| "btn_browse_folder": "Serverordner durchsuchen", | |
| "btn_add_update": "Update EXE hinzufügen", | |
| "btn_select_logo": "Benutzerdefiniertes Logo wählen", | |
| "btn_apply_build": "Anwenden & Paketierung erstellen" |
🤖 Prompt for AI Agents
In `@src/switchcraft/assets/lang/de.json` around lines 1098 - 1117, Several newly
added localization keys (e.g., "sap_wizard_title", "sap_card_title",
"sap_card_desc", "sap_step1_title"… "btn_select_logo", "btn_apply_build")
contain English or mixed-language strings; replace them with fully German
translations to ensure a consistent localized UX—for example translate "SAP
Management Wizard" and "Custom Logo wählen" into proper German, and update all
"sap_stepX_*", "sap_use_webview2", "no_logo_selected", "label_*", and "btn_*"
values to idiomatic German while keeping the key names unchanged so callers
continue to work.
| def _build_step_1(self): | ||
| """Step 1: Select SAP Installation Server path.""" | ||
| def on_pick_server(e: ft.FilePickerResultEvent): | ||
| if e.path: | ||
| self.server_path = e.path | ||
| path_text.value = e.path | ||
| self.update() | ||
|
|
||
| path_text = ft.Text(self.server_path or "No path selected", italic=True) | ||
| fp = ft.FilePicker(on_result=on_pick_server) | ||
| self.app_page.overlay.append(fp) | ||
|
|
||
| return ft.Column([ | ||
| ft.Text(i18n.get("sap_step1_title") or "1. Select SAP Installation Server", size=18, weight=ft.FontWeight.BOLD), | ||
| ft.Text(i18n.get("sap_step1_desc") or "Point to the root folder of your SAP nwsetupadmin server."), | ||
| ft.Row([ | ||
| ft.ElevatedButton(i18n.get("btn_browse_folder") or "Browse Server Folder", icon=ft.Icons.FOLDER_OPEN, on_click=lambda _: fp.get_directory_path()), | ||
| path_text | ||
| ]) | ||
| ]) |
There was a problem hiding this comment.
FilePicker accumulates in overlay on repeated navigation.
Each call to _build_step_1 appends a new FilePicker to self.app_page.overlay. When users navigate back to step 1, this creates duplicate pickers. Consider initializing the FilePicker once in __init__ or clearing the previous one before adding.
Proposed fix - initialize FilePicker in __init__
def __init__(self, page: ft.Page):
super().__init__(expand=True, scroll=ft.ScrollMode.AUTO)
self.app_page = page
self.sap_service = SapService()
# State
self.server_path = ""
self.update_files = []
self.logo_path = ""
self.use_webview2 = True
+ # FilePicker for server selection
+ self.server_picker = ft.FilePicker(on_result=self._on_pick_server)
+ self.app_page.overlay.append(self.server_picker)
+
self.current_step = 1Then update _build_step_1 to use self.server_picker instead of creating a new one.
🤖 Prompt for AI Agents
In `@src/switchcraft/gui_modern/views/sap_wizard_view.py` around lines 51 - 70,
The FilePicker is being appended to self.app_page.overlay each time
_build_step_1 runs, causing duplicates; fix by creating a single FilePicker once
(e.g., initialize self.server_picker = ft.FilePicker(on_result=...) in __init__)
and replace local fp usage in _build_step_1 with self.server_picker (append it
to self.app_page.overlay only once or ensure you remove any existing picker
before appending). Ensure the on_result handler updates self.server_path and
path_text the same way and that _build_step_1 references
self.server_picker.get_directory_path() in the ElevatedButton callback.
1 participant
Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.