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.

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

feature/55 first approach to campaigns #92

Closed
wants to merge 12 commits into from

Conversation

juan-ignacio-sanchez
Copy link
Contributor

@juan-ignacio-sanchez juan-ignacio-sanchez commented Dec 17, 2020

Description

Creates a new Model called "Campaign" that groups Drips through the CampaignDrip intermediate Model. Also allows the lib's user to configure the Drip Model to be used.

Closes issue(s)

Screenshots (if appropriate)

Changes include

  • Bugfix (non-breaking change that solves an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (change that is not backwards-compatible and/or changes current functionality)

from drip.models import Drip
from campaigns.models import Campaign, CampaignDrip

DRIP_AMOUNT = 10
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would use random.randint. Just a suggestion

DRIP_AMOUNT = 10


class DripsTestCase(TestCase):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please test also using a class different from Drip, to test the GenericForeignKey part

Comment on lines 15 to 17
return Drip.objects.filter(
id__in=self.campaign_drips.values_list('id', flat=True)
)
Copy link

Choose a reason for hiding this comment

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

This should be a model manager to maintain the django interface

@kaozdl
Copy link

kaozdl commented Dec 22, 2020

Looks good so far, keep in mind what we discussed 🚀

@juan-ignacio-sanchez juan-ignacio-sanchez changed the title WIP: feature/55 first approach to campaigns feature/55 first approach to campaigns Jan 8, 2021
@juan-ignacio-sanchez juan-ignacio-sanchez dismissed brunomichetti’s stale review January 8, 2021 02:38

This does not make sense anymore since I've changed implementation

@juan-ignacio-sanchez juan-ignacio-sanchez linked an issue Jan 8, 2021 that may be closed by this pull request
@brunomichetti brunomichetti removed their request for review January 28, 2022 13:23
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.

Campaigns
4 participants