feat: add check and edit options to backup service menu#6
feat: add check and edit options to backup service menu#6ImMohammad20000 merged 3 commits intoPasarGuard:mainfrom x0sina:main
Conversation
WalkthroughThe pasarguard.sh backup_service was replaced with an interactive looping menu (six options). Two new public functions—view_backup_service() and edit_backup_service()—were added. Cron/cron-entry handling was expanded to support daily and every-N-hours schedules and includes removal and rewrite logic tied to .env updates. Changes
Sequence DiagramsequenceDiagram
actor User
participant Menu as "Backup Service Menu"
participant View as "view_backup_service()"
participant Edit as "edit_backup_service()"
participant EnvCron as "Env (.env) & Cron"
User->>Menu: Select option
alt Check Backup Service
Menu->>View: invoke
View->>EnvCron: read .env and cron entries
EnvCron-->>View: return config
View-->>User: show current settings
else Edit Backup Service
Menu->>Edit: invoke
Edit->>User: prompt for bot key / chat ID / interval
User-->>Edit: provide inputs
Edit->>Edit: validate inputs
alt valid
Edit->>EnvCron: update .env (BACKUP_ vars)
Edit->>EnvCron: remove existing tagged cron entry
Edit->>EnvCron: write new tagged cron entry
EnvCron-->>Edit: confirm
Edit-->>User: confirm update
else invalid
Edit-->>User: show error, retry or cancel
end
else Reconfigure / Remove
Menu->>EnvCron: remove cron entry and clear env keys
EnvCron-->>Menu: confirm removal
Menu-->>User: confirm done
else Instant Backup
Menu->>EnvCron: trigger immediate backup action
EnvCron-->>Menu: result
Menu-->>User: show outcome
end
Menu-->>User: return to menu (loop)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pasarguard.sh (1)
462-477: Use comment tag for consistency in add_cron_job() to prevent orphaned cron entries.The inconsistency is confirmed.
add_cron_job()at line 469 removes entries via text matching (grep -v "$command"), whileedit_backup_service()at line 594 uses the comment tag (grep -v "# pasarguard-backup-service"). Since both functions add the same comment tag, they should both filter by it to ensure reliable removal regardless of command parameter changes.Update line 469 in
add_cron_job()to use the comment tag:- grep -v "$command" "$temp_cron" >"${temp_cron}.tmp" && mv "${temp_cron}.tmp" "$temp_cron" + grep -v "# pasarguard-backup-service" "$temp_cron" >"${temp_cron}.tmp" && mv "${temp_cron}.tmp" "$temp_cron"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pasarguard.sh(2 hunks)
🧰 Additional context used
🪛 Shellcheck (0.11.0)
pasarguard.sh
[error] 569-569: Couldn't parse this while loop. Fix to allow more checks.
(SC1073)
[error] 569-569: Couldn't find 'done' for this 'do'.
(SC1061)
[error] 605-605: Expected 'done' matching previously mentioned 'do'.
(SC1062)
[error] 605-605: Unexpected keyword/token. Fix any mentioned problems and try again.
(SC1072)
🔇 Additional comments (3)
pasarguard.sh (3)
325-385: Approve menu-driven backup service flow.The refactored
backup_service()function provides a well-structured menu experience with clear option handling. The loop-and-break pattern for reconfiguration (option 3) is intuitive. Sub-function delegation toview_backup_service()andedit_backup_service()improves maintainability.
479-511: LGTM – view_backup_service() implementation.Function correctly extracts and displays current configuration with proper interval parsing (daily vs. hourly). The user pause at line 510 is a good UX touch.
513-567: Approve edit_backup_service() case branches 1 & 2 (once syntax is fixed).The Telegram key and chat ID edit branches validate input and update the .env file correctly using
sed -iwith pipe escaping.Once the
donestatement is added at line 604, verify that the cron update logic in case 3 works end-to-end:.envupdate, crontab rewrite, and proper error feedback. Also test with both 24-hour and N-hour intervals to confirm both cron patterns generate correctly.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
pasarguard.sh (3)
485-487: Optional: Declare variables before assignment to separate concerns.Lines 485–487 combine declaration and assignment, which can mask return values. Consider declaring first:
- local telegram_bot_key=$(awk -F'=' '/^BACKUP_TELEGRAM_BOT_KEY=/ {print $2}' "$ENV_FILE") - local telegram_chat_id=$(awk -F'=' '/^BACKUP_TELEGRAM_CHAT_ID=/ {print $2}' "$ENV_FILE") - local cron_schedule=$(awk -F'=' '/^BACKUP_CRON_SCHEDULE=/ {print $2}' "$ENV_FILE" | tr -d '"') + local telegram_bot_key + telegram_bot_key=$(awk -F'=' '/^BACKUP_TELEGRAM_BOT_KEY=/ {print $2}' "$ENV_FILE") + local telegram_chat_id + telegram_chat_id=$(awk -F'=' '/^BACKUP_TELEGRAM_CHAT_ID=/ {print $2}' "$ENV_FILE") + local cron_schedule + cron_schedule=$(awk -F'=' '/^BACKUP_CRON_SCHEDULE=/ {print $2}' "$ENV_FILE" | tr -d '"')This aligns with the pattern suggested by Shellcheck SC2155.
519-521: Optional: Declare variables before assignment (consistent with view_backup_service).Apply the same refactoring here:
- local telegram_bot_key=$(awk -F'=' '/^BACKUP_TELEGRAM_BOT_KEY=/ {print $2}' "$ENV_FILE") - local telegram_chat_id=$(awk -F'=' '/^BACKUP_TELEGRAM_CHAT_ID=/ {print $2}' "$ENV_FILE") - local cron_schedule=$(awk -F'=' '/^BACKUP_CRON_SCHEDULE=/ {print $2}' "$ENV_FILE" | tr -d '"') + local telegram_bot_key + telegram_bot_key=$(awk -F'=' '/^BACKUP_TELEGRAM_BOT_KEY=/ {print $2}' "$ENV_FILE") + local telegram_chat_id + telegram_chat_id=$(awk -F'=' '/^BACKUP_TELEGRAM_CHAT_ID=/ {print $2}' "$ENV_FILE") + local cron_schedule + cron_schedule=$(awk -F'=' '/^BACKUP_CRON_SCHEDULE=/ {print $2}' "$ENV_FILE" | tr -d '"')
592-593: Optional: Declare command substitution variables separately.Lines 592–593 should follow the SC2155 pattern:
- local backup_command="$(which bash) -c '$APP_NAME backup'" - local temp_cron=$(mktemp) + local backup_command + backup_command="$(which bash) -c '$APP_NAME backup'" + local temp_cron + temp_cron=$(mktemp)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pasarguard.sh(2 hunks)
🧰 Additional context used
🪛 Shellcheck (0.11.0)
pasarguard.sh
[warning] 485-485: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 486-486: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 487-487: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 519-519: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 520-520: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 521-521: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 592-592: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 593-593: Declare and assign separately to avoid masking return values.
(SC2155)
🔇 Additional comments (3)
pasarguard.sh (3)
325-385: Syntax issue resolved; menu structure looks solid.The
donestatement at line 385 correctly closes thewhile true; doloop, fixing the previous shell parsing error. The menu routing is clean—options 1, 2, 5 stay in the loop, options 3–4 exit appropriately, and option 6 returns. Option 3'sbreakflows into the initial setup flow (lines 390+) for reconfiguration, which makes sense.
569-606: Cron update logic is sound; previous syntax error is resolved.The
donestatement at line 606 now correctly closes thewhile true; doloop at line 569. The case option 3 structure is correct:
- Input validation loop (lines 569–605)
breakat line 604 exits the loopdoneat line 606 closes it;;at line 607 closes the case blockThe cron-update pattern (lines 595–596) is also correct:
- Line 595 removes old entries by comment tag
# pasarguard-backup-service- Line 596 appends the new schedule with the same tag
This ensures clean updates without duplicates.
386-459: Control flow after menu is correct.The conditional at line 324 gates the menu: if service is enabled, users see options. Option 3 (reconfigure) breaks the loop and falls through to lines 390–459 to re-run setup. If the service is not enabled initially (line 386), the code shows the message and then runs setup. The flow is logical and handles the reconfigure path well.
2 participants
Summary by CodeRabbit