WARNING: THIS SITE IS A MIRROR OF GITHUB.COM / IT CANNOT LOGIN OR REGISTER ACCOUNTS / THE CONTENTS ARE PROVIDED AS-IS / THIS SITE ASSUMES NO RESPONSIBILITY FOR ANY DISPLAYED CONTENT OR LINKS / IF YOU FOUND SOMETHING MAY NOT GOOD FOR EVERYONE, CONTACT ADMIN AT ilovescratch@foxmail.com
Skip to content

fix: Mark site update doc as cancelled when cancelling an update rather than deleting it #4123

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
phot0n merged 4 commits into from
Dec 8, 2025
+56 −25
Merged

fix: Mark site update doc as cancelled when cancelling an update rather than deleting it #4123

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
78c6364
fix: mark site update doc as cancelled when cancelling an update rath…
phot0n Dec 6, 2025
132d6da
chore: remove a db call in deactivate_site in site update controller
phot0n Dec 6, 2025
07182cc
fix: use document api to respect doc permissions
phot0n Dec 7, 2025
9cdf8f8
fix: Dont show view job action in dashboard for cancelled site update
phot0n Dec 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion dashboard/src/objects/site.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1336,7 +1336,8 @@ export default {
},
{
label: 'View Job',
condition: () => row.status !== 'Scheduled',
condition: () =>
!['Scheduled', 'Cancelled'].includes(row.status),
onClick() {
router.push({
name: 'Site Update',
Expand Down
18 changes: 14 additions & 4 deletions press/press/doctype/site/site.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1274,11 +1274,21 @@ def edit_scheduled_update(

@dashboard_whitelist()
def cancel_scheduled_update(self, site_update: str):
if status := frappe.db.get_value("Site Update", site_update, "status") != "Scheduled":
frappe.throw(f"Cannot cancel a Site Update with status {status}")
try:
if (
_status := frappe.db.get_value(
"Site Update", site_update, "status", for_update=True, wait=False
)
) != "Scheduled":
frappe.throw(f"Cannot cancel a Site Update with status {_status}")

# TODO: Set status to cancelled instead of deleting the doc
frappe.delete_doc("Site Update", site_update)
except (frappe.QueryTimeoutError, frappe.QueryDeadlockError):
frappe.throw("The update is probably underway. Please reload/refresh to get the latest status.")

# used document api for applying doc permissions
doc = frappe.get_doc("Site Update", site_update)
Copy link
Member Author

@phot0n phot0n Dec 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no option of nowait in this plus we need to respect perms as well (user facing)

doc.status = "Cancelled"
doc.save()

@frappe.whitelist()
def move_to_group(self, group, skip_failing_patches=False, skip_backups=False):
Expand Down
18 changes: 6 additions & 12 deletions press/press/doctype/site_update/site_update.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,6 @@
// For license information, please see license.txt

frappe.ui.form.on('Site Update', {
onload: function (frm) {
frm.set_query('destination_bench', function () {
return {
filters: {
status: 'Active',
server: frm.doc.server,
},
};
});
},

Comment on lines -5 to -15
Copy link
Member Author

@phot0n phot0n Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

site update is a "log" - not sure why this was needed

refresh: function (frm) {
// Disable save button
frm.disable_save();
Expand All @@ -23,6 +12,8 @@ frappe.ui.form.on('Site Update', {
__('Visit Dashboard'),
);

if (frm.doc.status === 'Cancelled') return;

// Add custom buttons
[
[
Expand Down Expand Up @@ -55,14 +46,17 @@ frappe.ui.form.on('Site Update', {
frm.add_custom_button(
__('Change Status'),
() => {
let options = ['Success', 'Recovered', 'Failure', 'Fatal'];
frm.doc.status === 'Scheduled' ? options.push('Cancelled') : null;

const dialog = new frappe.ui.Dialog({
title: __('Change Status'),
fields: [
{
fieldtype: 'Select',
label: __('Status'),
fieldname: 'status',
options: ['Success', 'Recovered', 'Failure', 'Fatal'],
options: options,
},
],
});
Expand Down
4 changes: 2 additions & 2 deletions press/press/doctype/site_update/site_update.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@
"in_standard_filter": 1,
"label": "Status",
"no_copy": 1,
"options": "Pending\nRunning\nSuccess\nFailure\nRecovering\nRecovered\nFatal\nScheduled"
"options": "Pending\nRunning\nSuccess\nFailure\nRecovering\nRecovered\nFatal\nScheduled\nCancelled"
},
{
"fieldname": "destination_bench",
Expand Down Expand Up @@ -342,7 +342,7 @@
],
"index_web_pages_for_search": 1,
"links": [],
"modified": "2025-08-21 15:58:45.863368",
"modified": "2025-12-06 21:13:57.822331",
"modified_by": "Administrator",
"module": "Press",
"name": "Site Update",
Expand Down
38 changes: 32 additions & 6 deletions press/press/doctype/site_update/site_update.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,15 @@ class SiteUpdate(Document):
source_bench: DF.Link | None
source_candidate: DF.Link | None
status: DF.Literal[
"Pending", "Running", "Success", "Failure", "Recovering", "Recovered", "Fatal", "Scheduled"
"Pending",
"Running",
"Success",
"Failure",
"Recovering",
"Recovered",
"Fatal",
"Scheduled",
"Cancelled",
]
team: DF.Link | None
touched_tables: DF.Code | None
Expand Down Expand Up @@ -441,8 +449,7 @@ def deactivate_site(self):
job = agent.deactivate_site(
frappe.get_doc("Site", self.site), reference_doctype="Site Update", reference_name=self.name
)
frappe.db.set_value("Site Update", self.name, "deactivate_site_job", job.name)
frappe.db.set_value("Site Update", self.name, "status", "Running")
frappe.db.set_value("Site Update", self.name, {"deactivate_site_job": job.name, "status": "Running"})

def create_physical_backup(self):
site: Site = frappe.get_doc("Site", self.site)
Expand Down Expand Up @@ -713,11 +720,27 @@ def set_cause_of_failure_is_resolved(self):

@frappe.whitelist()
def set_status(self, status):
frappe.db.set_value("Site Update", self.name, "status", status)
return self.update_status(self.name, status)

@classmethod
def update_status(cls, name, status):
if status == "Cancelled":
try:
if (
_status := frappe.db.get_value("Site Update", name, "status", for_update=True, wait=False)
) != "Scheduled":
frappe.throw(f"Cannot cancel a Site Update with status {_status}")

except (frappe.QueryTimeoutError, frappe.QueryDeadlockError):
frappe.throw(
"The update is probably underway. Please reload/refresh to get the latest status."
)

frappe.db.set_value("Site Update", name, "status", status)


def update_status(name: str, status: str):
frappe.db.set_value("Site Update", name, "status", status)
SiteUpdate.update_status(name, status)
if status in ("Success", "Failure", "Fatal", "Recovered"):
frappe.db.set_value("Site Update", name, "update_end", frappe.utils.now())
update_start = frappe.db.get_value("Site Update", name, "update_start")
Expand Down Expand Up @@ -1102,7 +1125,10 @@ def run_scheduled_updates():

for update in updates:
try:
doc = frappe.get_doc("Site Update", update)
doc = frappe.get_doc("Site Update", update, for_update=True)
if doc.status != "Scheduled":
continue

doc.validate()
doc.start()
frappe.db.commit()
Expand Down
Loading