Skip to content
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

feat: Create plans and tiers tables #467

Merged
merged 9 commits into from
Jan 15, 2025
Merged

feat: Create plans and tiers tables #467

Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
d90351b
first pass to the plans and tiers tables
RulaKhaled Jan 9, 2025
2767d12
remove comments
RulaKhaled Jan 9, 2025
606857e
fix linting
RulaKhaled Jan 9, 2025
b132006
resolve comments
RulaKhaled Jan 10, 2025
ba51c68
update with migration
RulaKhaled Jan 10, 2025
33f51ff
update with correct name
RulaKhaled Jan 13, 2025
2af3938
create factories
RulaKhaled Jan 13, 2025
0ded145
add app-label back
ajay-sentry Jan 14, 2025
ec02329
update name of tables
RulaKhaled Jan 14, 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
47 changes: 47 additions & 0 deletions shared/django_apps/codecov_auth/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -997,3 +997,50 @@ def save(self, *args, **kwargs):
id=self.id
).update(is_active=False)
return super().save(*args, **kwargs)


class BillingRate(models.TextChoices):
MONTHLY = "monthly"
YEARLY = "yearly"
Copy link
Contributor

Choose a reason for hiding this comment

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

why not use PlanBillingRate?

Copy link
Contributor

Choose a reason for hiding this comment

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

this should be annually instead of yearly as well I think https://github.com/codecov/gazebo/blob/113a97f9e6a40ab9162576599308e123a25626aa/src/shared/utils/billing.ts#L29

Copy link
Contributor Author

Choose a reason for hiding this comment

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

didn't know we have it but i think we can't use enum.Enum directly in the choices argument of a Django model field, we'll have to do [(tag.value, tag.name) for tag in PlanBillingRate] which is not very clean.

I'll update to annually



class Plans(BaseModel):
tier = models.ForeignKey(
"Tiers", on_delete=models.CASCADE, related_name="plans", db_index=True
)
Copy link
Contributor

@nora-codecov nora-codecov Jan 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Suggested change
tier = models.ForeignKey(
"Tiers", on_delete=models.CASCADE, related_name="plans", db_index=True
)
tier = models.ForeignKey(
"Tiers", on_delete=models.SET_NULL, related_name="plans"
)

I removed the index, seems unnecessary for a small table, but up to you

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed with Nora, probably don't need an index here

Copy link
Contributor Author

@RulaKhaled RulaKhaled Jan 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

makes sense, tbh i always like to use indexes you never know how big the table will grow to be

base_unit_price = models.IntegerField(default=0)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
base_unit_price = models.IntegerField(default=0)
base_unit_price = models.IntegerField(default=0, blank=True)

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this one should always have a value, assuming blank=True means nullable

Copy link
Contributor Author

Choose a reason for hiding this comment

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

blank true means it can be null in the DB but it can't be optional in a form

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i think it makes sense if we are ever updating free plans

Copy link
Contributor

Choose a reason for hiding this comment

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

no, blank=True means you don't have to supply a value in a form/when you create the object, it's independent from nullable. it has to do with the field in code, not db. docs
if you have blank=False, you need to manually include a value for this field even if it's the default value
if you have blank=True, you don't need to explicitly set this field, if you leave it blank, it will use the default value. Because of this, I put blank=True on almost any field that has a default value so that I get the behavior I want.

benefits = ArrayField(models.TextField(), blank=True, default=list)
billing_rate = models.TextField(
choices=BillingRate.choices,
null=True,
blank=True,
)
is_active = models.BooleanField(default=True)
marketing_name = models.CharField(max_length=255)
max_seats = models.IntegerField(null=True, blank=True)
monthly_uploads_limit = models.IntegerField(null=True, blank=True)
paid_plan = models.BooleanField(default=False)
plan_name = models.CharField(max_length=255, unique=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

just a thought here, should we just call this "name?" since it's already on the plan 🤔


class Meta:
db_table = "plans"
Copy link
Contributor

Choose a reason for hiding this comment

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

is this needed?

Copy link
Contributor

Choose a reason for hiding this comment

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

what does this do? :O

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's just a clean way of naming the database

app_label = CODECOV_AUTH_APP_LABEL

def __str__(self):
return self.plan_name


class Tiers(BaseModel):
tier_name = models.CharField(max_length=255, unique=True)
bundle_analysis = models.BooleanField(default=False)
test_analytics = models.BooleanField(default=False)
flaky_test_detection = models.BooleanField(default=False)
patch_coverage = models.BooleanField(default=False)
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be project coverage right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes good catch

private_repo_support = models.BooleanField(default=False)

class Meta:
db_table = "tiers"
Copy link
Contributor

Choose a reason for hiding this comment

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

prob not needed for a new table unless there's naming overlap with another table

app_label = CODECOV_AUTH_APP_LABEL

def __str__(self):
return self.tier_name
Loading