Skip to content

fix(agent-job-notification): Add notification for deactivate site job - #6245

Merged
balamurali27 merged 3 commits into
developfrom
deactivate-site-notif
Apr 27, 2026
Merged

fix(agent-job-notification): Add notification for deactivate site job#6245
balamurali27 merged 3 commits into
developfrom
deactivate-site-notif

Conversation

@balamurali27

@balamurali27 balamurali27 commented Apr 24, 2026

Copy link
Copy Markdown
Contributor

Physical backup site updates have separate deactivate site job

Also update broken doc links

Physical backup site updates have separate deactivate site job
@balamurali27
balamurali27 requested a review from ssiyad as a code owner April 24, 2026 08:36
@greptile-apps

greptile-apps Bot commented Apr 24, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR extends update_with_rq_jobs_in_queue_err to handle the "Deactivate Site" agent job type and fixes two stale documentation URL anchors (GZIP_TAR_ERR, RQ_JOBS_IN_QUEUE). The core logic is straightforward, but the shared notification message now instructs users to "deactivate your site" even when the failing job itself is "Deactivate Site", which is confusing — the help link also points to deactivation instructions that are already being followed.

Confidence Score: 4/5

Safe to merge with a minor UX concern; no runtime errors or data loss risk.

The only finding is P2: the notification message body becomes self-referential when the failing job type is "Deactivate Site", which is confusing but does not break functionality or cause data loss. The URL anchor fixes are correct improvements. Score is 4 rather than 5 because the misleading message is a correctness-of-communication concern worth addressing before the feature ships.

press/press/doctype/agent_job/agent_job_notifications.py — the update_with_rq_jobs_in_queue_err message body needs a branch for the "Deactivate Site" case.

Important Files Changed

Filename Overview
press/press/doctype/agent_job/agent_job_notifications.py Adds "Deactivate Site" job type to the RQ-jobs-in-queue error handler; fixes two documentation URL anchors; slightly rewrites wait-time wording — but the existing notification message becomes self-referential when the failing job is "Deactivate Site".

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Agent Job Fails] --> B{Match string found?\n'Site might have lot\nof jobs in queue.'}
    B -- No --> C[Other handlers / default notification]
    B -- Yes --> D{job.job_type in\nallowed list?}
    D -- No --> C
    D -- Yes --> E[Set title: 'High number of queued jobs']
    E --> F[Set shared message body\n'please deactivate your site...']
    F --> G{job.job_type ==\n'Deactivate Site'?}
    G -- Yes --> H["⚠️ Message says 'deactivate site'\nbut user is already doing that"]
    G -- No --> I[Notification shown\nUpdate Site Pull / Update Site Migrate]
    H --> J[assistance_url = 'how-do-i-deactivate-my-site'\nalso self-referential]
Loading

Comments Outside Diff (1)

  1. press/press/doctype/agent_job/agent_job_notifications.py, line 334-340 (link)

    P2 Circular/confusing message for "Deactivate Site" job type

    When the failing job is "Deactivate Site", the notification message still tells the user to "please deactivate your site" — but they are already trying to do exactly that. The instruction and the assistance_url (pointing to "how do I deactivate my site") become self-referentially misleading. Consider branching the message body for this job type to give a different remediation step (e.g., "clear queued jobs manually and retry") instead of advising deactivation.

Reviews (1): Last reviewed commit: "fix(agent-job-notification): Add notific..." | Re-trigger Greptile

@codecov

codecov Bot commented Apr 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 56.25%. Comparing base (b9bfdc5) to head (c4e5113).
⚠️ Report is 22 commits behind head on develop.

Files with missing lines Patch % Lines
...press/doctype/agent_job/agent_job_notifications.py 0.00% 1 Missing ⚠️

❌ Your patch status has failed because the patch coverage (0.00%) is below the target coverage (75.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #6245      +/-   ##
===========================================
+ Coverage    49.53%   56.25%   +6.72%     
===========================================
  Files          935      935              
  Lines        77596    77596              
  Branches       353      525     +172     
===========================================
+ Hits         38434    43649    +5215     
+ Misses       39138    33919    -5219     
- Partials        24       28       +4     
Flag Coverage Δ
dashboard 90.74% <ø> (+30.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@balamurali27
balamurali27 enabled auto-merge April 27, 2026 03:27
@balamurali27
balamurali27 merged commit 754cc87 into develop Apr 27, 2026
14 of 15 checks passed
@balamurali27
balamurali27 deleted the deactivate-site-notif branch April 27, 2026 05:26
@frappe-pr-bot

Copy link
Copy Markdown
Collaborator

🎉 This PR is included in version 0.23.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants