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

Jump to bottom

Bonus links and transactions sketch #3648

Draft
wants to merge 2 commits into
base: develop
Choose a base branch
from
Draft

Bonus links and transactions sketch #3648

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
faf2470
WIP
jcushman Nov 1, 2024
5e7d493
select_for_update in single query
jcushman Nov 1, 2024
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
8 changes: 2 additions & 6 deletions perma_web/api/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -497,10 +497,7 @@ def post(self, request, format=None):
if not folder.organization and not folder.sponsored_by:
links_remaining, _ , bonus_links = user.get_links_remaining()
if bonus_links and not links_remaining:
# (this works because it's part of the same transaction with the select_for_update --
# we don't have to use the same object)
request.user.bonus_links = bonus_links - 1
request.user.save(update_fields=['bonus_links'])
user.update_bonus_links(-1)
bonus_link = True

link = serializer.save(created_by=request.user, bonus_link=bonus_link)
Expand Down Expand Up @@ -665,8 +662,7 @@ def delete(self, request, guid, format=None):
link.save()

if link.bonus_link:
link.created_by.bonus_links = (link.created_by.bonus_links or 0) + 1
link.created_by.save(update_fields=['bonus_links'])
link.created_by.update_bonus_links(1)

return Response(status=status.HTTP_204_NO_CONTENT)

Expand Down
33 changes: 33 additions & 0 deletions perma_web/perma/migrations/0052_alter_historicallinkuser_bonus_links_and_more.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
# Generated by Django 4.2.16 on 2024-11-01 13:38

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
('perma', '0051_auto_20241030_1732'),
]

operations = [
migrations.AlterField(
model_name='historicallinkuser',
name='bonus_links',
field=models.PositiveIntegerField(default=0),
),
migrations.AlterField(
model_name='historicalregistrar',
name='bonus_links',
field=models.PositiveIntegerField(default=0),
),
migrations.AlterField(
model_name='linkuser',
name='bonus_links',
field=models.PositiveIntegerField(default=0),
),
migrations.AlterField(
model_name='registrar',
name='bonus_links',
field=models.PositiveIntegerField(default=0),
),
]
45 changes: 29 additions & 16 deletions perma_web/perma/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
from django.conf import settings
from django.core.files.storage import storages
from django.db import models, transaction
from django.db.models import Q, Max, Count, Sum, JSONField, F, Exists, OuterRef, When, Case
from django.db.models import Q, Max, Count, Sum, JSONField, F, Exists, OuterRef, When, Case, Greatest
from django.db.models.functions import Now, Upper, TruncDate
from django.db.models.query import QuerySet
from django.contrib.postgres.indexes import GistIndex, GinIndex, OpClass
Expand Down Expand Up @@ -198,7 +198,7 @@ class Meta:
unlimited = models.BooleanField(default=False, help_text="If unlimited, link_limit and related fields are ignored.")
link_limit = models.IntegerField(default=settings.DEFAULT_CREATE_LIMIT)
link_limit_period = models.CharField(max_length=8, default=settings.DEFAULT_CREATE_LIMIT_PERIOD, choices=(('once','once'),('monthly','monthly'),('annually','annually')))
bonus_links = models.PositiveIntegerField(blank=True, null=True)
bonus_links = models.PositiveIntegerField(default=0)

@cached_property
def customer_type(self):
Expand Down Expand Up @@ -504,8 +504,7 @@ def credit_for_purchased_links(self, purchases):
try:
with transaction.atomic():
link_quantity = int(purchase["link_quantity"])
self.bonus_links = (self.bonus_links or 0) + link_quantity
self.save(update_fields=['bonus_links'])
self.update_bonus_links(link_quantity)
try:
r = requests.post(
settings.ACKNOWLEDGE_PURCHASE_URL,
Expand Down Expand Up @@ -879,11 +878,11 @@ def save(self, *args, **kwargs):
# make sure email is still formatted correctly.
self.format_email_fields()

with transaction.atomic():
super().save(*args, **kwargs)

if not self.root_folder_id:
# make sure root folder is created for each user.
if not self.root_folder_id:
with transaction.atomic():
super().save(*args, **kwargs)

root_folder = Folder.objects.create(
name='Personal Links',
created_by=self,
Expand All @@ -894,6 +893,10 @@ def save(self, *args, **kwargs):
# so we don't run through our custom logic twice
super().save()

else:
# regular save, no transaction
super().save(*args, **kwargs)

def get_full_name(self):
""" Use either First Last or first half of email address as user's name. """
return f"{self.first_name} {self.last_name}" if self.first_name or self.last_name else self.email.split('@')[0]
Expand Down Expand Up @@ -1141,8 +1144,8 @@ def get_links_remaining(self):
# Special handling for non-trial users who lack active paid subscriptions:
# apply the same rules that are applied to new users
if not self.in_trial and not self.nonpaying and self.subscription_status != 'active':
return (self.links_remaining_in_period(settings.DEFAULT_CREATE_LIMIT_PERIOD, settings.DEFAULT_CREATE_LIMIT, unlimited=False), settings.DEFAULT_CREATE_LIMIT_PERIOD, self.bonus_links or 0)
return (self.links_remaining_in_period(self.link_limit_period, self.link_limit), self.link_limit_period, self.bonus_links or 0)
return (self.links_remaining_in_period(settings.DEFAULT_CREATE_LIMIT_PERIOD, settings.DEFAULT_CREATE_LIMIT, unlimited=False), settings.DEFAULT_CREATE_LIMIT_PERIOD, self.bonus_links)
return (self.links_remaining_in_period(self.link_limit_period, self.link_limit), self.link_limit_period, self.bonus_links)

def link_creation_allowed(self):
links_remaining, _, bonus_links = self.get_links_remaining()
Expand Down Expand Up @@ -1218,6 +1221,19 @@ def remove_line_from_notes(self, containing):
if self.notes:
self.notes = re.sub(f"\n*{containing}.*", '', self.notes)

def update_bonus_links(self, count):
# Use Greatest to ensure bonus_links doesn't go below 0
LinkUser.objects.filter(id=self.id).update(
bonus_links=Greatest(F('bonus_links') + count, 0)
)

# mark field as deferred so we don't rely on an outdated value
if not hasattr(self, '_deferred_fields'):
self._deferred_fields = set()
self._deferred_fields.add('bonus_links')
if hasattr(self, 'bonus_links'):
delattr(self, 'bonus_links')


class UserOrganizationAffiliation(models.Model):
organization = models.ForeignKey(Organization, on_delete=models.CASCADE)
Expand Down Expand Up @@ -1493,8 +1509,7 @@ def update_parents_cached_has_children(parent_id=None, previous_parent_id=None):
if (parent.organization_id or parent.sponsored_by_id) and (any_link := bonus_links.first()):
user = any_link.created_by
count = bonus_links.update(bonus_link=False)
user.bonus_links = F('bonus_links') + count
user.save(update_fields=['bonus_links'])
user.update_bonus_links(count)

# update the cached paths of this folder and all its descendants
update_cached_path(subtree_ids, parent.tree_root_id)
Expand Down Expand Up @@ -1827,8 +1842,7 @@ def move_to_folder_for_user(self, folder, user):
# Don't let anybody move folders around, until this link is
# safely inside its destination folder, lest denormalized
# ownership-related fields get out of sync
for folder in itertools.chain(self.folders.all(), [folder]):
Folder.objects.select_for_update().get(pk=folder.tree_root_id)
Folder.objects.filter(pk_in=itertools.chain(self.folders.all(), [folder])).select_for_update().values_list('id')

# remove this link from any folders it's in for this user
self.folders.remove(*self.folders.accessible_to(user))
Expand All @@ -1840,10 +1854,9 @@ def move_to_folder_for_user(self, folder, user):
self.organization = folder.organization
if self.bonus_link and (folder.organization or folder.sponsored_by):
self.bonus_link = False
user.bonus_links = F('bonus_links') + 1
user.update_bonus_links(1)

self.save(update_fields=['organization', 'bonus_link'])
user.save(update_fields=['bonus_links'])

def guid_as_path(self):
# For a GUID like ABCD-1234, return a path like AB/CD/12.
Expand Down
Loading