Skip to content

Forbid negative premium prices#3108

Merged
gbrodman merged 1 commit into
google:masterfrom
gbrodman:negativePrices
Jul 7, 2026
Merged

Forbid negative premium prices#3108
gbrodman merged 1 commit into
google:masterfrom
gbrodman:negativePrices

Conversation

@gbrodman

@gbrodman gbrodman commented Jun 25, 2026

Copy link
Copy Markdown
Collaborator

This hasn't been an issue, but it seems like a good idea. Note that I'm allowing prices of 0 here just in case there's some shenanigans at some point in time where we want a domain to technically be premium without having a cost.


This change is Reviewable

This hasn't been an issue, but it seems like a good idea. Note that I'm
allowing prices of 0 here just in case there's some shenanigans at some
point in time where we want a domain to technically be premium without
having a cost.
@gbrodman
gbrodman requested a review from CydeWeys July 1, 2026 17:40

@CydeWeys CydeWeys left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I wonder if there's anywhere else in the codebase we should enforce prices/fees being non-negative? Maybe in the Fee classes, and something in the Tld class that sets the standard create/renewal price?

@CydeWeys made 1 comment.
Reviewable status: 0 of 2 files reviewed, all discussions resolved.

@CydeWeys CydeWeys left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@CydeWeys reviewed 2 files and all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on gbrodman).

@gbrodman gbrodman left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

TLDs are an odd case because those are just loaded automatically from the YAML. For Fees, we do already verify that it's nonnegative.

@gbrodman made 1 comment.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on gbrodman).

@gbrodman
gbrodman added this pull request to the merge queue Jul 7, 2026

@CydeWeys CydeWeys left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

But we could throw an error on negative values when loading from the YAML, right? Also the YAML doesn't even have to be checked into source control (for non-production domains).

@CydeWeys made 1 comment.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on gbrodman).

@gbrodman gbrodman left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

it's not quite that simple but kinda yeah, doing that in a separate PR

@gbrodman made 1 comment.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on gbrodman).

Merged via the queue into google:master with commit 4d6b5a8 Jul 7, 2026
16 checks passed
@gbrodman
gbrodman deleted the negativePrices branch July 7, 2026 20:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants