Skip to content

fix(site): Add server side validation for new site and server - #7455

Merged
shadrak98 merged 2 commits into
frappe:developfrom
shadrak98:add-server-side-vld
Sep 11, 2026
Merged

fix(site): Add server side validation for new site and server#7455
shadrak98 merged 2 commits into
frappe:developfrom
shadrak98:add-server-side-vld

Conversation

@shadrak98

Copy link
Copy Markdown
Contributor

Earlier this validation was only on client side for teams on unpaid invoice, it will now check on server side as well.

@greptile-apps

greptile-apps Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 3/5

Not safe to merge until the parent-team regression and outstanding cancelled-invoice counting issue are fixed.

Reviews (2) · Last reviewed commit: "fix(team): Check spending limit in serve..."

def has_unpaid_invoices(self):
"""Two or more unpaid subscription invoices blocks new sites and servers, matching the New Site and New Server forms."""
return (
frappe.db.count("Invoice", {"team": self.name, "status": "Unpaid", "type": "Subscription"}) >= 2

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Cancelled invoices block creation

Cancellation sets docstatus to 2 without changing an invoice's Unpaid status. This count therefore includes cancelled invoices, so two cancelled invoices—or one cancelled and one current invoice—can incorrectly block site and server creation.

Suggested change
frappe.db.count("Invoice", {"team": self.name, "status": "Unpaid", "type": "Subscription"}) >= 2
frappe.db.count(
"Invoice",
{"team": self.name, "status": "Unpaid", "type": "Subscription", "docstatus": ("<", 2)},
)
>= 2
Prompt To Fix With AI
This is a comment left during a code review.
Path: press/press/doctype/team/team.py
Line: 817

Comment:
**Cancelled invoices block creation**

Cancellation sets `docstatus` to 2 without changing an invoice's `Unpaid` status. This count therefore includes cancelled invoices, so two cancelled invoices—or one cancelled and one current invoice—can incorrectly block site and server creation.

```suggestion
			frappe.db.count(
				"Invoice",
				{"team": self.name, "status": "Unpaid", "type": "Subscription", "docstatus": ("<", 2)},
			)
			>= 2
```

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

@codecov-commenter

codecov-commenter commented Sep 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 62.02%. Comparing base (07485b0) to head (faf67df).
⚠️ Report is 18 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #7455      +/-   ##
===========================================
+ Coverage    61.94%   62.02%   +0.07%     
===========================================
  Files         1067     1067              
  Lines       102449   102569     +120     
  Branches      1777     1779       +2     
===========================================
+ Hits         63460    63615     +155     
+ Misses       38944    38909      -35     
  Partials        45       45              
Flag Coverage Δ
dashboard 85.94% <ø> (ø)

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

☔ View full report in Codecov by Harness.
📢 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.

@mergify

mergify Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

Comment on lines +1320 to +1322
if self.apply_limits and self.spending_limit <= self.total_subscribed_amount():
why = "You have exceeded your spending limit. Please contact support to increase your limits."
return (False, why)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Child Teams Lose Exemption

Invited child teams have apply_limits enabled by default, but this limit check runs before the parent_team exemption. Once a child team reaches its spending limit, it is incorrectly blocked from creating sites despite the existing exemption.

Prompt To Fix With AI
This is a comment left during a code review.
Path: press/press/doctype/team/team.py
Line: 1320-1322

Comment:
**Child Teams Lose Exemption**

Invited child teams have `apply_limits` enabled by default, but this limit check runs before the `parent_team` exemption. Once a child team reaches its spending limit, it is incorrectly blocked from creating sites despite the existing exemption.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

@shadrak98
shadrak98 merged commit 64860a9 into frappe:develop Sep 11, 2026
13 of 14 checks passed
shadrak98 added a commit that referenced this pull request Sep 11, 2026
fix(site): Add server side validation for new site and server (backport #7455)
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