From bd7a65975f66440b0e2d431ed96644a87db1321d Mon Sep 17 00:00:00 2001 From: badbl0cks <4161747+badbl0cks@users.noreply.github.com> Date: Tue, 8 Apr 2025 00:59:40 -0700 Subject: [PATCH] Bugfixes for emails and bugfixes for trade acceptance quantities being checked on create, closes #1 --- Dockerfile | 5 +- accounts/migrations/0001_initial.py | 4 +- accounts/views.py | 2 +- cards/migrations/0001_initial.py | 2 +- deploy.sh | 4 ++ reset-db_make-migrations_seed-data.sh | 3 - theme/templates/email/common/footer.txt | 5 +- theme/templates/email/common/header.txt | 2 +- theme/templates/email/common/subject.txt | 2 +- .../email/trades/trade_update_accepted.txt | 5 +- .../email/trades/trade_update_received.txt | 5 +- .../trade_update_rejected_by_acceptor.txt | 7 +- .../trade_update_rejected_by_initiator.txt | 7 +- .../email/trades/trade_update_sent.txt | 5 +- .../trade_update_thanked_by_acceptor.txt | 7 +- .../trades/trade_update_thanked_by_both.txt | 11 ++- .../trade_update_thanked_by_initiator.txt | 7 +- trades/forms.py | 4 +- trades/migrations/0001_initial.py | 2 +- trades/models.py | 72 +++++++++---------- trades/signals.py | 20 ++++-- 21 files changed, 95 insertions(+), 86 deletions(-) create mode 100644 deploy.sh diff --git a/Dockerfile b/Dockerfile index afbdd02..ba54ecc 100644 --- a/Dockerfile +++ b/Dockerfile @@ -2,7 +2,6 @@ FROM python:3.12.2-bookworm # Set environment variables -ENV PYTHONDONTWRITEBYTECODE 1 ENV PYTHONUNBUFFERED 1 # Create and set work directory called `app` @@ -22,16 +21,14 @@ COPY . /code/ COPY .env.production /code/.env ENV HOME=/code -# Install NPM & node.js RUN apt-get update && apt-get install -y nodejs npm xvfb netcat-openbsd +# Install playwright (via pip and install script) RUN playwright install-deps && playwright install # Expose port 8000 EXPOSE 8000 -#USER 10003:10003 - RUN python manage.py collectstatic --noinput #RUN python manage.py loaddata seed/* && python manage.py createcachetable django_cache diff --git a/accounts/migrations/0001_initial.py b/accounts/migrations/0001_initial.py index 756b7c6..f53f39c 100644 --- a/accounts/migrations/0001_initial.py +++ b/accounts/migrations/0001_initial.py @@ -1,4 +1,4 @@ -# Generated by Django 5.1.2 on 2025-04-07 04:19 +# Generated by Django 5.1.2 on 2025-04-08 06:24 import accounts.models import django.contrib.auth.models @@ -33,7 +33,7 @@ class Migration(migrations.Migration): ('is_active', models.BooleanField(default=True, help_text='Designates whether this user should be treated as active. Unselect this instead of deleting accounts.', verbose_name='active')), ('date_joined', models.DateTimeField(default=django.utils.timezone.now, verbose_name='date joined')), ('show_friend_code_on_link_previews', models.BooleanField(default=False, help_text='This will primarily affect share link previews on X, Discord, etc.', verbose_name='Show Friend Code on Link Previews')), - ('reputation_score', models.PositiveIntegerField(default=0)), + ('reputation_score', models.IntegerField(default=0)), ('groups', models.ManyToManyField(blank=True, help_text='The groups this user belongs to. A user will get all permissions granted to each of their groups.', related_name='user_set', related_query_name='user', to='auth.group', verbose_name='groups')), ('user_permissions', models.ManyToManyField(blank=True, help_text='Specific permissions for this user.', related_name='user_set', related_query_name='user', to='auth.permission', verbose_name='user permissions')), ], diff --git a/accounts/views.py b/accounts/views.py index c549b40..ceb176d 100644 --- a/accounts/views.py +++ b/accounts/views.py @@ -85,7 +85,7 @@ class DeleteFriendCodeView(LoginRequiredMixin, DeleteView): ) return redirect(self.get_success_url()) - trade_offer_exists = TradeOffer.all_offers.filter(initiated_by_id=self.object.pk).exists() + trade_offer_exists = TradeOffer.objects.filter(initiated_by_id=self.object.pk).exists() trade_acceptance_exists = TradeAcceptance.objects.filter(accepted_by_id=self.object.pk).exists() if trade_offer_exists or trade_acceptance_exists: diff --git a/cards/migrations/0001_initial.py b/cards/migrations/0001_initial.py index ecd4248..6428bab 100644 --- a/cards/migrations/0001_initial.py +++ b/cards/migrations/0001_initial.py @@ -1,4 +1,4 @@ -# Generated by Django 5.1.2 on 2025-04-07 04:19 +# Generated by Django 5.1.2 on 2025-04-08 06:24 import django.db.models.deletion from django.db import migrations, models diff --git a/deploy.sh b/deploy.sh new file mode 100644 index 0000000..2cfa5cf --- /dev/null +++ b/deploy.sh @@ -0,0 +1,4 @@ +#!/bin/bash +python manage.py migrate --noinput +python manage.py clear_cache +python manage.py collectstatic --noinput \ No newline at end of file diff --git a/reset-db_make-migrations_seed-data.sh b/reset-db_make-migrations_seed-data.sh index d19f192..a6a2fcf 100755 --- a/reset-db_make-migrations_seed-data.sh +++ b/reset-db_make-migrations_seed-data.sh @@ -24,6 +24,3 @@ uv run python manage.py loaddata seed/0* echo "Creating cache table..." uv run python manage.py createcachetable - -echo "Seeding default friend codes..." -uv run python manage.py seed_default_friend_codes \ No newline at end of file diff --git a/theme/templates/email/common/footer.txt b/theme/templates/email/common/footer.txt index 56ed0a4..092e55d 100644 --- a/theme/templates/email/common/footer.txt +++ b/theme/templates/email/common/footer.txt @@ -1,2 +1,3 @@ -Happy trading! -PKMN Trade Club \ No newline at end of file +Thank you for using PᴋMɴ Trade Club. + +Happy trading! \ No newline at end of file diff --git a/theme/templates/email/common/header.txt b/theme/templates/email/common/header.txt index cc18f03..4de79b2 100644 --- a/theme/templates/email/common/header.txt +++ b/theme/templates/email/common/header.txt @@ -1 +1 @@ -Hello {{ recipient_user }}, +Hello {{ recipient_user }}, \ No newline at end of file diff --git a/theme/templates/email/common/subject.txt b/theme/templates/email/common/subject.txt index b1467ca..5ba7688 100644 --- a/theme/templates/email/common/subject.txt +++ b/theme/templates/email/common/subject.txt @@ -1 +1 @@ -[PKMN Trade Club] \ No newline at end of file +[PᴋMɴ Trade Club] \ No newline at end of file diff --git a/theme/templates/email/trades/trade_update_accepted.txt b/theme/templates/email/trades/trade_update_accepted.txt index 9546ddd..7947a62 100644 --- a/theme/templates/email/trades/trade_update_accepted.txt +++ b/theme/templates/email/trades/trade_update_accepted.txt @@ -5,11 +5,12 @@ Great news! {{ acting_user }} ({{ acting_user_ign }} {{ acting_user_friend_code Trade Details: - They have: {{ want_card }} - They want: {{ has_card }} -(#{{ hash }}) What's next? You can now mark the trade as "Sent" once you've offered the card to them in the app, or reject the trade if needed. Visit your dashboard to manage this trade: {{ domain }}{% url 'dashboard' %} -{% include 'email/common/footer.txt' %} \ No newline at end of file +{% include 'email/common/footer.txt' %} + +Trade ID: #{{ hash }} \ No newline at end of file diff --git a/theme/templates/email/trades/trade_update_received.txt b/theme/templates/email/trades/trade_update_received.txt index 3626e16..975e4bf 100644 --- a/theme/templates/email/trades/trade_update_received.txt +++ b/theme/templates/email/trades/trade_update_received.txt @@ -5,11 +5,12 @@ Great news! {{ acting_user }} ({{ acting_user_ign }} {{ acting_user_friend_code Trade Details: - They sent: {{ want_card }} - They received: {{ has_card }} -(#{{ hash }}) What's next? Send a thank you to this user to increase their reputation! Visit your dashboard to send thanks: {{ domain }}{% url 'dashboard' %} -{% include 'email/common/footer.txt' %} \ No newline at end of file +{% include 'email/common/footer.txt' %} + +Trade ID: #{{ hash }} \ No newline at end of file diff --git a/theme/templates/email/trades/trade_update_rejected_by_acceptor.txt b/theme/templates/email/trades/trade_update_rejected_by_acceptor.txt index 4f97fb1..160e442 100644 --- a/theme/templates/email/trades/trade_update_rejected_by_acceptor.txt +++ b/theme/templates/email/trades/trade_update_rejected_by_acceptor.txt @@ -1,15 +1,16 @@ {% include 'email/common/header.txt' %} -We're sorry to inform you that {{ acting_user }} ({{ acting_user_friend_code }}) has canceled their trade acceptance. +We're sorry to inform you that {{ acting_user }} ({{ acting_user_ign }} {{ acting_user_friend_code }}) has canceled their trade acceptance. Trade Details: - They had: {{ want_card }} - They wanted: {{ has_card }} -(#{{ hash }}) Your trade offer is still active and available for other users to accept. Visit your dashboard to manage your trade offers: {{ domain }}{% url 'dashboard' %} -{% include 'email/common/footer.txt' %} \ No newline at end of file +{% include 'email/common/footer.txt' %} + +Trade ID: #{{ hash }} \ No newline at end of file diff --git a/theme/templates/email/trades/trade_update_rejected_by_initiator.txt b/theme/templates/email/trades/trade_update_rejected_by_initiator.txt index 12e80ba..46f5903 100644 --- a/theme/templates/email/trades/trade_update_rejected_by_initiator.txt +++ b/theme/templates/email/trades/trade_update_rejected_by_initiator.txt @@ -1,15 +1,16 @@ {% include 'email/common/header.txt' %} -We're sorry to inform you that {{ acting_user }} ({{ acting_user_friend_code }}) has rejected the trade. +We're sorry to inform you that {{ acting_user }} ({{ acting_user_ign }} {{ acting_user_friend_code }}) has rejected the trade. Trade Details: - You had: {{ has_card }} - You wanted: {{ want_card }} -(#{{ hash }}) Don't worry - there are plenty of other trade opportunities available! You can browse our marketplace for similar trades. Visit the marketplace: {{ domain }}{% url 'trade_offer_list' %} -{% include 'email/common/footer.txt' %} \ No newline at end of file +{% include 'email/common/footer.txt' %} + +Trade ID: #{{ hash }} \ No newline at end of file diff --git a/theme/templates/email/trades/trade_update_sent.txt b/theme/templates/email/trades/trade_update_sent.txt index 56781ba..e8c1625 100644 --- a/theme/templates/email/trades/trade_update_sent.txt +++ b/theme/templates/email/trades/trade_update_sent.txt @@ -5,11 +5,12 @@ Trade Details: - You have: {{ want_card }} - You want: {{ has_card }} -(#{{ hash }}) What's next? Once you respond to the trade in the app, please mark the trade as "Received" in your dashboard. Visit your dashboard to manage this trade: {{ domain }}{% url 'dashboard' %} -{% include 'email/common/footer.txt' %} \ No newline at end of file +{% include 'email/common/footer.txt' %} + +Trade ID: #{{ hash }} \ No newline at end of file diff --git a/theme/templates/email/trades/trade_update_thanked_by_acceptor.txt b/theme/templates/email/trades/trade_update_thanked_by_acceptor.txt index 8b0d384..fdb5801 100644 --- a/theme/templates/email/trades/trade_update_thanked_by_acceptor.txt +++ b/theme/templates/email/trades/trade_update_thanked_by_acceptor.txt @@ -1,15 +1,16 @@ {% include 'email/common/header.txt' %} -{{ acting_user }} ({{ acting_user_friend_code }}) has sent their thanks for the successful trade! +{{ acting_user }} ({{ acting_user_ign }} {{ acting_user_friend_code }}) has sent their thanks for the successful trade! Trade Details: - They sent: {{ want_card }} - They received: {{ has_card }} -(#{{ hash }}) What's next? Send a thank you to this user to increase their reputation! Visit your dashboard to send thanks: {{ domain }}{% url 'dashboard' %} -{% include 'email/common/footer.txt' %} \ No newline at end of file +{% include 'email/common/footer.txt' %} + +Trade ID: #{{ hash }} \ No newline at end of file diff --git a/theme/templates/email/trades/trade_update_thanked_by_both.txt b/theme/templates/email/trades/trade_update_thanked_by_both.txt index 3a9fe12..60dbfc0 100644 --- a/theme/templates/email/trades/trade_update_thanked_by_both.txt +++ b/theme/templates/email/trades/trade_update_thanked_by_both.txt @@ -1,14 +1,13 @@ {% include 'email/common/header.txt' %} -{{ acting_user }} ({{ acting_user_friend_code }}) has sent their thanks for the successful trade! +{{ acting_user }} ({{ acting_user_ign }} {{ acting_user_friend_code }}) has sent their thanks for the successful trade! Trade Details: -- {% if is_initiator %}They sent: {{ has_card }}{% else %}You sent: {{ want_card }}{% endif %} -- {% if is_initiator %}They received: {{ want_card }}{% else %}You received: {{ has_card }}{% endif %} -(#{{ hash }}) +- {% if is_initiator %}You{% else %}They{% endif %} sent: {{ want_card }} +- {% if is_initiator %}You{% else %}They{% endif %} received: {{ has_card }} This trade is now completed; no further actions can be made. -Thank you for using PKMN Trade Club. +{% include 'email/common/footer.txt' %} -{% include 'email/common/footer.txt' %} \ No newline at end of file +Trade ID: #{{ hash }} \ No newline at end of file diff --git a/theme/templates/email/trades/trade_update_thanked_by_initiator.txt b/theme/templates/email/trades/trade_update_thanked_by_initiator.txt index dabccd9..8ba2897 100644 --- a/theme/templates/email/trades/trade_update_thanked_by_initiator.txt +++ b/theme/templates/email/trades/trade_update_thanked_by_initiator.txt @@ -1,15 +1,16 @@ {% include 'email/common/header.txt' %} -{{ acting_user }} ({{ acting_user_friend_code }}) has sent their thanks for the successful trade! +{{ acting_user }} ({{ acting_user_ign }} {{ acting_user_friend_code }}) has sent their thanks for the successful trade! Trade Details: - You sent: {{ want_card }} - You received: {{ has_card }} -(#{{ hash }}) What's next? Send a thank you to this user to increase their reputation! Visit your dashboard to send thanks: {{ domain }}{% url 'dashboard' %} -{% include 'email/common/footer.txt' %} \ No newline at end of file +{% include 'email/common/footer.txt' %} + +Trade ID: #{{ hash }} \ No newline at end of file diff --git a/trades/forms.py b/trades/forms.py index 98b5e8e..02f7e62 100644 --- a/trades/forms.py +++ b/trades/forms.py @@ -62,7 +62,7 @@ class TradeAcceptanceCreateForm(forms.ModelForm): TradeAcceptance.AcceptanceState.RECEIVED, ] available_requested_ids = [] - for through_obj in trade_offer.trade_offer_have_cards.all(): + for through_obj in trade_offer.have_cards_available: active_count = trade_offer.acceptances.filter( requested_card=through_obj.card, state__in=active_states @@ -73,7 +73,7 @@ class TradeAcceptanceCreateForm(forms.ModelForm): # Update available offered_card choices from the TradeOffer's "want" side. available_offered_ids = [] - for through_obj in trade_offer.trade_offer_want_cards.all(): + for through_obj in trade_offer.want_cards_available: active_count = trade_offer.acceptances.filter( offered_card=through_obj.card, state__in=active_states diff --git a/trades/migrations/0001_initial.py b/trades/migrations/0001_initial.py index cc4f2db..e3193e5 100644 --- a/trades/migrations/0001_initial.py +++ b/trades/migrations/0001_initial.py @@ -1,4 +1,4 @@ -# Generated by Django 5.1.2 on 2025-04-07 04:19 +# Generated by Django 5.1.2 on 2025-04-08 06:24 import django.db.models.deletion from django.db import migrations, models diff --git a/trades/models.py b/trades/models.py index a7e2298..eab9fda 100644 --- a/trades/models.py +++ b/trades/models.py @@ -26,28 +26,22 @@ class TradeOfferManager(models.Manager): def get_queryset(self): qs = super().get_queryset().select_related( + "initiated_by", "initiated_by__user", ).prefetch_related( "trade_offer_have_cards__card", "trade_offer_want_cards__card", "acceptances", + "acceptances__accepted_by", "acceptances__requested_card", "acceptances__offered_card", "acceptances__accepted_by__user", ) - cutoff = timezone.now() - timedelta(days=28) - qs = qs.filter(created_at__gte=cutoff) return qs.order_by("-updated_at") -class TradeOfferAllManager(models.Manager): - def get_queryset(self): - # Return all trade offers without filtering by the cutoff. - return super().get_queryset() - class TradeOffer(models.Model): objects = TradeOfferManager() - all_offers = TradeOfferAllManager() id = models.AutoField(primary_key=True) is_closed = models.BooleanField(default=False, db_index=True) @@ -106,6 +100,17 @@ class TradeOffer(models.Model): # Use super().save() here to avoid recursion. super(TradeOffer, self).save(update_fields=["rarity_level", "rarity_icon"]) + # New derived properties for available cards + @property + def have_cards_available(self): + # Returns the list of have_cards (through objects) that still have available quantity. + return [item for item in self.trade_offer_have_cards.all() if item.quantity > item.qty_accepted] + + @property + def want_cards_available(self): + # Returns the list of want_cards (through objects) that still have available quantity. + return [item for item in self.trade_offer_want_cards.all() if item.quantity > item.qty_accepted] + class TradeOfferHaveCard(models.Model): """ Through model for TradeOffer.have_cards. @@ -175,6 +180,16 @@ class TradeAcceptance(models.Model): THANKED_BY_BOTH = 'THANKED_BY_BOTH', 'Thanked by Both' REJECTED_BY_INITIATOR = 'REJECTED_BY_INITIATOR', 'Rejected by Initiator' REJECTED_BY_ACCEPTOR = 'REJECTED_BY_ACCEPTOR', 'Rejected by Acceptor' + + # DRY improvement: define active states once as a class-level constant. + ACTIVE_STATES = [ + AcceptanceState.ACCEPTED, + AcceptanceState.SENT, + AcceptanceState.RECEIVED, + AcceptanceState.THANKED_BY_INITIATOR, + AcceptanceState.THANKED_BY_ACCEPTOR, + AcceptanceState.THANKED_BY_BOTH, + ] trade_offer = models.ForeignKey( TradeOffer, @@ -187,13 +202,11 @@ class TradeAcceptance(models.Model): on_delete=models.PROTECT, related_name='trade_acceptances' ) - # The acceptor selects one card the initiator is offering (from have_cards) requested_card = models.ForeignKey( "cards.Card", on_delete=models.PROTECT, related_name='accepted_requested' ) - # And one card from the initiator's wanted cards (from want_cards) offered_card = models.ForeignKey( "cards.Card", on_delete=models.PROTECT, @@ -266,40 +279,25 @@ class TradeAcceptance(models.Model): return not self.is_completed_or_rejected def clean(self): - # Validate that the requested and offered cards exist in the through tables. + from django.core.exceptions import ValidationError try: - have_through_obj = self.trade_offer.trade_offer_have_cards.get(card_id=self.requested_card_id) + have_card = self.trade_offer.trade_offer_have_cards.get(card_id=self.requested_card_id) except TradeOfferHaveCard.DoesNotExist: raise ValidationError("The requested card must be one of the trade offer's available cards (have_cards).") - try: - want_through_obj = self.trade_offer.trade_offer_want_cards.get(card_id=self.offered_card_id) + want_card = self.trade_offer.trade_offer_want_cards.get(card_id=self.offered_card_id) except TradeOfferWantCard.DoesNotExist: raise ValidationError("The offered card must be one of the trade offer's requested cards (want_cards).") - if not self.pk and self.trade_offer.is_closed: - raise ValidationError("This trade offer is closed. No more acceptances are allowed.") - - active_states = [ - self.AcceptanceState.ACCEPTED, - self.AcceptanceState.SENT, - self.AcceptanceState.RECEIVED, - self.AcceptanceState.THANKED_BY_INITIATOR, - self.AcceptanceState.THANKED_BY_ACCEPTOR, - self.AcceptanceState.THANKED_BY_BOTH, - ] - - active_acceptances = self.trade_offer.acceptances.filter(state__in=active_states) - if self.pk: - active_acceptances = active_acceptances.exclude(pk=self.pk) - - requested_count = active_acceptances.filter(requested_card_id=self.requested_card_id).count() - if requested_count >= have_through_obj.quantity: - raise ValidationError("This requested card has been fully accepted.") - - offered_count = active_acceptances.filter(offered_card_id=self.offered_card_id).count() - if offered_count >= want_through_obj.quantity: - raise ValidationError("This offered card has already been fully used.") + # Only perform these validations on creation (when self.pk is None). + if self.pk is None: + if self.trade_offer.is_closed: + raise ValidationError("This trade offer is closed. No more acceptances are allowed.") + # Use direct comparison with qty_accepted and quantity. + if have_card.qty_accepted >= have_card.quantity: + raise ValidationError("The requested card has no available quantity.") + if want_card.qty_accepted >= want_card.quantity: + raise ValidationError("The offered card has no available quantity.") def get_step_number(self): if self.state in [ diff --git a/trades/signals.py b/trades/signals.py index df92f30..0d94e5c 100644 --- a/trades/signals.py +++ b/trades/signals.py @@ -58,6 +58,10 @@ def update_trade_offer_closed_status(trade_offer): @receiver(pre_save, sender=TradeAcceptance) def trade_acceptance_pre_save(sender, instance, **kwargs): + # Skip signal processing during raw fixture load or when saving a new instance + if kwargs.get("raw", False) or instance._state.adding: + return + if instance.pk: old_instance = TradeAcceptance.objects.get(pk=instance.pk) instance._old_state = old_instance.state @@ -98,9 +102,9 @@ def trade_acceptance_email_notification(sender, instance, created, **kwargs): return # check if were in debug mode - if settings.DEBUG: - print("DEBUG: skipping email notification in debug mode") - return + # if settings.DEBUG: + # print("DEBUG: skipping email notification in debug mode") + # return acting_user = instance._actioning_user state = instance.state @@ -126,14 +130,16 @@ def trade_acceptance_email_notification(sender, instance, created, **kwargs): # Determine the non-acting party: - if instance.trade_offer.initiated_by == acting_user: + if instance.trade_offer.initiated_by.user.pk == acting_user.pk: # The initiator made the change; notify the acceptor. recipient_user = instance.accepted_by.user - else: + elif instance.accepted_by.user.pk == acting_user.pk: # The acceptor made the change; notify the initiator. recipient_user = instance.trade_offer.initiated_by.user + else: + return - is_initiator = instance.trade_offer.initiated_by == acting_user + is_initiator = instance.trade_offer.initiated_by.user.pk == acting_user.pk email_context = { "has_card": instance.requested_card, @@ -145,7 +151,7 @@ def trade_acceptance_email_notification(sender, instance, created, **kwargs): "recipient_user_ign": instance.accepted_by.in_game_name if is_initiator else instance.trade_offer.initiated_by.in_game_name, "acting_user_friend_code": instance.trade_offer.initiated_by.friend_code if is_initiator else instance.accepted_by.friend_code, "is_initiator": is_initiator, - "domain": Site.objects.get_current().domain, + "domain": "https://" + Site.objects.get_current().domain, "pk": instance.pk, } email_template = "email/trades/trade_update_" + state + ".txt"